[Flight] Keep an element pending while a referenced row is resolving - #37542
Merged
unstubbable merged 3 commits intoSep 8, 2026
Merged
Conversation
A row that is still parsing can hand its partially built value to references that were registered on it during that parse. `initializeModelChunk` fulfilled every such listener, on the assumption that all of them are cyclic references back into the parsing row. Only some are. A listener from a nested parse that is not part of a cycle belongs to a handler that does not wait on the parsing row, so fulfilling it early completes that handler with an object that still has references outstanding. When that handler owns an element, `initializeElement` runs on incomplete props. In DEV the props are frozen, so the write that arrives later throws `Cannot assign to read only property`, and `rejectReference` escalates the error into the rows that wait on the element, up to the root. Only the debug tree can produce this shape. The RSC stream writes element props inline, while the debug channel outlines a props object that an element shares with its own componentInfo into a separate row, and that row can still wait on a client module. `resolveBlockedCycle` already tells a cyclic reference from any other, but it returned `null` for mid-parse listeners because `handler.chunk` was assigned after the drain loop. This change assigns it before the loop and classifies each listener. A reference whose handler is transitively waiting on the parsing row is a genuine cycle and receives the value now, because neither side can complete before the other. Every other listener is queued back on the parsing row and fulfilled when that row completes, like any reference into a blocked row. A row whose own parse fails used to hand the partial value to its mid-parse listeners before it threw. It now errors through `triggerErrorOnChunk`, which rejects them the same way a reference into any other errored row is rejected. The `if (handler.errored) throw` after the loop is removed. It also caught a rejection during the loop, which now reaches `triggerErrorOnChunk` on its own because `handler.chunk` is set. One behaviour changes beyond the reported bug. When `initializeDebugChunk` errors a chunk before `parseModel` runs, the old code set `INITIALIZED` over that status if the model had no pending references, and left it `ERRORED` otherwise. The chunk now stays `ERRORED` in both cases. That is what the `triggerErrorOnChunk` call in `initializeDebugChunk` intends, and the TODO above the `parseModel` call already notes that the chunk can be `ERRORED` there. PR react#37398 deferred `Object.freeze(element.props)` until the outstanding references have resolved. That removes the exception but not the cause: the element is still initialized on an incomplete object and is visible through `_debugInfo` with a `null` placeholder until the late write lands. With the early release fixed, the freeze needs no change. **Alternatives Considered** - Deferring every listener whose handler is not the parsing row's own deadlocks `foo ↔ bar` in `can deduped outlined references inside promises`. One side of a genuine cycle has to accept the partial object. - Holding an element back while its props row is `BLOCKED` breaks `should handle deduped props of re-used elements in fragments`, where the row is blocked on an unrelated module and the props object itself is complete. - A per-object count of pending writes plus a reverse `dependents` edge works, but adds a second dependency graph next to `deps` and special-cases elements. Fixes react#37361 Closes react#37398
github-actions
Bot
added
the
React Core Team
unstubbable
marked this pull request as ready for review
September 8, 2026 09:32
eps1lon
reviewed
Sep 8, 2026
| response: Response, | ||
| chunk: BlockedChunk<T>, | ||
| value: T, | ||
| reason: any, |
Collaborator
There was a problem hiding this comment.
Can we type this to match InitializedChunk['reason']? The callsite already has a comment saying why it's sound but that Flow would consider it unsound is hidden.
Collaborator
Author
There was a problem hiding this comment.
Done, together with the split below so the call site is actually checked: handler.reason narrows to null | FlightStreamController after the errored guard, and Flow verifies it against the parameter.
Comment on lines
+1720
to
+1722
| if (chunk === null || chunk.status !== BLOCKED) { | ||
| return; | ||
| } |
Collaborator
There was a problem hiding this comment.
Probably an existing type-issue but according to types InitializationHandler can only have a BlockedChunk so why do we need to check status here at runtime?
Collaborator
Author
There was a problem hiding this comment.
This is a real runtime condition. BlockedChunk is the chunk's type when the handler recorded it, but the object is mutated in place: it can have been errored by rejectReference (any of the handler's references failing) or completed by a cyclic reference during the drain loop before the last deps-- arrives here. Both inline blocks this replaces had the same check on main. I made it explicit with the SomeChunk cast that
wakeChunkIfInitialized uses, and a comment.
Comment on lines
+1723
to
+1724
| // For a stream chunk, `handler.reason` holds its controller. | ||
| initializeBlockedChunk(response, chunk, handler.value, handler.reason); |
Collaborator
There was a problem hiding this comment.
But don't we have a BlockedChunk here?
Collaborator
Author
There was a problem hiding this comment.
Right, it is a BlockedChunk; the comment meant the chunk it becomes. Reworded: handler.reason is the controller of a stream whose chunk was blocked on its debug info (set where resolveStream waits on
initializeDebugChunk), null otherwise, and it becomes the initialized chunk's reason either way.
| // `initializeModelChunk` handles that case when the parse ends. | ||
| function initializeChunkIfUnblocked( | ||
| response: Response, | ||
| handler: InitializationHandler, |
Collaborator
There was a problem hiding this comment.
Should we split the InitializationHandler type up into an errored and "normal" version to narrow down its reason?
Collaborator
Author
There was a problem hiding this comment.
Yes, done. The four places that flip a handler cast to ErroredInitializationHandler before writing, like the chunk transitions do. The refinements fall out: handler.reason is mixed inside the errored branches and null | FlightStreamController after the guard in initializeChunkIfUnblocked, so the typed initializeBlockedChunk parameter is now checked.
Comment on lines
+2671
to
+2675
| const getDebugInfoWithProps = | ||
| require('internal-test-utils').getDebugInfo.bind(null, { | ||
| ignoreProps: false, | ||
| useFixedTime: true, | ||
| }); |
Collaborator
There was a problem hiding this comment.
getDebugInfo should really be hoisted into the beforeEach to match our existing test patterns. getDebugInfoWithProps can also be inlined since it's only used once.
Collaborator
Author
There was a problem hiding this comment.
Done. getDebugInfo is bound in beforeEach and the test calls it directly. Hoisting shifts the hardcoded source locations in the with real timers tests by 27 lines, so those are updated too; while at it I moved the two helpers up next to the existing ones instead of leaving them at the bottom, which was only there to avoid that shift.
Comment on lines
+2693
to
+2696
| // The props row still waits on the module, so the element must still be a | ||
| // lazy. Without the fix it is already initialized here, with the null | ||
| // placeholder where `ClientModule` belongs. | ||
| expect(asyncServerElement.$$typeof).toBe(Symbol.for('react.lazy')); |
Collaborator
There was a problem hiding this comment.
Is this important? Isn't it sufficient to let the test run without throwing because props are frozen?
Collaborator
Author
There was a problem hiding this comment.
The throw is only the DEV symptom. A test that just checks the response resolves also passes with #37398's deferred freeze, which keeps handing the element the incomplete props; DevTools then reads the null placeholder. This assertion is the one that pins the actual invariant (the element stays a lazy until its props row completes) independent of the freeze; the assertions after the module resolves cover the no-throw outcome. I reworded the comment to say that.
The test now checks the frozen-props `TypeError` (the symptom) and the early resolution of the debug-tree element (the cause), in that order. It records the payload status of the lazy that wraps the element before the module resolves and asserts it after the behaviour assertions, so on `main` the first failure is the `TypeError`. With the client from element having resolved before its props row did. Without that assertion the test passes with react#37398 and cannot tell the fix from the alternative it supersedes. The `$$typeof` check is gone; the element is read through `_init(_payload)`, as `ReactFlightDOMEdge-test.js` does, which is also what DevTools reads.
RioNoir
approved these changes
Sep 8, 2026
eps1lon
approved these changes
Sep 8, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Sep 8, 2026
…37542) A row that is still parsing can hand its partially built value to references that were registered on it during that parse. `initializeModelChunk` fulfilled every such listener, on the assumption that all of them are cyclic references back into the parsing row. Only some are. A listener from a nested parse that is not part of a cycle belongs to a handler that does not wait on the parsing row, so fulfilling it early completes that handler with an object that still has references outstanding. When that handler owns an element, `initializeElement` runs on incomplete props. In DEV the props are frozen, so the write that arrives later throws `Cannot assign to read only property`, and `rejectReference` escalates the error into the rows that wait on the element, up to the root. Only the debug tree can produce this shape. The RSC stream writes element props inline, while the debug channel outlines a props object that an element shares with its own componentInfo into a separate row, and that row can still wait on a client module. `resolveBlockedCycle` already tells a cyclic reference from any other, but it returned `null` for mid-parse listeners because `handler.chunk` was assigned after the drain loop. This change assigns it before the loop and classifies each listener. A reference whose handler is transitively waiting on the parsing row is a genuine cycle and receives the value now, because neither side can complete before the other. Every other listener is queued back on the parsing row and fulfilled when that row completes, like any reference into a blocked row. A row whose own parse fails used to hand the partial value to its mid-parse listeners before it threw. It now errors through `triggerErrorOnChunk`, which rejects them the same way a reference into any other errored row is rejected. The `if (handler.errored) throw` after the loop is removed. It also caught a rejection during the loop, which now reaches `triggerErrorOnChunk` on its own because `handler.chunk` is set. One behaviour changes beyond the reported bug. When `initializeDebugChunk` errors a chunk before `parseModel` runs, the old code set `INITIALIZED` over that status if the model had no pending references, and left it `ERRORED` otherwise. The chunk now stays `ERRORED` in both cases. That is what the `triggerErrorOnChunk` call in `initializeDebugChunk` intends, and the TODO above the `parseModel` call already notes that the chunk can be `ERRORED` there. PR #37398 deferred `Object.freeze(element.props)` until the outstanding references have resolved. That removes the exception but not the cause: the element is still initialized on an incomplete object and is visible through `_debugInfo` with a `null` placeholder until the late write lands. With the early release fixed, the freeze needs no change. **Alternatives Considered** - Deferring every listener whose handler is not the parsing row's own deadlocks `foo ↔ bar` in `can deduped outlined references inside promises`. One side of a genuine cycle has to accept the partial object. - Holding an element back while its props row is `BLOCKED` breaks `should handle deduped props of re-used elements in fragments`, where the row is blocked on an unrelated module and the props object itself is complete. - A per-object count of pending writes plus a reverse `dependents` edge works, but adds a second dependency graph next to `deps` and special-cases elements. Fixes #37361 Closes #37398 DiffTrain build for [6c0e104](6c0e104)
github-actions Bot
pushed a commit
to code/lib-react
that referenced
this pull request
Sep 8, 2026
…eact#37542) A row that is still parsing can hand its partially built value to references that were registered on it during that parse. `initializeModelChunk` fulfilled every such listener, on the assumption that all of them are cyclic references back into the parsing row. Only some are. A listener from a nested parse that is not part of a cycle belongs to a handler that does not wait on the parsing row, so fulfilling it early completes that handler with an object that still has references outstanding. When that handler owns an element, `initializeElement` runs on incomplete props. In DEV the props are frozen, so the write that arrives later throws `Cannot assign to read only property`, and `rejectReference` escalates the error into the rows that wait on the element, up to the root. Only the debug tree can produce this shape. The RSC stream writes element props inline, while the debug channel outlines a props object that an element shares with its own componentInfo into a separate row, and that row can still wait on a client module. `resolveBlockedCycle` already tells a cyclic reference from any other, but it returned `null` for mid-parse listeners because `handler.chunk` was assigned after the drain loop. This change assigns it before the loop and classifies each listener. A reference whose handler is transitively waiting on the parsing row is a genuine cycle and receives the value now, because neither side can complete before the other. Every other listener is queued back on the parsing row and fulfilled when that row completes, like any reference into a blocked row. A row whose own parse fails used to hand the partial value to its mid-parse listeners before it threw. It now errors through `triggerErrorOnChunk`, which rejects them the same way a reference into any other errored row is rejected. The `if (handler.errored) throw` after the loop is removed. It also caught a rejection during the loop, which now reaches `triggerErrorOnChunk` on its own because `handler.chunk` is set. One behaviour changes beyond the reported bug. When `initializeDebugChunk` errors a chunk before `parseModel` runs, the old code set `INITIALIZED` over that status if the model had no pending references, and left it `ERRORED` otherwise. The chunk now stays `ERRORED` in both cases. That is what the `triggerErrorOnChunk` call in `initializeDebugChunk` intends, and the TODO above the `parseModel` call already notes that the chunk can be `ERRORED` there. PR react#37398 deferred `Object.freeze(element.props)` until the outstanding references have resolved. That removes the exception but not the cause: the element is still initialized on an incomplete object and is visible through `_debugInfo` with a `null` placeholder until the late write lands. With the early release fixed, the freeze needs no change. **Alternatives Considered** - Deferring every listener whose handler is not the parsing row's own deadlocks `foo ↔ bar` in `can deduped outlined references inside promises`. One side of a genuine cycle has to accept the partial object. - Holding an element back while its props row is `BLOCKED` breaks `should handle deduped props of re-used elements in fragments`, where the row is blocked on an unrelated module and the props object itself is complete. - A per-object count of pending writes plus a reverse `dependents` edge works, but adds a second dependency graph next to `deps` and special-cases elements. Fixes react#37361 Closes react#37398 DiffTrain build for [6c0e104](react@6c0e104)
github-actions Bot
pushed a commit
to srpatcha/react
that referenced
this pull request
Sep 9, 2026
…eact#37542) A row that is still parsing can hand its partially built value to references that were registered on it during that parse. `initializeModelChunk` fulfilled every such listener, on the assumption that all of them are cyclic references back into the parsing row. Only some are. A listener from a nested parse that is not part of a cycle belongs to a handler that does not wait on the parsing row, so fulfilling it early completes that handler with an object that still has references outstanding. When that handler owns an element, `initializeElement` runs on incomplete props. In DEV the props are frozen, so the write that arrives later throws `Cannot assign to read only property`, and `rejectReference` escalates the error into the rows that wait on the element, up to the root. Only the debug tree can produce this shape. The RSC stream writes element props inline, while the debug channel outlines a props object that an element shares with its own componentInfo into a separate row, and that row can still wait on a client module. `resolveBlockedCycle` already tells a cyclic reference from any other, but it returned `null` for mid-parse listeners because `handler.chunk` was assigned after the drain loop. This change assigns it before the loop and classifies each listener. A reference whose handler is transitively waiting on the parsing row is a genuine cycle and receives the value now, because neither side can complete before the other. Every other listener is queued back on the parsing row and fulfilled when that row completes, like any reference into a blocked row. A row whose own parse fails used to hand the partial value to its mid-parse listeners before it threw. It now errors through `triggerErrorOnChunk`, which rejects them the same way a reference into any other errored row is rejected. The `if (handler.errored) throw` after the loop is removed. It also caught a rejection during the loop, which now reaches `triggerErrorOnChunk` on its own because `handler.chunk` is set. One behaviour changes beyond the reported bug. When `initializeDebugChunk` errors a chunk before `parseModel` runs, the old code set `INITIALIZED` over that status if the model had no pending references, and left it `ERRORED` otherwise. The chunk now stays `ERRORED` in both cases. That is what the `triggerErrorOnChunk` call in `initializeDebugChunk` intends, and the TODO above the `parseModel` call already notes that the chunk can be `ERRORED` there. PR react#37398 deferred `Object.freeze(element.props)` until the outstanding references have resolved. That removes the exception but not the cause: the element is still initialized on an incomplete object and is visible through `_debugInfo` with a `null` placeholder until the late write lands. With the early release fixed, the freeze needs no change. **Alternatives Considered** - Deferring every listener whose handler is not the parsing row's own deadlocks `foo ↔ bar` in `can deduped outlined references inside promises`. One side of a genuine cycle has to accept the partial object. - Holding an element back while its props row is `BLOCKED` breaks `should handle deduped props of re-used elements in fragments`, where the row is blocked on an unrelated module and the props object itself is complete. - A per-object count of pending writes plus a reverse `dependents` edge works, but adds a second dependency graph next to `deps` and special-cases elements. Fixes react#37361 Closes react#37398 DiffTrain build for [6c0e104](react@6c0e104)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
A row that is still parsing can hand its partially built value to references that were registered on it during that parse.
initializeModelChunkfulfilled every such listener, on the assumption that all of them are cyclic references back into the parsing row. Only some are. A listener from a nested parse that is not part of a cycle belongs to a handler that does not wait on the parsing row, so fulfilling it early completes that handler with an object that still has references outstanding.When that handler owns an element,
initializeElementruns on incomplete props. In DEV the props are frozen, so the write that arrives later throwsCannot assign to read only property, andrejectReferenceescalates the error into the rows that wait on the element, up to the root. Only the debug tree can produce this shape. The RSC stream writes element props inline, while the debug channel outlines a props object that an element shares with its own componentInfo into a separate row, and that row can still wait on a client module.resolveBlockedCyclealready tells a cyclic reference from any other, but it returnednullfor mid-parse listeners becausehandler.chunkwas assigned after the drain loop. This change assigns it before the loop and classifies each listener. A reference whose handler is transitively waiting on the parsing row is a genuine cycle and receives the value now, because neither side can complete before the other. Every other listener is queued back on the parsing row and fulfilled when that row completes, like any reference into a blocked row.A row whose own parse fails used to hand the partial value to its mid-parse listeners before it threw. It now errors through
triggerErrorOnChunk, which rejects them the same way a reference into any other errored row is rejected. Theif (handler.errored) throwafter the loop is removed. It also caught a rejection during the loop, which now reachestriggerErrorOnChunkon its own becausehandler.chunkis set.One behaviour changes beyond the reported bug. When
initializeDebugChunkerrors a chunk beforeparseModelruns, the old code setINITIALIZEDover that status if the model had no pending references, and left itERROREDotherwise. The chunk now staysERROREDin both cases. That is what thetriggerErrorOnChunkcall ininitializeDebugChunkintends, and the TODO above theparseModelcall already notes that the chunk can beERROREDthere.PR #37398 deferred
Object.freeze(element.props)until the outstanding references have resolved. That removes the exception but not the cause: the element is still initialized on an incomplete object and is visible through_debugInfowith anullplaceholder until the late write lands. With the early release fixed, the freeze needs no change.Alternatives Considered
foo ↔ barincan deduped outlined references inside promises. One side of a genuine cycle has to accept the partial object.BLOCKEDbreaksshould handle deduped props of re-used elements in fragments, where the row is blocked on an unrelated module and the props object itself is complete.dependentsedge works, but adds a second dependency graph next todepsand special-cases elements.Fixes #37361
Closes #37398