Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replaceReducer safely #2017

Closed
davidnormo opened this issue Oct 5, 2016 · 8 comments
Closed

replaceReducer safely #2017

davidnormo opened this issue Oct 5, 2016 · 8 comments

Comments

@davidnormo
Copy link

I am thinking of using store.replaceReducer to swap reducers so that an action type can target a particular reducer. This pattern would help with reusing action types on different pages of an application.

For example:

  1. on /pageA we have reducer A and the following state tree: { A: { ... } }
  2. MY_ACTION is dispatched and handled by A and produces a new state A.
  3. User goes to /pageB, we replaces reducer A with reducer B and end up with { A: { ... }, B: { ... } }
  4. MY_ACTION is dispatched that B handles and updates state B but not state A.

The problem with the above approach is that Redux does not currently provide any safety while replacing a reducer. For example the following scenario is a problem:

fetch().then(dispatch(action));

store.replaceReducer(B);

// dispatch happens by might not have been intended for reducer B - a race condition!

Is there something I'm missing here? Or would you be willing to consider a PR to make this possible?

@markerikson
Copy link
Contributor

I'm not really clear on what the actual issue or concern is here. Can you clarify?

@davidnormo
Copy link
Author

Sure, sorry I'll try to clarify.

It seems to me that replaceReducer has no safety against race conditions from async actions. So the following scenario has a race condition:

  1. A thunk is dispatched, it dispatches an action to say a fetch has started, and then calls fetch()
  2. For whatever reason the reducer is replaced with a new reducer
  3. The response from the server is returned and the thunk dispatches another action to say the server has responded.
  4. Problem: The new reducer has been called with the action but the thunk intended for the original reducer alone to be receive it.

Safety from this kind of scenario is helpful if you want to use replaceReducer to swap reducers, for example, when you want a separate reducer for each page of your application. I understand this isn't the usual approach to target separate reducers.

Does that make more sense?
Thanks

@markerikson
Copy link
Contributor

Hmm. Honestly, my take on this is that it's up to you as a user to handle that sort of thing. Redux's core is all about synchronous operations. Dispatching is synchronous, getting the state is synchronous, subscriptions are synchronous, and swapping out the reducer function is synchronous. Any async behavior is outside the core store itself (usually layered on top using applyMiddleware or similar).

While replaceReducer is fairly commonly used for adding/remove slice reducers and rebuilding the root reducer, I can't say I've ever heard of anyone swapping out the entire root reducer to handle different pages. I suppose it's feasible, but certainly a rare use case.

I don't see any changes that should be made to Redux to support this. If you want to handle synchronization between async calls and a root reducer that should be handling them, I think it's up to you to implement that yourself.

And, fwiw, I'd also suggest looking at other approaches besides swapping out the entire root reducer. I'm really not seeing any benefit from that.

@naw
Copy link
Contributor

naw commented Oct 5, 2016

I suppose it's feasible, but certainly a rare use case.

I agree with @markerikson. This is feasible (and I've considered it myself), but definitely not a common pattern.

It's not clear to me how redux could solve this problem in the general case. In particular, this kind of "safety" is not necessarily desirable for every use of replaceReducer, so at the very least, it would have to be configurable.

That said, the fundamental issue isn't really specific to replaceReducer --- any time you have asynchronous code (like a thunk) dispatching actions, it's quite feasible the action is no longer relevant (i.e. you might have moved on to another page), and your application has to deal with this. In the general sense, the application has to distinguish between what's relevant and what's not, which often means the actions have to be a little richer in providing context that the reducers can analyze.

Given any arbitrary problem, there are a few questions to consider:

  1. Can you imagine a solution?
  2. Can you imagine a solution that exists solely in user land?
  3. Would the solution be better somehow if it existed inside the redux library itself?

Perhaps redux can accommodate your solution via something a lot more generic (like a hook in a certain place or something).

Or would you be willing to consider a PR to make this possible?

Personally I don't see a lot of potential here because I'm having trouble imagining what such a PR would look like, but if you've got a clever idea, I think it would be fine for you to share it, at least at the conceptual level.

@naw
Copy link
Contributor

naw commented Oct 5, 2016

Also, I think it's important to remember that redux is a fairly low-level tool. If you want specific high-level functionality, it probably belongs in user land (or in a complimentary library). If redux is standing in the way somehow (i.e. by not exposing necessary data or necessary hooks), it's worth considering where/how redux can cooperate.

@markerikson
Copy link
Contributor

Also, I think it's important to remember that redux is a fairly low-level tool. If you want specific high-level functionality, it probably belongs in user land (or in a complimentary library).

Absolutely. See #1813 for a relevant discussion. The genericness of store enhancers and the store.subscribe method have resulted in a wide array of interesting addons. For example, check out the Store category of my Redux addons catalog. By not prescribing a specific way to deal with subscriptions, it's allowed others to go implement numerous interesting approaches themselves.

@davidnormo
Copy link
Author

@markerikson @naw Thanks for your feedback, appreciate you taking the time to write a bit more.

As you've said, my proposal is a solution to the pagination problem i.e. how to manage different reducers that handle different data from the server but handle the same/similar actions. I've looked at the real world example whose approach is to have unique action types that are passed to actions which are created on the fly.

This approach was helpful but we've found it doesn't scale well when the number of actions increases. In our case, we have ~51 action types most of which are duplicated just to target the reducer for that page to handle a toggle, or some button that puts the application in a certain state. The application is medium sizes (~36k lines of code), but judging from requirements, the application could double in size in the next few months.

We've tried automatically applying a unique prefix to action types. So MY_ACTION becomes PAGE1_MY_ACTION but found that makes the application less maintainable and introduces more magic as we have to apply the prefixing to action creators and reducers.

I just wanted to give a bit of context to the reason why I'm exploring new methods. Thanks again for your help, if you know of anything that might help, please let me know.

@markerikson
Copy link
Contributor

One slight variation to the "prefixed actions" approach is to only define one set of action types, but include extra metadata in each action object that the varied reducers can look at, such as: {type : "MY_ACTION", data : {...}, meta : {page : 1} }.

Couple more links you may want to look at. First, my Redux addons catalog does have a ton of utilities and libraries listed, including libs that help put per-component state into your Redux store, auto-generating prefixed actions and reducers, and other approaches to reusing reducer logic.

Also, I just added a Structuring Reducers section to the Redux docs, which discusses some various approaches to things like reusing reducer logic.

Finally, I usually hang out in the Reactiflux chat channels most evenings. If you'd like to discuss this in more detail, feel free to drop by and ping me. The invite link is at http://www.reactiflux.com .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants