-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
Feature request for a notify middleware: Prevent notifying all listeners in certain conditions #2445
Comments
This isn't really the job of redux, as it's supposed to be small, simple, and somewhat dumb in it's implementation. It shouldn't distinguish who to notify because those listeners can use the state in a variety of ways. Basically, it can't and shouldn't know how state is used. It sounds like what you want is reselect: https://github.com/reactjs/reselect |
@timdorr thanks for the fast answer, I think I didn't correctly described the issue here. I do not implies that Redux should handle when or when not to notify listeners, but being able to receive a function that does that check for him which knows about the data structure etc. The point here is that in a complex application with multiple subscribes everywhere we have no ways to prevent a loop of dispatcher to being called and this sometimes kills the application. Additionally, some nice tools such as reduxDevTool can spam a lots of dispatch information which just crash the browser or generate so much noise that it becomes useless. I will give you an example of a usage and let me know if that makes sense for you // User can specify when he wants the notify to happens
// In this example (so it make sense) I use Immutable.js
// so I can test if there is a change in any node of the store with one super fast check
const preventUselessNotifies = (oldState, newState) => !(oldState && oldState.equals && oldState.equals(newState));
store = createStore(reducers, initState);
// This is an example of setting the middleware but could be passed directly in the createStore too
// I found it easier to add this in the API since the argument checks in createStore are implicit
store.setNotifyMiddleware(preventUselessNotifies); I know react-redux's Alternatively, if Redux still don't want to go there, It's not impossible to reuse the optimisation from react-redux as a layer replacing |
I linked an example PR to help illustrate what I meant, this is not polished, just explain better what I meant |
@zeachco : the "right" way to implement this is as a store enhancer that would override the store's For a related discussion, see #1813 . Are you actually experiencing performance issues of some kind, or is this a more theoretical concern? |
There is a measured performance impact, but I understand my case might be very specific as it's a fast paced multiplayer platform and event dispatches happen very often (much more dispatch than a traditional web app). If I understand correctly, the proposed solution is to transform my latest PR into a middleware/enhancer instead (which replace Redux completely and remove the need to have Redux in the first place since this is what the enhancer does: replacing I was hoping to have a different solution as this freeze the snapshot-ed version of I will read #1813 and probably continue using an in-house solution for that specific project. |
Uh... no, you're misunderstanding what enhancers are. A store enhancer accepts As an example, Another example is the So no, enhancers do not "remove the need for Redux", enhancers build on the Redux core to provide customized behavior. |
I didn't get that part indeed 😅 , it looks to me that the confusion comes from https://github.com/reactjs/redux/blob/master/src/applyMiddleware.js#L21 Thanks again |
For future references: For my case, it worked with minimal code in the enhancer, I didn't had to rewrite the dispatch functionnality |
Yep, told you something like that would work :) |
Hi, this is a feature request
What is the current behavior?
Every dispatch cause all listeners to be called without the possibility to prevent it.
The "hackish" to do it would be with the
enhancer
function fromcreateStore
. I don't find this clean for the following reason:It require to rewrite
createStore
completely which make me wonder why do we require Redux in the first place if it's to pass a function that replace it completely.What is the expected behavior?
I think allowing to pass a middleware would be helpfull for cases such as Redux used with Immutable.js. A middleware could verify that the state has muted and not call uselessly all the listeners if not. Other usecase could also apply such as "locking" dispatch in certain conditions for other performances reasons.
I think the goal of Redux is to notify the state changes, not to notify that a dispatch happen otherwise his role wouldn't be to hold a store a well but more a simple event bus system. Maybe I didn't get correctly the misson of Redux here so let me know if I'm in the wrong here.
The current Redux version is 3.6.0
this applies for all browsers / node environments and all OS
I currently have a working version of this which need polishing so I would provide the PR if this feature is accepted.
Thank you
The text was updated successfully, but these errors were encountered: