fix(core): use latest children & options after async app.init()#650
Open
mathedu4all wants to merge 1 commit into
Open
fix(core): use latest children & options after async app.init()#650mathedu4all wants to merge 1 commit into
mathedu4all wants to merge 1 commit into
Conversation
createRoot's render() is async and awaits app.init() on the first call. If a second render() arrives during the init window (e.g. a parent re-render from a ResizeObserver), it skips init, applies its newer options and commits its newer children. But when the first call's await resolves, it resumes with the stale children and applicationOptions from its own closure, reverting both the app state (e.g. canvas size) and the committed fiber tree to the older values. Track the latest children and options in shared closure variables and, after init completes, use those instead of the stale arguments. When no race occurs the values are identical, so behaviour is unchanged; in the race case React bails out on the repeated children by referential equality, so there is no extra render cost. Adds a unit test that gates Application.prototype.init behind a controllable promise, interleaves a second render(), and asserts the final committed children and applied options reflect the latest call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Problem
createRoot'srender()isasyncandawaitsapp.init()on the first call. If a secondrender()arrives while the first is still awaiting init (e.g. the parent re-renders due to a resize/layout change), the second call seesisInitialising === true, skips the init branch, applies its newerapplicationOptionsand commits its newer children — correct so far.But when the first call's
await app.init()finally resolves, it continues using the stalechildrenandapplicationOptionscaptured in its own closure: it re-applies the old options to the app (potentially reverting canvaswidth/height, etc.) and callsreconciler.updateContainer(...)with the old tree. This reverts both the app state and the fiber to the older values.We hit this on hard page refresh (not on SPA navigation): surrounding UI finishes loading and shrinks the container within the
app.init()window, so aResizeObserver-driven second render lands mid-init. The result is children rendered with the pre-resize tree/size — in our app, coordinate-bound objects snap to the screen origin instead of their correct positions.Root cause
The first
renderinvocation closes over both itschildrenandapplicationOptionsarguments and re-uses them after theawait, unaware that a later invocation already pushed newer values into the fiber/app.Fix
Track the most recent
childrenandapplicationOptionsin closure variables shared acrossrendercalls, and afterawait app.init()completes, use those latest values instead of the stale closure arguments.Why this is safe
applicationOptionsapply loop and the finalupdateContainerare untouched.updateContaineris called a second time with the children the concurrent render already committed — React bails out on referential equality, so there's no extra render cost.Test
Adds a unit test in
test/unit/core/createRoot.test.tsthat reproduces the race by gatingApplication.prototype.initbehind a controllable promise, issuing a secondrender()during the init window, then resolving init. It asserts the final committed children and applied options both reflect the second render. The test fails without the fix (the resumed first render overwrites with stale'A') and passes with it.Verified locally:
vitest run unit→ all unit tests pass;xs lint→ no new errors.