[Not for merge] 16.6.2 release #14207
Closed
Conversation
This file contains hidden or 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
…ook#14182) * BUG: ReactPartialRenderer / New Context polutes mutable global state The new context API stores the provided values on the shared context instance. When used in a synchronous context, this is not an issue. However when used in an concurrent context this can cause a "push provider" from one react render to have an effect on an unrelated concurrent react render. I've encountered this bug in production when using renderToNodeStream, which asks ReactPartialRenderer for bytes up to a high water mark before yielding. If two Node Streams are created and read from in parallel, the state of one can polute the other. I wrote a failing test to illustrate the conditions under which this happens. I'm also concerned that the experimental concurrent/async React rendering on the client could suffer from the same issue. * Use unique thread ID for each partial render to access Context This first adds an allocator that keeps track of a unique ThreadID index for each currently executing partial renderer. IDs are not just growing but are reused as streams are destroyed. This ensures that IDs are kept nice and compact. This lets us use an "array" for each Context object to store the current values. The look up for these are fast because they're just looking up an offset in a tightly packed "array". I don't use an actual Array object to store the values. Instead, I rely on that VMs (notably V8) treat storage of numeric index property access as a separate "elements" allocation. This lets us avoid an extra indirection. However, we must ensure that these arrays are not holey to preserve this feature. To do that I store the _threadCount on each context (effectively it takes the place of the .length property on an array). This lets us first validate that the context has enough slots before we access the slot. If not, we fill in the slots with the default value.
|
React: size: 🔺+0.1%, gzip: 🔺+0.1% Details of bundled changes.Comparing: d5e1bf0...0c756fb react
react-dom
Generated by 🚫 dangerJS |
|
16.6.3 is out so I'm going to close this PR~ |
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.
Suspense-related patches and proper support for context in the server renderer.
Includes everything up to b98adb6, then I cherry-picked
b545546 and 6c22b6c as well.
Opening this PR as a reference. Not for merge.
Test plan: