jedireza/aqua

Why not use connect() to bind stores

jacogreyling opened this issue · 3 comments

Just curious,

any particular reason why you've not opted for the react-redux module in your code? All the documentation around shows how to connect a React component to a Redux store using connect().

As per the official documentation: "If you really need to, you can manually pass store as a prop to every connect()ed component, but we only recommend to do this for stubbing store in unit tests, or in non-fully-React codebases. Normally, you should just use ."

This is not criticism, I'm just curious as to the rationale behind it?

Thx!

Thanks for asking. I don't remember my original thoughts for not using react-redux. Maybe it was to keep things simple (one less module/dependency).

After skimming some of the docs...

...there are a couple things that stick out to me:

  • First, connect() brings a little bit of magic to the table, which I try to stay away from with Aqua.
  • Second, I'm not a huge fan of the API. An example of some syntax that I think is confusing to look at is:
const VisibleTodoList = connect(
  mapStateToProps,
  mapDispatchToProps
)(TodoList)

Maybe I haven't given it enough time to grow on me. The docs do mention:

Technically you could write the container components by hand using store.subscribe(). We don't advise you to do this because React Redux makes many performance optimizations that are hard to do by hand.

So I don't doubt there are some upsides to using it. I'd be interested in the effect using react-redux would have to the Aqua code base. I don't plan on spending time looking into this, but if someone wants to make a proposal and show some examples I'd be happy to work through it more then.

Thank you as always for your comprehensive response. I agree I also don't like the notation used with overloaded methods (ok so I come from a Java background). As you mention there seems to be upsides though that needs to be explored further.

Just looking at the connect() function I can see the following:

      componentDidMount() {
        // it remembers to subscribe to the store so it doesn't miss updates
        this.unsubscribe = store.subscribe(this.handleChange.bind(this))
      }
      
      componentWillUnmount() {
        // and unsubscribe later
        this.unsubscribe()
      }
    
      handleChange() {
        // and whenever the store state changes, it re-renders.
        this.forceUpdate()
      }

Comparing that to your code, it's very similar. The only difference is on an update they call the React forceUpdate() API which will in turn calls render(). (They handle getState() in render())

FWIW as someone new to react I appreciated seeing another way of implementing the connect() paradigm and agree that the connect API syntax is confusing.