Initialize update queue object on mount #17560
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e6eb323
to
f2d73ef
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 7768843:
|
Details of bundled changes.Comparing: e039e69...7768843 react-dom
react-native-renderer
react-test-renderer
react-noop-renderer
react-art
react-reconciler
ReactDOM: size: 0.0%, gzip: 0.0% Size changes (experimental) |
Fixes a bug related to rebasing updates. Once an update has committed, it should never un-commit, even if interrupted by a higher priority update. The fix includes a refactor of how update queues work. This commit is a combination of two PRs: - facebook#17483 by @sebmarkbage refactors the hook update queue - facebook#17510 by @acdlite refactors the class and root update queue Landing one without the other would cause state updates to sometimes be inconsistent across components, so I've combined them into a single commit in case they need to be reverted. Co-authored-by: Sebastian Markbåge <[email protected]> Co-authored-by: Andrew Clark <[email protected]>
Instead of lazily initializing update queue objects on the first update, class and host root queues are created on mount. This simplifies the logic for appending new updates and matches what we do for hooks.
Details of bundled changes.Comparing: e039e69...7768843 react-reconciler
react-noop-renderer
react-dom
react-art
react-test-renderer
react-native-renderer
ReactDOM: size: -0.5%, gzip: -0.1% Size changes (stable) |
f2d73ef
to
7768843
Compare
|
I don't think this should be on |
|
That makes sense but I didn't want to deal with the ramifications of other stuff that reads from it. Like DevTools. At least right now. |
|
Basically I'm trying to keep the scope of this "bugfix" somewhat narrow. Can do a fuller refactor later. |
sebmarkbage
approved these changes
Dec 10, 2019
nicksp
reviewed
Dec 10, 2019
| ' '.repeat(depth + 1) + '~', | ||
| '[' + update.expirationTime + ']', | ||
| ); | ||
| } while (update !== null && update !== first); |
There was a problem hiding this comment.
Can update be changed while inside the loop? Do we need to additionally check for not null being inside if (update !== null)?
There was a problem hiding this comment.
Good catch. It's missing a update = update.next line at the end of the loop. Honestly I don't think anyone actually uses this method anymore, so I'll probably just remove it.
There was a problem hiding this comment.
Yeah, I believe removal would be safe here and in the block below.
| ' '.repeat(depth + 1) + '~', | ||
| '[' + pendingUpdate.expirationTime + ']', | ||
| ); | ||
| } while (pendingUpdate !== null && pendingUpdate !== firstPending); |
There was a problem hiding this comment.
Same question whether pendingUpdate needs to be checked for ! == null when already inside if check
Merged
This was referenced Mar 6, 2020
This was referenced Sep 25, 2024
This was referenced Sep 28, 2024
This was referenced Sep 29, 2024
This was referenced Oct 4, 2024
This was referenced Oct 5, 2024
This was referenced Oct 12, 2024
This was referenced Oct 25, 2024
This was referenced Oct 26, 2024
This was referenced Oct 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to b617db3
Instead of lazily initializing update queue objects on the first update, class and host root queues are created on mount. This simplifies the logic for appending new updates and matches what we do for hooks.