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

useReducer - eagerReducer optimization discussion/questions #15088

Closed
Andarist opened this issue Mar 12, 2019 · 2 comments · Fixed by #22445
Closed

useReducer - eagerReducer optimization discussion/questions #15088

Andarist opened this issue Mar 12, 2019 · 2 comments · Fixed by #22445

Comments

@Andarist
Copy link
Contributor

I'd like to continue the discussion started by me under a recent blog post by Dan as encouraged by Dan 😉 gaearon/overreacted.io@99bfdca#r32694433

Just to summarize what I've stumbled upon when experimenting with useReducer after reading that hoisted & declared in render reducers are treated differently (I've wanted to explore how they are handled by React):

  1. I have no idea how to reenter eagerReducer calculation after first scheduled work (& after render gets fully processed & committed). This might very well be just me not understanding how fibers work - but currently I'm confused by this. It doesn't enter this code branch because fiber.expirationTime is not 0 (NoWork) and the work gets scheduled right away. Any pointers regarding this? Is this valid? Might this be a bug somewhere?
  2. The logic around reducer bailouts is somewhat iffy for me - maybe it's just a matter of mentioning those in the documentation:
  • not every reducer update can bailout from rendering. If the action queue gets processed in the render phase then it's just not possible, we are already rendering after all.
  • action queue gets processed in the render phase for reducers declared inside render, this means that any new state computation might depend on the "fresh" props. This is not the case for the bailout mechanism though - it's only possible to perform a bailout when dispatching an action (so when we do not have access to the fresh props). Should this restriction be mentioned in the docs? IMHO this behaviour is inconsistent - for the greater good, so it's acceptable but I think it should be documented because it's slightly inconsistent & people might trip over this.

Note I'm happy to provide documentation changes if needed, I'd like to discuss those points first to get a better understanding of things.

Andarist referenced this issue in gaearon/overreacted.io Mar 12, 2019
@gaearon
Copy link
Collaborator

gaearon commented Mar 12, 2019

I have no idea how to reenter eagerReducer calculation after first scheduled work

I don't know what that means. What are you trying to do? Preferably describe it from user perspective than impl details.

The logic around reducer bailouts is somewhat iffy for me

Why does it matter to you? These heuristics can change over time. They shouldn't affect how your reducer works.

If the action queue gets processed in the render phase then it's just not possible, we are already rendering after all.

We are rendering that component but the bailout still determines whether we'll recurse on children or not. That's the important part.

@Andarist
Copy link
Contributor Author

We are rendering that component but the bailout still determines whether we'll recurse on children or not. That's the important part.

I stand corrected! This indeed is working thanks for this line. It's a new concept (for me) that render might get called but it might not result in children reconciliation and I got confused.

I don't know what that means. What are you trying to do? Preferably describe it from user perspective than impl details.

From the user perspective, everything works OK(-ish). The whole reason I've started testing this was to know the implementation details - not to depend on them or exploit them but to get a better understanding of how things work.

Therefore I'd like to focus on some of the implementation details here.

After investigating this more I must correct what I have stated about the observed behaviour - eagerState/eagerReducer computation doesn't "stop working" after first scheduled work, but rather it doesn't work when processing next dispatched action after processing an action which has resulted in work to be scheduled. To rephrase - this code path gets entered only when dispatching an action after mount or after bailing out. Let me demonstrate with a sandbox - https://codesandbox.io/s/zwwyw2z38p . Just click INCREMENT and BAILOUT after that - you can observe that BAILOUT action gets processed inside render although it seems to me that the bailout could have take place when dispatching an action - like it happens if we click BAILOUT right away.

This seems like we could optimize this because we could not schedule the render at all in this bailout case, but again - I might not understand something about how Fiber works. Maybe this is needed and I'm just confused about it.

The more interesting demo I can provide is this - https://codesandbox.io/s/735onv7mv6 . Just click BAILOUT 3x times and then click INCREMENT. What we can observe is that BAILOUT actions are preserved in the action queue and are being processed when we finally call render. And this is somewhat strange because those actions were previously "ignored" because we decided to bail out based on them.

This is IMHO strange because initially those 3 actions were computing new state using "old" reducer, but at the render time they might use the "new" reducer - the one seeing the newest props & stuff. And if the bail out logic inside reducer depends i.e. on some props (when I try to think how this could happen in real life I'm imagining props.disabled) then we might compute new state based on completely stale actions (does the action queue ever expire? 🤔 ) using the "new" updated reducer.

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