fix: prevent Suspense fallback flash during App Router client navigation (#639)#647
Open
southpolesteve wants to merge 3 commits intomainfrom
Open
fix: prevent Suspense fallback flash during App Router client navigation (#639)#647southpolesteve wants to merge 3 commits intomainfrom
southpolesteve wants to merge 3 commits intomainfrom
Conversation
…ion (#639) Replace flushSync + root.render() with a NavigationRoot wrapper component that holds RSC content in React state. Navigation updates go through startTransition(() => setState()), which tells React to keep the current committed UI visible until all Suspense boundaries in the new tree have resolved, then commit atomically. flushSync forced an immediate synchronous commit including unresolved Suspense fallbacks, producing a visible double-flash where content outside a Suspense boundary (e.g. a heading) updated before content inside it was ready. The NavigationRoot approach matches Next.js App Router behavior. Also fixes the same bug for server action re-renders, which were bypassing NavigationRoot via root.render() and destroying the wrapper. Adds regression tests and a /suspense-nav-test fixture page that reproduces the exact partial-flash scenario from the issue report.
commit: |
|
Collaborator
Author
|
/bigbonk review |
Contributor
|
@southpolesteve Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
The NavigationRoot fix committed Suspense fallbacks (short content) before the resolved content, so restoreScrollPosition was calling scrollTo on a page too short to reach the saved position. The fix retries scrollTo each rAF until the scroll target is reached or a 1.5s deadline passes, which lets it succeed once the full content (e.g. ItemList) is in the DOM. Also adds history.scrollRestoration = 'manual' so the browser's auto scroll restoration does not override our manual restoration, and adds useTransition to NavigationRoot so navigateRsc awaits the transition commit before resolving (so __VINEXT_RSC_PENDING__ carries full timing). Adds item detail fixture route and a scroll restoration E2E test.
…ommits Per analysis by @NathanDrake2406 in issue #639: without buffering, createFromFetch processes a streaming response and creates React elements with lazy chunks for async server components. React commits the Suspense fallback first (short content), then the resolved content in a second pass - causing the flash. Buffering the full response body before createFromFetch ensures the flight parser processes all rows in one synchronous pass. React renders and commits the complete content in a single pass with no Suspense suspension. The known remaining limitation (CSS animation replay on navigation) is a deeper architectural issue requiring segment-level diffing, tracked separately and referenced in the PR.
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
1 participant
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.
Summary
flushSync(() => root.render(...))with aNavigationRootwrapper component that holds RSC content in React statestartTransition(() => setState(newContent))insideNavigationRoot, which tells React to keep the current committed UI visible until all Suspense boundaries in the new tree resolve, then commit atomicallyNavigationRootviaroot.render()and destroying the wrapperRoot cause
flushSyncforced a synchronous commit of the incoming RSC tree including any unresolved Suspense fallbacks. This produced a visible double-flash: content outside a Suspense boundary (e.g. a heading) updated immediately while content inside it still showed the loading fallback, before finally resolving.Why
startTransition(() => root.render(...))wasn't enoughroot.render()replaces the entire fiber tree, so React has no previously committed content to hold onto. New Suspense boundaries in the replacement tree can still flash their fallbacks.startTransition(() => setState(...))inside a persistent component is the correct approach: React keeps that component's current committed output visible until the new render (including all Suspense boundaries) is fully ready, matching Next.js App Router behavior.Changes
packages/vinext/src/server/app-browser-entry.ts—NavigationRootcomponent +_scheduleRscUpdatewiringtests/fixtures/app-basic/app/suspense-nav-test/— new fixture reproducing the exact partial-flash scenario from the issuetests/e2e/app-router/navigation-flows.spec.ts— regression tests for issue App Router client navigation double-flashes Suspense fallbacks and janky back-button scroll restoration #639tests/e2e/app-router/loading.spec.ts— updated: removed incorrect assertion (showingloading.tsxduring navigation to a new route is correct Next.js behavior)Closes #639