fix: Run reducers with the props from the queued update #21419
Conversation
|
Comparing: 2442d98...2a092ae Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
|
It seems like this was previously acknowledged as a bug: #17953 (comment), from a duplicate issue. @eps1lon do you think it might be reasonable to ping a maintainer for a brief look? With respect to their time, I wonder if this PR was overlooked because the title doesn't suggest that it includes a fix. Obviously there's a chance that it turns out to be more subtle, but maybe it turns out to be an easy merge? |
I didn't want to waste their time but it seems like @gaearon is at least sympathetic towards test + bad fix (https://twitter.com/dan_abramov/status/1402015574780170240) so maybe you're right. |
This seems pretty important. :) we wouldn’t want to replace one set of mistakes with another set of mistakes, and to avoid this we need to understand precisely how this fix works. I haven’t looked but it would be helpful if someone dug into it and explained it. In either case, addressing this is on the todo list, and we’ll get to it before the next major release. We likely can’t fix it in a minor anyway because it’s too risky. We were planning for a wider fix (eg entirely removing the bailout mechanism for reducers specifically) but we’ll include reviewing this change as a prerequisite to this work. |
Agreed. I was just hoping for
-- https://twitter.com/dan_abramov/status/1402015574780170240 😛 |
dd404d0 to
765ae43
Compare
765ae43 to
2a092ae
Compare
* Fork dispatchAction for useState/useReducer * Remove eager bailout from forked dispatchReducerAction, update tests * Update eager reducer/state logic to handle state case only * sync reconciler fork * rename test * test cases from #15198 * comments on new test cases * comments on new test cases * test case from #21419 * minor tweak to test name to kick CI
* Fork dispatchAction for useState/useReducer * Remove eager bailout from forked dispatchReducerAction, update tests * Update eager reducer/state logic to handle state case only * sync reconciler fork * rename test * test cases from facebook#15198 * comments on new test cases * comments on new test cases * test case from facebook#21419 * minor tweak to test name to kick CI
Summary
Closes #21416
Closes #17953
I don't fully understand the fix yet but it worked so I'm trying my luck with CI 🤷♂️ and test https://twitter.com/dan_abramov/status/1402015574780170240
Test Plan