Alternative API proposals

[gaearon]

This API is taken from Redux <Provider>, <Connector>, @provide, @connect.
Right now I don't have examples in this repo but it matches what you've seen in Redux so far.

It's the best we've got now, but I think we can do better!

Common pain points:

• Not intuitive how way to separate smart and dumb components with <Connector>, @connect
• You have to manually bind action creators with bindActionCreators helper which some don't like
• Too much nesting for small examples (<Provider>, <Connector> both need function children)

Let's go wild here. Post your alternative API suggestions.

They should satisfy the following criteria:

• Some component at the root must hold the store instance. (Akin to <Provider>)
• It should be possible to connect to state no matter how deep in the tree
• It should be possible to select the state you're interested in with a select function
• Smart / dumb components separation needs to be encouraged
• There should be one obvious way to separate smart / dumb components
• It should be obvious how to turn your functions into action creators
• Smart components should probably be able to react to updates to the state in componentDidUpdate
• Smart components' select function needs to be able to take their props into account
• Smart component should be able to do something before/after dumb component dispatches an action
• We should have shouldComponentUpdate wherever we can

[gaearon]

For example, we could drop <Connector> altogether and encourage people to use @connect decorator as a function in another module.

Common case

A dumb component would look exactly as it does now.
A smart component would look like

import React from 'react';
import { connect } from 'react-redux';
import Counter from '../components/Counter';
import * as CounterActions from '../actions/CounterActions';

export default connect(
  state => ({
    counter: state.counter
  }),
  dispatch => ({
    increment: dispatch(CounterActions.increment()),
    decrement: dispatch(CounterActions.decrement())
  })
)(Counter);

Note that the smart component doesn't have to be declared as a component. Also note that state => ... and dispatch => ... is all it accepts.

Case with more control

Want more customization? Want a componentDidUpdate hook? Want to select different things depending on the current props? Well, maybe you need to put a component in the middle then:

import React from 'react';
import connect from './connect';
import { bindActionCreators } from 'redux';
import Counter from '../components/Counter';
import * as CounterActions from '../actions/CounterActions';

class CounterContainer {
  componentDidUpdate() {
    ...
  }

  render() {
    const props = somehowSelectChildProps(this.props);
    return <Counter {...props} />
  }
}

export default connect(
  state => ({
    counter: state.counter
  }),
  dispatch => ({
    increment: () => dispatch(CounterActions.increment()),
    decrement: () => dispatch(CounterActions.decrement())
  })
)(CounterContainer);

This is an “explicit” smart component that is required for more advanced cases.
Note that you didn't have to move files or refactor anything.
You just put a component in the middle into the same file.

Shortcuts

Finally, we can still offer bindActionCreators, but with a actionCreators => dispatch => obj signature, so that the result is usable as the second parameter:

import React from 'react';
import { connect } from 'react-redux';
import Counter from '../components/Counter';
import * as CounterActions from '../actions/CounterActions';

export default connect(
  state => ({  counter: state.counter }),
  bindActionCreators(CounterActions)
)(Counter);

Perhaps we can even go further and bind automatically if an object is passed.

import React from 'react';
import { connect } from 'react-redux';
import Counter from '../components/Counter';
import * as CounterActions from '../actions/CounterActions';

export default connect(
  state => ({  counter: state.counter }),
  CounterActions
)(Counter);

“Wait!”, I hear you say. What if an action depends on some prop from the state? Well, in this case you put a component in the middle like I described above.

import React from 'react';
import { connect } from 'react-redux';
import Counter from '../components/Counter';
import * as CounterActions from '../actions/CounterActions';

class CounterContainer {
  increment() {
    this.props.increment(this.props.id);
  }

  render() {
    return <Counter {...this.props} increment={this.increment} />
  }
}

export default connect(
  state => ({  counter: state.counter }),
  CounterActions
)(CounterContainer);

Any sufficiently complicated case => component in the middle. Easy!

Am I missing something?

[acdlite]

👍 I like this. It's essentially the same API as https://github.com/acdlite/redux-rx#createconnectorselectstate-render, just without streams.

[acdlite]

Ah, only thing I see missing is a way to access props from owner — e.g. if you're wrapped by Relay.

[gaearon]

Can you clarify?
Isn't this “Case with more control” above?

[acdlite]

That works, I'm just not sure I like it... Seems like too common a case to necessitate what is essentially two smart components.

[acdlite]

Could we just pass the owner props as the second argument?

[gaearon]

From the user point of view they're just declaring one “real” component so no big deal IMO. On the other hand once you start selecting data in a tricky way, you begin to want finer control over perf optimizations, lifecycle hooks and maybe passing data down in a trickier way so it's likely you'll want a component anyway.

Once we start passing props to the state getter, we'll also probably want to pass props to the action creators getter. However, this forces us to bind on every prop change, which is a perf hit and unfriendly to shouldComponentUpdate optimizations down the rendering chain.

Example: you might want to read from props if you have a route handler that gets specific data related to the current route parameters. But then you already want to fire an action in componentWillReceiveProps and componentDidMount to fetch the data! So you already need an intermediate component anyway.

[skevy]

So are you just removing the idea of a higher order component here? The signature of "connect" seems the same as what was proposed in #86 (at least the shorthand one you suggested where you just pass an object of actions as the second param).

[acdlite]

@gaearon

On the other hand once you start selecting data in a tricky way, you begin to want finer control over perf optimizations, lifecycle hooks and maybe passing data down in a trickier way so it's likely you'll want a component anyway.

I really think accessing props from the owner is a much more common case that using lifecycle hooks. Props passing is the fundamental contract of React. We'll soon live in a world where pure functions are valid React components. The fewer "smart" components the better — creating class components will start to become a low-level implementation detai. Function components will be the new default. (At least that's what should happen. We'll see if the community catches on.)

However, this forces us to bind on every prop change, which is a perf hit and unfriendly to shouldComponentUpdate optimizations down the rendering chain.

This seems like a micro-optimization but okay. You could get around this by binding once if an object is passed ("bind automatically if an object is passed") but bind every time if its a function. Also if an action creator depends on a prop it's going to lead to updates further down the rendering chain, anyway.

[acdlite]

^ The reason I say it's a micro-optimization is you're rarely going to pass action creators more than one level down the component tree.

[skevy]

@acdlite also, it's easy enough to prevent constant rebinding with memoization. We had explored this a bit in redux#86

[ryancole]

I also agree with the sentiment that you'll rarely pass action creators more than one level down a component tree. This makes me question the over all usefulness of bindActionCreators. It seems to contrast the simplicity and obviousness of redux in general. It seems like it'd be clearer to just force users to pass dispatch down as a prop to every component, as described in the current readme file.

As a user, it seems like if dispatch is always needed to call an action, then maybe some way to remove the onus on the developer to pass dispatch down the tree. It'd be cool if you could just import your action methods and call them, no?

[acdlite]

@ryancole I still think bindActionCreators() (or some equivalent) is useful. You shouldn't pass dispatch() to a dumb component; you should bind it in a smart component first. E.g. your dumb components should look like this:

class CounterButton extends Component {
  render() {
    const { increment } = this.props;
    <button onClick={increment} />; 
  }
}

Rather than this:

class CounterButton extends Component {
  render() {
    const { dispatch } = this.props;
    <button onClick={dispatch(increment())} />; 
  }
}

It may seem like a trivial difference, but the first version is more separated from the implementation details of how it receives its callback. This leads to more scalable, maintainable, testable code.

It'd be cool if you could just import your action methods and call them, no?

You have to bind them somewhere. Remember, action creators in Redux are simply pure functions. Either we bind them in smart components, or we have to bind them at store creation time, in which case we'd need some sort of API for accessing the bound components. Or you could use singletons, but yuck.

[acdlite]

I like @gaearon's idea of passing action creators as the second param and auto-binding:

export default connect(
  state => ({  counter: state.counter }),
  CounterActions
)(CounterContainer);

That way we only need to bind once, and the user doesn't need to worry about bindActionCreators.

I would amend that proposal to also support a second form, where you pass a function that maps to unbound action creators:

export default connect(
  state => ({  counter: state.counter }),
  (state, props) => ({
    increment: () => CounterActions.increment(props.something)
  })
)(CounterContainer);

that way you can access the store state and props, if needed. bindActionCreators() becomes an implementation detail.

[ryancole]

@acdlite I agree with how you explained why bindActionCreators is needed, now. I wasn't thinking in terms of smart and dumb components.

Although something about the idea of a tree of components having most of the parent, outer-most components as smart components and then all the edge node components as dumb (this is what I think this smart / dumb component pattern lends itself to) kind of seems like a stink. I don't have an optimal pattern in mind, and I know smart / dumb components are a current popular pattern, but this as a pattern seems like it creates scenarios where if a dumb component is way down the tree you'll have to pass down action methods or dispatch all the way down the tree to get to it, thus making the components on the way to that component possibly carry along unneeded props just to satisfy their children. Maybe this is result of bad component tree design or something, though, on my part.

[aaronjensen]

Just thinking outside the box here, but what if bound actions were just a part of the state:

export default connect(
  state => ({ counter: state.counter,
    increment: state.actions.counter.increment(state.something) })
)(CounterContainer);

or, less radical:

export default connect(
  (state, bind) => ({ counter: state.counter, 
    increment: bind(CounterActions.increment(state.something)) })
)(CounterContainer);

It seems weird to me to do generate two separate objects that are ultimately merged into the child's props.

[acdlite]

@aaronjensen Regarding your first proposal:

First of all, the state object is not necessarily a plain JavaScript object. Redux makes no assumptions about the type of state returned from the reducer. For instance, you could use an Immutable Map.

Second of all, where are the action creators coming from? You say they're part of the state, but how did they get there? We'd need some sort of system for registering action creators at the global level.

[aaronjensen]

@acdlite Yep, that's right. We have a system for registering reducers, it didn't seem a long stretch to have one for actions. And you're right re: state not being a plain JS object of course. In that case two arguments could come along: (state, actions) or actions could have their own reducer but that seems a little odd.

Tbh, I can't think of a particularly compelling reason for it other than slight convenience at that point at the cost of required registration.

[acdlite]

How is globally registering action creators more elegant than this?

export default connect(
  state => ({  counter: state.counter }),
  CounterActions
)(CounterContainer);

EDIT: nevermind, I see that you changed your mind :)

[aaronjensen]

Yeah, that solution does look good. It didn't sit well with me at first that props were being combined in secret, but being able to handle the binding automatically makes it worth it. I also like your proposal for the second form which takes a function.

It'd probably be a bad idea to assume that all functions are action creators, yea?

export default connect(
  (state, props) => ({  
    counter: state.counter,
    increment: () => CounterActions.increment(props.something)
  })
)(CounterContainer);

If not, then it allows for everything w/ one argument:

export default connect(
  state => ({
    counter: state.counter
    ...CounterActions,
    ...OtherActions,
  }),
)(CounterContainer);

You could also make it an n-arity function and just merge all of the objects (binding all functions automatically).

This only works if it's safe to assume all functions are action creators though...

[emmenko]

we could drop <Connector> altogether and encourage people to use @connect decorator

I think it's a good idea, and we can do the same for Provider. This will simplify the API and create less confusion.

This leads to more scalable, maintainable, testable code

@acdlite I was also thinking of dropping bindActionCreators, as it's just syntactic sugar for dispatch(myAction()), but you make a valid point.

And passing the actions as a second argument of connect makes it a good API, given that the binding becomes an implementation detail of the decorator and the user doesn't care about it.

One thing I would also like to have is namespacing props. Basically instead of just spreading the actions or whatever to this.props, we can have a actions object that contains all the actions, and just pass the object to props. Same thing could be done for state. I think this is important when you start having other data in props (e.g.: router) and helps avoiding possible conflicts when merging props.
Here an example:

// Instead of a flat structure like
this.props = {
  children: Object,

  dispatch: Function,
  // state
  todos: Object,
  counter: Object,
  // actions
  addTodo: Object,
  increment: Object,
  decrement: Object,

  // router
  isTransitioning: Boolean,
  location: Object,
  params: Object,
  route: Object,
  routeParams: Object,

  // custom props
  foo: Object,
  onClick: Function
}


// we could have a much cleaner structure
this.props = {
  children: Object,

  dispatch: Function,
  // state
  state: {
    todos: Object,
    counter: Object
  },
  // actions
  actions: {
    addTodo: Object,
    increment: Object,
    decrement: Object
  },

  // router
  router: {
    isTransitioning: Boolean,
    location: Object,
    params: Object,
    route: Object,
    routeParams: Object
  },

  // custom props
  foo: Object,
  onClick: Function
}

Thoughts?

[gaearon]

I really think accessing props from the owner is a much more common case that using lifecycle hooks.

Generally connecting to Redux should be done at route handler level, and in this case you need the hooks too. Smart components close to the top level rarely receive props that somehow uniquely identify them, so even if they have props, I doubt these props are so relevant to selecting the state. For example, you won't connect <TodoItem>—you'll probably connect the whole <TodoList> in which case props are irrelevant, as you want to select the whole related slice of state.

Can you please help me by providing a few examples where props are important at the connect level?

This seems like a micro-optimization

It's really not. :-) It seems like a micro-optimization but it's the beginning of death by a thousand cuts. Redux connector sits close to the top of the application, and if at this level we're getting new functions on every prop change, no component down the chain can benefit from shouldComponentUpdate. The whole subtree is contaminated by a few changing functions.

Surely you won't see this problem at the beginning, but as soon as you get a perf bottleneck in one of your components, adding PureRenderMixin to it won't “just work” anymore because Redux rebinds these actions on every prop change. We don't want people to end up in this situation.

You could get around this by binding once if an object is passed ("bind automatically if an object is passed") but bind every time if its a function.

I can.. But then changing two seemingly equivalent signatures will have bad perf consequences for the whole rendering chain. It's too easy to make this mistake and later have no idea how to optimize your app because shouldComponentUpdate stopped helping anywhere down the chain.

On the other hand, if we force user to create a component, this won't be a problem, as they will pass the component's functions down. And the component's functions can look into props just fine.

Also if an action creator depends on a prop it's going to lead to updates further down the rendering chain, anyway.

Yes, but in a way totally manageable by shouldComponentUpdate! If unrelatedToHeavyComponents state changes too often, but <HeavyComponents> down the chain accept increment, they won't update every time unrelatedToHeavyComponents changes. On the other hand, if we go with binding on prop change, <HeavyComponents> will receive new increment every time unrelatedToHeavyComponents changes and they'll have to re-render. We don't know for sure which props are used by which action creators. I still think never binding is the easiest solution. It's not too hard to write a component, and you're in full control if you do.

^ The reason I say it's a micro-optimization is you're rarely going to pass action creators more than one level down the component tree.

Can you elaborate on that? I usually pass them several levels down (at some point they turn into dumb on* props but I still pass them down).

it's easy enough to prevent constant rebinding with memoization

Memoization only avoids “rebind on every render” in favor of “rebind on every prop change”. It's certainly better than nothing, but worse than “never rebind”.

[gaearon]

@emmenko Namespacing will also kill shouldComponentUpdate optimizations because you can't shallowly compare props anymore and nobody will write code to handle namespaces separately.

[emmenko]

@gaearon right, haven't consider this. I guess there's no much way around it then...

[gaearon]

How about we add a third parameter: merge.

Default:

export default connect(
  state => ({ counter: state.counter }),
  CounterActions,
  (state, actions, props) => ({
    ...state,
    ...actions
  })
)(Counter);

But you can also...

export default connect(
  state => ({ counter: state.counter }),
  CounterActions,
  (state, actions, props) => ({
    ...state,
    ...actions,
    increment: (...args) => actions.increment(props.counterId, ...args)
  })
)(Counter);

This gives you the same control, but any binding is explicit and performed by you.

[gaearon]

In fact as long as you don't care for lifecycle hooks you can do this:

export default connect(
  state => ({ counters: state.counters }),
  CounterActions,
  (state, actions, props) => ({
    counter: state.counters[props.counterId],
    increment: () => actions.increment(props.counterId)
  })
)(Counter);

Binding is performed by you instead of the library, so you can easily find where it happens if you have shouldComponentUpdate problems. If this begins to work bad performance-wise, you have a clear upgrade path: turn merge function into a component.

[emmenko]

So to be clear, connect would have following signature now?

function connect (state: Function, actions: Object, merge: Function)

This gives you the same control, but any binding is explicit and performed by you.

Not sure exactly what do you mean by binding though. This has nothing to do with "binding the dispatch", right?

you have a clear upgrade path: turn merge function into a component

You mean by putting a component in the middle like in your first example? Or do you mean something else?

Thanks!

[skevy]

You said above that "connecting" components using usually happens at or near the route handler level.

I wholeheartedly agree with this, and is definitely what I've experienced in general. When you bind to Redux (or really, binding to any other Flux library) too far down, then changes are difficult to trace and weird things start to happen.

However, that doesn't mean that you wouldn't want to provide the action creators deeper in the tree. That leads me to believe that maybe we can keep "connect" simple - don't worry about binding any action creators with it - and then introduce some other helper that can be a convienence to bind action creators on a component level.

I worry that connect is becoming too heavy, and originally I really liked how simple connect was to understand. It's almost self-documenting.

[gaearon]

I think we should remove provide(). It doesn't work for server rendering (store is different on each request). It also doesn't work for Redux DevTools because they need a separate provider. It locks you into bad patterns.

The only downside of <Provider> is this function-as-a-child gotcha. It will be solved in React 0.14, so I'm convinced that removing provide() is the way forward.

[mmerickel]

Late to the conversation but wanted to add an alternative I haven't seen discussed for an action api. One thing I've noticed when writing my first couple redux apps is that redux does a fantastic job of getting almost all of the coupling of my model out of my components. I wanted to continue this trend by removing any coupling to the exact actions creators I'm invoking. The idea was to bind the actions to the dispatcher at the same/similar time that I "bind" the store to the react component tree. In this way I didn't need to import and/or know exactly which action creator I was invoking. I simply use it by name just like I use the state.

@connect(
    (state, props) => ({
        todos: state.todos,
    }),
    (actions/*, dispatch? */) => ({
        addTodo: actions.addTodo,
    }),
)
class MyAppContainer extends React.Component {
    render() {
        const {addTodo, todos} = this.props;
        return (
            <div>
                <button click={addTodo}>Add Todo</button>
                <ul>{todos.map(::this.renderTodo)}</ul>
            </div>
        );
    }
}

const store = createStore(reducer);
const actions = {
    addTodo(name) {
        return {type: ADD_TODO, name};
    }
};

React.render(<Provider store={store} actions={actions}>{() => <MyAppContainer />}</Provider>, document.getElementById('app'));

This is nice because action creators tend to store most of the business logic in the app and they often need things like api utils, etc. This pre-binding is a perfect opportunity to configure your action creators with things like an apiUtils object and then that stuff doesn't need to be known about in the view components and your action creator also doesn't need to be coupled to a specific apiUtils singleton like I see in many examples.

To be clear the Provider would be responsible for binding all actions passed into it to the store. This could be separated into a StoreProvider and ActionProvider if desired, and do not need to be done at the same time but I think that may be too much. The Provider then just adds the store and actions context values allowing @connect to access them.

[ghost]

I second the idea @mmerickel describes above. I was actually going to suggest exactly the same thing since I have been binding the actionCreators at the top level and passing them down. I'd love if actions could be "selected" in the same way as slices of state in the connect decorator.

I've been staying away from using dispatch in my dumb components and instead just using pre-bound actionCreators from props, but I like that there is a very easy way in his sample above to get at the dispatcher. To me this supports both styles of working with actions, and because the api would be very similar to what is already understood for selecting state, it would reduce cognitive load required to start being productive with the react-redux bindings.

[gaearon]

@danmartinez101 @mmerickel

One of the reasons I don't want to do this is because code splitting will be harder.
People are likely to keep wanting the same structure, and will do hacks to make it work with code splitting.
The current proposal works with code splitting with no modifications.

I feel this is more opinionated than I'm willing to go. I'm happy to see this explored in alternative bindings though!

Everyone, thanks for the discussion, let's continue in #16.

[mmerickel]

@gaearon Can you clarify what you mean by code splitting? My assumption by exposing the dispatch to the action getter was that this would help if you didn't want to do <Provider actions={actions}>. Maybe exposing a binder instead of dispatch works better which seems quite similar to what your original proposals have been above.

[gaearon]

@mmerickel Code splitting = when your app's modules are loaded by demand. As result, not all actions are available from the beginning.

[mmerickel]

That's an issue with the single store as well isn't it? Presumably when you add reducers you can subscribe to that change and add actions as well, no?