Find a way to cut getReducer and replaceReducer from the API

[gaearon]

They are used for hot reloading by react-redux and code splitting. However the solution feels wrong and ad-hoc to me.

Something like a store enhancer that would wrap the reducer a la devTools seems like the way forward. This way hot reloading would be opt in, but much more reliable.

Whatever the solution may be, it has to solve the following issues to be considered:
#37 #301 #340 #346 #348 reduxjs/redux-devtools#68

Ideas welcome!

[taylorhakes]

Here is my proposal. I am sure it has some issues.

Expose reducer as a simple function to Higher Order Stores.

Higher order stores would be created like this:

createStore(middleware, devtools, persistState);

Higher Order Stores would have the following signature:

function higherOrderStore({ reducer, dispatch, getState, subscribe }) {
  // wrap components

  return {
    reducer: newReducer,
    dispatch: newDispatch,
    // ...
  };
}

The create store function would have the following signature

createStore(...funcs) {
  const store = new Store();
  const finalStore = compose(...funcs, store);
  finalStore.dispatch({ action: '@@init' });
  return store;
}

This allows the store to be very flexible. @@init can be called by other components or not at all.

This is just off the top of my head. Haven't thought of all the implications of this change. It definitely would solve #376 though.

[gaearon]

Interesting, thanks. On the first sight I like it, but I haven't thought through the implications.
I'd say we'll release 1.0 as is, but this will be on our radar as the next important breaking change.

[gaearon]

@taylorhakes Can you hack up a proof of concept of this in a branch?

[wbecker]

+1

[taylorhakes]

Sure, I can do it this weekend.

[acstll]

I have one more proposal. The idea is that createStore returns itself, so you can create a copy of a store updating reducer and/or state. It's a function but of course keeps the 3 main methods getState, dispatch and subscribe.

EDIT: plus a reducer getter or the same getReducer method.

Here's how I'd implement it: https://github.com/acstll/chopped-redux/blob/next/index.js#L32-L40
You'd also have to take listeners as third argument, so you can keep them as well in new copies.

So the API would be this:

let store = createStore(reducer, initialState)

// Create a new store replacing `reducer` but keeping state and listeners.
store = store(nextReducer)

With this, I think React bindings code would be like this:

Now

  componentWillReceiveProps(nextProps) {
    const { store } = this.state;
    const { store: nextStore } = nextProps;

    if (store !== nextStore) {
      const nextReducer = nextStore.getReducer();
      store.replaceReducer(nextReducer);
    }
  }

After

  componentWillReceiveProps(nextProps) {
    const { store } = this.state;
    const { store: nextStore } = nextProps;

    if (store !== nextStore) {
      store = store(nextStore.reducer)
    }
  }

// the before EDIT `store = nextStore(null, store.getState())` would lose its original listeners

I have to say I actually don't see any problems with the getReducer() and replaceReducer() code, it's a lot more explicit, which should be good.

[taylorhakes]

@acstll What does the higher order store API look like? Wondering if it will solve this? #376

[acstll]

@taylorhakes what you get from the first createStore call is the actual store instance, there's no higher order store. :)

Wondering if it will solve this? #376

I don't think so 😁 If you need to chain-wrap the reducer you need to have it exposed in some way. Either with getReducer() or a getter or something.

From #376:

The previous example shows that compose makes the store.dispatch and reducer wrapping go in opposite directions and that is an issue.

Maybe the problem is with compose and the applyMiddleware implementation? I actually like your idea of applying middleware directly to the store instance and not creating a higher-order function. Looks like this needs a lot more discussion.

[gaearon]

See my proposal: #624, #625.

[acstll]

Brilliant 👌

@gaearon you could even expose the onNextReducer method to allow people wanting to dynamically load reducers to do it.

[gaearon]

@acstll

Do you mean expose on the store instance? Truth be told, I'd rather avoid it. My goal is to make { dispatch, getState, subscribe } the store API, as I don't think cluttering the store object from enhancers is any better than inheritance. There should be separation of store interface and the means to control enhancers.

I'll think about how to implement code splitting with the same switchReducer—it shouldn't be difficult.

[acstll]

@gaearon yes I meant that.

I'll think about how to implement code splitting with the same switchReducer

Looking forward to it. 👍

[taylorhakes]

I think this proposal is great, but it doesn't address the issue discussed here #376 (it doesn't necessarily have to). I would like to reopen that issue if this is the road we decide.

Sorry I wasn't able to implement my proposal above. I have been really busy with other things. I will try to find some time this week.

[gaearon]

@taylorhakes

I was a bit disoriented by the approach taken in localState there. It would really help if you could come up with a smaller not-working-with-current-API-but-should-have-been-working example.

[arackaf]

I've been reading this thread, and the related ones with interest. I'm new to Redux, though code splitting is definitely something I've used in the past with success, and likely will continue to do so going forward.

Can I ask why you're looking to get rid of replaceReducer? It looks like a direct, simple and effective solution to this. Are there deeper issues with it I'd need more experience with Redux to be aware of?

[gaearon]

I think I'm satisfied with removing getReducer().
It seems like any way to cut replaceReducer() is more awkward than leaving it in.

[Markus-ipse]

@gaearon Sorry for commenting on a closed issue, but I ended up here from issue #37, about code-splitting in large apps, which I'm trying to do now, but that issue and this one is closed but I'm not entirely sure that the code-splitting issue is solved or is it?
If there is a way to handle code-splitting is there any docs, examples or issues to point me in the right direction?

[gaearon]

@hummlas The current solution is to use store.replaceReducer(). For example, if you combine reducers from a bunch of them, when you load a new one, you want to combine them again and replace the store's reducer with the new combined one.

[Markus-ipse]

@gaearon Wow, thanks for the quick reply! I thought I saw somewhere that store.replaceReducer was deprecated, but anyhow, after conferring with a colleague we decided that it might be easier (for us at least) to always have all reducers in the main bundle and and let the route-specific bundles only contain components, etc. :)

[gaearon]

I thought I saw somewhere that store.replaceReducer was deprecated

It was un-deprecated later :-).
Yeah, depends on your use case. If reducers are small, it's no big deal.

[eriknyk]

HI, any benevolent soul that can provide an example of its implementation?

[gaearon]

http://stackoverflow.com/questions/32968016/how-to-dynamically-load-reducers-for-code-splitting-in-a-redux-application

[arackaf]

Hi @gaearon - is it expected that after a call to replaceReducer, a default call to dispatch would be made with an undefined state, so the new default can be set? It doesn't seem to happen.

As it is, the workaround is trivial

store.replaceReducer(reducer);
store.dispatch(initialize());

with

case INITIALIZE:
return initialState;

I'm just curious if I'm missing something.

[gaearon]

I'm pretty sure it happens: https://github.com/rackt/redux/blob/92f44c4b902934f08834e545416ae0fb9814d30c/src/createStore.js#L136.

[gaearon]

Of course, you'll only have the initial state for parts of the tree that didn't have any state associated with them. This is useful when you're doing hot reloading, or when you're adding a new code-split reducer under an existing reducer tree.

If you want to clean up all the state when running replaceReducer(), you should fire a custom action that does that on the root reducer level.

[arackaf]

That clarifies things perfectly - thank you much! I'm still quite new to react and redux.

[maximoleinyk]

Pretty simple example for those who may be interested.