undefined | Better HN

0 pointsacemarke8y ago0 comments

To some extent, it was - if you look at the earliest versions of Redux, `connect()` and its predecessor forms were indeed being used as a decorator.

However, I personally advise against using `connect()` as a decorator, for several reasons:

- It's still a Stage 2 proposal. Now, the Class Properties syntax isn't final either (currently Stage 3), which the React team (and I) highly recommend using. However, the Class Properties syntax seems to be much more stable, the behavior it's implementing is a lot simpler, and if by chance it happens to change in the future, it should be relatively easy to code-mod (and the React team has said they would release a code-mod if that happens). Meanwhile, the decorators spec has changed several times (including recently), and the Babel plugins have also had to change behavior and implementation over time.

- It obscures the real class definition. The standard advice for testing Redux-connected components is to export the "plain" class separately as a named export, and `export default connect()(MyComponent)`, then import and test the plain version. If you use @connect() as a decorator, the plain version isn't accessible, and testing becomes more difficult.

- Going along with that, I've seen many questions about why defaultProps and propTypes don't work right when @connect() is used, and it's because those get applied to the wrapper component, not the plain component, and thus things don't work the way you would want them to.

I see no advantages to using connect as a decorator. I encourage people to write their mapState functions separately anyway for clarity and testability (instead of inline as an argument to connect), so it's just a matter of moving the line with `connect` elsewhere in the file and changing the syntax slightly.

0 comments

couchand8y ago

I definitely wasn't recommending that usage, just illustrating what it might look like. I personally think it looks much cleaner, which I would call an advantage.

I've considered the first point, but hadn't thought about the second and third. I'm guessing the last point isn't a concern if you're using class properties for those?

Given all this, if you were redesigning the API today, would you now make connect take the component as the first parameter instead?

acemarkeOP8y ago

No. The reason why it's written as not just a HOC, but a curried-ish function is so that you could potentially reuse the "connection" definition for multiple components:

    const SpecificConnection = connect(mapState, mapDispatch);
    const ConnectedFirst = SpecificConnection(FirstComponent);
    const ConnectedSecond = SpecificConnection(SecondComponent);

Admittedly, I suspect that use case isn't popping up very often. Most of the time what I see is that a given component type is only connected once, and a given connection setup is only used with one component. I also don't remember seeing specific comments by Dan or Andrew saying that's why it's written this way - I'm inferring the intent from the API definition. Still, it's a potentially useful capability in the API, so no reason to throw it away.

couchand8y ago

Theoretically that could be useful, but I've also never seen connections used this way. I have seen components connected in more than one way, but of course that's no easier with the curried style. And, I'd point out, if someone actually wanted to share the configuration, it's trivial enough to write:

  const SpecificConnection = (Component) => connect(Component, mapState, mapDispatch);

Not that I'd argue against currying generally -- it's only that it turns out to be a little awkward here with the various optional parameters to connect, and also that the connection setup in practice seems to be tightly coupled to the connected component.

j / k navigate · click thread line to collapse

0 pointsacemarke8y ago0 comments

To some extent, it was - if you look at the earliest versions of Redux, `connect()` and its predecessor forms were indeed being used as a decorator.

However, I personally advise against using `connect()` as a decorator, for several reasons:

0 comments

couchand8y ago

I definitely wasn't recommending that usage, just illustrating what it might look like. I personally think it looks much cleaner, which I would call an advantage.

I've considered the first point, but hadn't thought about the second and third. I'm guessing the last point isn't a concern if you're using class properties for those?

Given all this, if you were redesigning the API today, would you now make connect take the component as the first parameter instead?

acemarkeOP8y ago

No. The reason why it's written as not just a HOC, but a curried-ish function is so that you could potentially reuse the "connection" definition for multiple components:

    const SpecificConnection = connect(mapState, mapDispatch);
    const ConnectedFirst = SpecificConnection(FirstComponent);
    const ConnectedSecond = SpecificConnection(SecondComponent);

couchand8y ago

  const SpecificConnection = (Component) => connect(Component, mapState, mapDispatch);

j / k navigate · click thread line to collapse