From 16950098ca123fb2743e56fd6cb9248761aa29cf Mon Sep 17 00:00:00 2001 From: Steve Faulkner Date: Sun, 22 Mar 2026 07:52:20 -0500 Subject: [PATCH 1/3] fix: prevent Suspense fallback flash during App Router client navigation (#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. --- .../vinext/src/server/app-browser-entry.ts | 72 ++++++++++++++-- tests/e2e/app-router/loading.spec.ts | 38 ++++++++ tests/e2e/app-router/navigation-flows.spec.ts | 86 +++++++++++++++++++ tests/fixtures/app-basic/app/page.tsx | 3 + .../app/suspense-nav-test/FilterToggle.tsx | 16 ++++ .../app-basic/app/suspense-nav-test/page.tsx | 43 ++++++++++ 6 files changed, 251 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/app-basic/app/suspense-nav-test/FilterToggle.tsx create mode 100644 tests/fixtures/app-basic/app/suspense-nav-test/page.tsx diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 27589b830..3be469b4e 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -9,7 +9,7 @@ import { encodeReply, setServerCallback, } from "@vitejs/plugin-rsc/browser"; -import { flushSync } from "react-dom"; +import { createElement, Fragment, startTransition, useState } from "react"; import { hydrateRoot } from "react-dom/client"; import { PREFETCH_CACHE_TTL, @@ -120,6 +120,42 @@ async function readInitialRscStream(): Promise> { return rscResponse.body; } +// --------------------------------------------------------------------------- +// NavigationRoot — persistent wrapper for concurrent RSC navigation +// +// startTransition(() => root.render(newTree)) does NOT correctly prevent +// Suspense fallbacks from flashing during navigation. When root.render() +// replaces the entire fiber tree, React has no "previously committed content" +// to hold onto — new Suspense boundaries in the incoming tree may flash their +// fallbacks before their content resolves. +// +// The correct fix: hold RSC content in React state inside a persistent +// component. startTransition(() => setState(newContent)) inside a persistent +// component tells React to keep that component's current committed output +// visible until the new render (including all Suspense boundaries) is fully +// resolved, then commit atomically. This is how Next.js App Router prevents +// loading-boundary flashes during client navigation. +// --------------------------------------------------------------------------- + +// Stable setter assigned by NavigationRoot on its first render. +// React guarantees that the setState dispatcher returned by useState is stable, +// so this closure is effectively constant for the lifetime of the page. +let _scheduleRscUpdate: ((content: ReactNode) => void) | null = null; + +function NavigationRoot({ initial }: { initial: ReactNode }) { + const [content, setContent] = useState(initial); + + _scheduleRscUpdate = (newContent: ReactNode) => { + startTransition(() => { + setContent(newContent); + }); + }; + + // Fragment wrapper: renders content directly with no extra DOM nodes so + // the hydration output is identical to the server-rendered HTML. + return createElement(Fragment, null, content); +} + function registerServerActionCallback(): void { setServerCallback(async (id, args) => { const temporaryReferences = createTemporaryReferenceSet(); @@ -162,7 +198,13 @@ function registerServerActionCallback(): void { }); if (isServerActionResult(result)) { - getReactRoot().render(result.root); + // Route through NavigationRoot so root.render() doesn't destroy the wrapper. + // Server action results are fully resolved so startTransition commits promptly. + if (_scheduleRscUpdate) { + _scheduleRscUpdate(result.root); + } else { + getReactRoot().render(result.root); + } if (result.returnValue) { if (!result.returnValue.ok) throw result.returnValue.data; return result.returnValue.data; @@ -170,7 +212,11 @@ function registerServerActionCallback(): void { return undefined; } - getReactRoot().render(result as ReactNode); + if (_scheduleRscUpdate) { + _scheduleRscUpdate(result as ReactNode); + } else { + getReactRoot().render(result as ReactNode); + } return result; }); } @@ -181,9 +227,12 @@ async function main(): Promise { const rscStream = await readInitialRscStream(); const root = await createFromReadableStream(rscStream); + // Hydrate with NavigationRoot so subsequent navigations go through setState + // transitions rather than root.render() replacements. NavigationRoot's + // Fragment wrapper renders identical DOM to the SSR HTML — no hydration mismatch. reactRoot = hydrateRoot( document, - root as ReactNode, + createElement(NavigationRoot, { initial: root as ReactNode }), import.meta.env.DEV ? { onCaughtError() {} } : undefined, ); @@ -252,9 +301,17 @@ async function main(): Promise { } const rscPayload = await createFromFetch(Promise.resolve(navResponse)); - flushSync(() => { - getReactRoot().render(rscPayload as ReactNode); - }); + // Route update through NavigationRoot's setState so React treats this + // as a concurrent transition: old UI stays visible until all Suspense + // boundaries in the new RSC tree resolve, then commits atomically. + if (_scheduleRscUpdate) { + _scheduleRscUpdate(rscPayload as ReactNode); + } else { + // Fallback: shouldn't occur after hydration completes. + startTransition(() => { + getReactRoot().render(rscPayload as ReactNode); + }); + } } catch (error) { console.error("[vinext] RSC navigation error:", error); window.location.href = href; @@ -278,6 +335,7 @@ async function main(): Promise { const rscPayload = await createFromFetch( fetch(toRscUrl(window.location.pathname + window.location.search)), ); + // HMR bypasses NavigationRoot for immediate code-change feedback. getReactRoot().render(rscPayload as ReactNode); } catch (error) { console.error("[vinext] RSC HMR error:", error); diff --git a/tests/e2e/app-router/loading.spec.ts b/tests/e2e/app-router/loading.spec.ts index 64b417fc4..2c469afd4 100644 --- a/tests/e2e/app-router/loading.spec.ts +++ b/tests/e2e/app-router/loading.spec.ts @@ -2,6 +2,13 @@ import { test, expect } from "@playwright/test"; const BASE = "http://localhost:4174"; +async function waitForHydration(page: import("@playwright/test").Page) { + await expect(async () => { + const ready = await page.evaluate(() => !!(window as any).__VINEXT_RSC_ROOT__); + expect(ready).toBe(true); + }).toPass({ timeout: 10_000 }); +} + test.describe("Loading boundaries (loading.tsx)", () => { test("slow page eventually renders content", async ({ page }) => { await page.goto(`${BASE}/slow`); @@ -45,4 +52,35 @@ test.describe("Loading boundaries (loading.tsx)", () => { timeout: 10_000, }); }); + + /** + * Client navigation to a slow page works end-to-end. + * + * Note: showing loading.tsx during client navigation to a new route is + * correct Next.js behavior — React shows the Suspense boundary for a + * route segment that has never been rendered before. The regression we + * guard against (issue #639) is *partial* UI flash where content *outside* + * a Suspense boundary updates before content *inside* it is ready. That + * scenario is covered in navigation-flows.spec.ts. + */ + test("client navigation to slow page completes without full reload", async ({ page }) => { + await page.goto(`${BASE}/`); + await expect(page.locator("h1")).toHaveText("Welcome to App Router"); + await waitForHydration(page); + + await page.evaluate(() => { + (window as any).__NAV_MARKER__ = true; + }); + + // Client-navigate — do NOT use page.goto() (that is a hard/SSR navigation). + await page.click('[data-testid="slow-link"]'); + + // The slow page has a 2s server delay; wait up to 10s total. + await expect(page.locator("h1")).toHaveText("Slow Page", { timeout: 10_000 }); + await expect(page.locator("main > p")).toHaveText("This page has a loading boundary."); + + // Confirm no full-page reload occurred. + const marker = await page.evaluate(() => (window as any).__NAV_MARKER__); + expect(marker).toBe(true); + }); }); diff --git a/tests/e2e/app-router/navigation-flows.spec.ts b/tests/e2e/app-router/navigation-flows.spec.ts index ce9f6eab6..55d11a287 100644 --- a/tests/e2e/app-router/navigation-flows.spec.ts +++ b/tests/e2e/app-router/navigation-flows.spec.ts @@ -235,3 +235,89 @@ test.describe("App Router navigation flows", () => { await expect(page.locator('[data-testid="client-search-q"]')).toHaveText("test-param"); }); }); + +// --------------------------------------------------------------------------- +// Regression tests for issue #639: Suspense partial-flash during navigation +// +// The bug: flushSync() caused React to synchronously commit a partial new tree +// where content *outside* a Suspense boundary (e.g. a heading) had updated to +// the new value but content *inside* the Suspense boundary was still showing +// its fallback. This "partial flash" made it look like the heading was wrong. +// +// The fix: NavigationRoot holds RSC content in React state and uses +// startTransition to schedule updates. React's transition semantics keep the +// *entire* previous committed tree visible until all Suspense boundaries in the +// new tree have resolved, then commits atomically. +// +// We test this with /suspense-nav-test: a page where the heading is *outside* +// Suspense but the list is *inside* Suspense. Both depend on the same ?filter +// param. A partial flash would produce heading="All Items" + list fallback at +// the same time. The correct behavior is: old heading stays until list is ready, +// then both update together. +// --------------------------------------------------------------------------- +test.describe("issue #639 — Suspense partial-flash regression", () => { + test("heading and list update together during same-page navigation", async ({ page }) => { + // Start with filter applied so Suspense has resolved content. + await page.goto(`${BASE}/suspense-nav-test?filter=active`); + await waitForHydration(page); + + await expect(page.locator("#page-heading")).toHaveText("Filtered: active"); + await expect(page.locator("#item-list")).toBeVisible(); + + // Instrument: record if we ever observe the NEW heading ("All Items") at + // the same moment as the list loading fallback — that is the partial flash. + await page.evaluate(() => { + (window as any).__PARTIAL_FLASH_SEEN__ = false; + const observer = new MutationObserver(() => { + const heading = document.getElementById("page-heading"); + const loadingFallback = document.getElementById("list-loading"); + // Partial flash: new heading visible while list is still loading + if (heading?.textContent === "All Items" && loadingFallback) { + (window as any).__PARTIAL_FLASH_SEEN__ = true; + } + }); + observer.observe(document.body, { + childList: true, + subtree: true, + characterData: true, + }); + (window as any).__PARTIAL_FLASH_OBSERVER__ = observer; + }); + + // Navigate back to the unfiltered state (clears the filter). + await page.click("#toggle-filter"); + + // Wait for the full transition to complete. + await expect(page.locator("#page-heading")).toHaveText("All Items", { timeout: 10_000 }); + await expect(page.locator("#item-list")).toBeVisible(); + + await page.evaluate(() => { + (window as any).__PARTIAL_FLASH_OBSERVER__?.disconnect(); + }); + + const partialFlashSeen = await page.evaluate(() => (window as any).__PARTIAL_FLASH_SEEN__); + expect(partialFlashSeen).toBe(false); + }); + + test("back/forward navigation completes correctly on suspense page", async ({ page }) => { + await page.goto(`${BASE}/suspense-nav-test`); + await waitForHydration(page); + await expect(page.locator("#page-heading")).toHaveText("All Items"); + + // Navigate to filtered state + await page.click("#toggle-filter"); + await expect(page.locator("#page-heading")).toHaveText("Filtered: active", { + timeout: 10_000, + }); + + // Navigate back + await page.goBack(); + await expect(page.locator("#page-heading")).toHaveText("All Items", { timeout: 10_000 }); + + // Navigate forward + await page.goForward(); + await expect(page.locator("#page-heading")).toHaveText("Filtered: active", { + timeout: 10_000, + }); + }); +}); diff --git a/tests/fixtures/app-basic/app/page.tsx b/tests/fixtures/app-basic/app/page.tsx index f0d4978f3..1a93cc232 100644 --- a/tests/fixtures/app-basic/app/page.tsx +++ b/tests/fixtures/app-basic/app/page.tsx @@ -9,6 +9,9 @@ export default function HomePage() { Go to About Go to Blog Go to Dashboard + + Go to Slow + Go to Headers Test diff --git a/tests/fixtures/app-basic/app/suspense-nav-test/FilterToggle.tsx b/tests/fixtures/app-basic/app/suspense-nav-test/FilterToggle.tsx new file mode 100644 index 000000000..93ffb59fd --- /dev/null +++ b/tests/fixtures/app-basic/app/suspense-nav-test/FilterToggle.tsx @@ -0,0 +1,16 @@ +"use client"; +import { useRouter } from "next/navigation"; + +export function FilterToggle({ current }: { current: string }) { + const router = useRouter(); + return ( + + ); +} diff --git a/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx b/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx new file mode 100644 index 000000000..f336d643a --- /dev/null +++ b/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx @@ -0,0 +1,43 @@ +import { Suspense } from "react"; +import { FilterToggle } from "./FilterToggle"; + +async function ItemList({ filter }: { filter: string }) { + // Artificial delay: long enough for Playwright to catch intermediate state. + await new Promise((r) => setTimeout(r, 400)); + const items = filter + ? [{ id: 1, text: `Filtered: ${filter}` }] + : [ + { id: 1, text: "Item A" }, + { id: 2, text: "Item B" }, + { id: 3, text: "Item C" }, + ]; + return ( +
    + {items.map((i) => ( +
  • {i.text}
    • + ))} +
+ ); +} + +export default async function SuspenseNavTestPage({ + searchParams, +}: { + searchParams: Promise<{ filter?: string }>; +}) { + const { filter = "" } = await searchParams; + + return ( +
+ {/* Heading is OUTSIDE the Suspense boundary. + With flushSync, it updates before the list is ready — partial flash. + With startTransition (NavigationRoot), it only updates once the list + is also ready — both change together (issue #639). */} +

{filter ? `Filtered: ${filter}` : "All Items"}

+ + Loading items...
}> + + + + ); +} From f0d522ab6d47854fc8945b5c2ac1632b477fb125 Mon Sep 17 00:00:00 2001 From: Steve Faulkner Date: Sun, 22 Mar 2026 08:37:55 -0500 Subject: [PATCH 2/3] fix: also fix back-button scroll restoration jank (#639) 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. --- .../vinext/src/server/app-browser-entry.ts | 62 ++++++++++++++----- packages/vinext/src/shims/navigation.ts | 32 +++++++--- tests/e2e/app-router/navigation-flows.spec.ts | 46 ++++++++++++++ .../app/suspense-nav-test/item/[id]/page.tsx | 28 +++++++++ .../app-basic/app/suspense-nav-test/page.tsx | 18 +++--- 5 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 tests/fixtures/app-basic/app/suspense-nav-test/item/[id]/page.tsx diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 3be469b4e..b18fc4e65 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -9,7 +9,15 @@ import { encodeReply, setServerCallback, } from "@vitejs/plugin-rsc/browser"; -import { createElement, Fragment, startTransition, useState } from "react"; +import { + createElement, + Fragment, + startTransition, + useEffect, + useRef, + useState, + useTransition, +} from "react"; import { hydrateRoot } from "react-dom/client"; import { PREFETCH_CACHE_TTL, @@ -137,17 +145,40 @@ async function readInitialRscStream(): Promise> { // loading-boundary flashes during client navigation. // --------------------------------------------------------------------------- -// Stable setter assigned by NavigationRoot on its first render. -// React guarantees that the setState dispatcher returned by useState is stable, -// so this closure is effectively constant for the lifetime of the page. -let _scheduleRscUpdate: ((content: ReactNode) => void) | null = null; +// Exposed by NavigationRoot. Returns a Promise that resolves once the +// transition commits to the DOM — callers can await it to know when the +// new content is actually visible (used by navigateRsc so that +// __VINEXT_RSC_PENDING__ resolves at the right time for scroll restoration). +let _scheduleRscUpdate: ((content: ReactNode) => Promise) | null = null; function NavigationRoot({ initial }: { initial: ReactNode }) { const [content, setContent] = useState(initial); + // useTransition gives us isPending so we know exactly when a transition + // has committed. We use that to resolve the promise returned to navigateRsc, + // which in turn lets __VINEXT_RSC_PENDING__ resolve at the right moment for + // scroll restoration (restoreScrollPosition in navigation.ts awaits it). + const [isPending, startTransitionHook] = useTransition(); + const resolveRef = useRef<(() => void) | null>(null); + + // After each commit: if a transition just completed, resolve the waiter. + // useEffect runs after the browser has painted the committed tree, which + // is the correct point for scroll restoration to apply. + useEffect(() => { + if (!isPending && resolveRef.current) { + const resolve = resolveRef.current; + resolveRef.current = null; + resolve(); + } + }); - _scheduleRscUpdate = (newContent: ReactNode) => { - startTransition(() => { - setContent(newContent); + _scheduleRscUpdate = (newContent: ReactNode): Promise => { + return new Promise((resolve) => { + // Overwrite any prior pending resolve — if a second navigation fires + // before the first commits, the first waiter is abandoned (acceptable). + resolveRef.current = resolve; + startTransitionHook(() => { + setContent(newContent); + }); }); }; @@ -201,7 +232,7 @@ function registerServerActionCallback(): void { // Route through NavigationRoot so root.render() doesn't destroy the wrapper. // Server action results are fully resolved so startTransition commits promptly. if (_scheduleRscUpdate) { - _scheduleRscUpdate(result.root); + void _scheduleRscUpdate(result.root); } else { getReactRoot().render(result.root); } @@ -213,7 +244,7 @@ function registerServerActionCallback(): void { } if (_scheduleRscUpdate) { - _scheduleRscUpdate(result as ReactNode); + void _scheduleRscUpdate(result as ReactNode); } else { getReactRoot().render(result as ReactNode); } @@ -301,11 +332,14 @@ async function main(): Promise { } const rscPayload = await createFromFetch(Promise.resolve(navResponse)); - // Route update through NavigationRoot's setState so React treats this - // as a concurrent transition: old UI stays visible until all Suspense - // boundaries in the new RSC tree resolve, then commits atomically. + // Await the transition commit so that this function only resolves once + // the new content is actually painted. __VINEXT_RSC_PENDING__ (set by + // the popstate handler to this promise) is what restoreScrollPosition + // waits on before applying saved scroll — resolving too early would + // cause scroll to be set on the old (outgoing) content with the wrong + // layout, producing the back-button jank described in issue #639. if (_scheduleRscUpdate) { - _scheduleRscUpdate(rscPayload as ReactNode); + await _scheduleRscUpdate(rscPayload as ReactNode); } else { // Fallback: shouldn't occur after hydration completes. startTransition(() => { diff --git a/packages/vinext/src/shims/navigation.ts b/packages/vinext/src/shims/navigation.ts index 2e23b5b8c..06605baf2 100644 --- a/packages/vinext/src/shims/navigation.ts +++ b/packages/vinext/src/shims/navigation.ts @@ -495,18 +495,28 @@ function restoreScrollPosition(state: unknown): void { void Promise.resolve().then(() => { const pending: Promise | null = window.__VINEXT_RSC_PENDING__ ?? null; + // Retry scrollTo until the page is tall enough to reach the target. + // + // The first attempt may land short when a Suspense boundary was not + // previously in the committed tree (e.g. navigating back to a page with + // an async list). React commits the Suspense fallback first (short + // content, max-scroll = 0), then commits the resolved content in a + // second pass. Each rAF retry gives newly-committed content a chance to + // be in the DOM so the scroll can reach its target. + const scrollWithRetry = (deadline: number) => { + window.scrollTo(x, y); + if (Math.abs(window.scrollY - y) > 2 && Date.now() < deadline) { + requestAnimationFrame(() => scrollWithRetry(deadline)); + } + }; + if (pending) { - // Wait for the RSC navigation to finish rendering, then scroll. void pending.then(() => { - requestAnimationFrame(() => { - window.scrollTo(x, y); - }); + requestAnimationFrame(() => scrollWithRetry(Date.now() + 1500)); }); } else { // No RSC navigation in flight (Pages Router or already settled). - requestAnimationFrame(() => { - window.scrollTo(x, y); - }); + requestAnimationFrame(() => scrollWithRetry(Date.now() + 1500)); } }); } @@ -866,6 +876,14 @@ export function unauthorized(): never { // Listen for popstate on the client if (!isServer) { + // Disable the browser's built-in scroll restoration so it doesn't override + // our manual restoration. The browser's 'auto' mode fires asynchronously and + // can reset scroll to 0 after our rAF-based restoration already set the + // correct position. We own scroll restoration entirely via restoreScrollPosition. + if ("scrollRestoration" in history) { + history.scrollRestoration = "manual"; + } + window.addEventListener("popstate", (event) => { notifyListeners(); // Restore scroll position for back/forward navigation diff --git a/tests/e2e/app-router/navigation-flows.spec.ts b/tests/e2e/app-router/navigation-flows.spec.ts index 55d11a287..a402f33a5 100644 --- a/tests/e2e/app-router/navigation-flows.spec.ts +++ b/tests/e2e/app-router/navigation-flows.spec.ts @@ -320,4 +320,50 @@ test.describe("issue #639 — Suspense partial-flash regression", () => { timeout: 10_000, }); }); + + /** + * Regression test for issue #639 — back-button scroll restoration jank. + * + * The bug: flushSync committed an incomplete tree (with Suspense fallbacks) + * before scroll restoration ran. Because fallbacks have different heights + * than resolved content, the restored scroll position landed in the wrong + * place, then jumped again when content resolved — visible stutter. + * + * The fix (NavigationRoot): the old content stays visible during the + * transition, so scroll is set on stable layout. The new content then + * commits at that scroll position. No layout shift, no stutter. + */ + test("back-button restores scroll position after navigating to item detail", async ({ page }) => { + await page.goto(`${BASE}/suspense-nav-test`); + await waitForHydration(page); + + // Wait for list to load (async, 400ms delay) + await expect(page.locator("#item-list")).toBeVisible({ timeout: 10_000 }); + + // Scroll well down the list so there is a meaningful position to restore. + await page.evaluate(() => window.scrollTo(0, 600)); + const scrollBefore = await page.evaluate(() => window.scrollY); + expect(scrollBefore).toBeGreaterThan(0); + + // Navigate via evaluate click (no Playwright auto-scroll). + await page.evaluate(() => { + const link = document.querySelector("#item-list a:first-child") as HTMLAnchorElement; + link?.click(); + }); + await expect(page.locator("#item-heading")).toBeVisible({ timeout: 10_000 }); + + // Press back. + await page.goBack(); + await expect(page.locator("#item-list")).toBeVisible({ timeout: 10_000 }); + + // Allow time for the retry-based scroll restoration to succeed. + // The list has a 400ms load delay; restoration retries until the page + // is tall enough to reach the target position. + await page.waitForTimeout(800); + + const scrollAfter = await page.evaluate(() => window.scrollY); + + // Scroll should be close to where we left off (within 50px tolerance). + expect(Math.abs(scrollAfter - scrollBefore)).toBeLessThan(50); + }); }); diff --git a/tests/fixtures/app-basic/app/suspense-nav-test/item/[id]/page.tsx b/tests/fixtures/app-basic/app/suspense-nav-test/item/[id]/page.tsx new file mode 100644 index 000000000..48ff55344 --- /dev/null +++ b/tests/fixtures/app-basic/app/suspense-nav-test/item/[id]/page.tsx @@ -0,0 +1,28 @@ +import Link from "next/link"; + +// Async detail page — simulates a DB fetch that takes a moment. +// This is important for the scroll-restoration test: back-button scroll +// should only fire once the destination content is ready, not on a partial +// tree with wrong layout heights. +async function ItemDetail({ id }: { id: string }) { + await new Promise((r) => setTimeout(r, 200)); + return ( +
+

Details for item {id}

+

This content loads asynchronously (200ms).

+
+ ); +} + +export default async function ItemPage({ params }: { params: Promise<{ id: string }> }) { + const { id } = await params; + return ( +
+

Item {id}

+ + ← Back to list + + +
+ ); +} diff --git a/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx b/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx index f336d643a..f1f27ca3b 100644 --- a/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx +++ b/tests/fixtures/app-basic/app/suspense-nav-test/page.tsx @@ -1,20 +1,22 @@ +import Link from "next/link"; import { Suspense } from "react"; import { FilterToggle } from "./FilterToggle"; +const ALL_ITEMS = Array.from({ length: 20 }, (_, i) => ({ + id: i + 1, + text: `Item ${String.fromCharCode(65 + i)}`, // Item A … Item T +})); + async function ItemList({ filter }: { filter: string }) { // Artificial delay: long enough for Playwright to catch intermediate state. await new Promise((r) => setTimeout(r, 400)); - const items = filter - ? [{ id: 1, text: `Filtered: ${filter}` }] - : [ - { id: 1, text: "Item A" }, - { id: 2, text: "Item B" }, - { id: 3, text: "Item C" }, - ]; + const items = filter ? [{ id: 1, text: `Filtered: ${filter}` }] : ALL_ITEMS; return (
    {items.map((i) => ( -
  • {i.text}
    • +
  • + {i.text} +
    • ))}
); From e952f8006a1155147f9dee4cc85e4576d53d12f7 Mon Sep 17 00:00:00 2001 From: Steve Faulkner Date: Sun, 22 Mar 2026 11:42:53 -0500 Subject: [PATCH 3/3] fix: buffer full RSC response before rendering to eliminate partial commits 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. --- .../vinext/src/server/app-browser-entry.ts | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index b18fc4e65..e755f8301 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -331,13 +331,35 @@ async function main(): Promise { setClientParams({}); } - const rscPayload = await createFromFetch(Promise.resolve(navResponse)); - // Await the transition commit so that this function only resolves once - // the new content is actually painted. __VINEXT_RSC_PENDING__ (set by - // the popstate handler to this promise) is what restoreScrollPosition - // waits on before applying saved scroll — resolving too early would - // cause scroll to be set on the old (outgoing) content with the wrong - // layout, producing the back-button jank described in issue #639. + // Buffer the full RSC response body before passing it to createFromFetch. + // + // Without buffering, createFromFetch receives a streaming response and + // creates React elements with "lazy" chunks for async server components. + // When React renders these, Suspense boundaries suspend and React commits + // the fallback first (short content), then the resolved content in a + // second pass. This causes two problems: + // 1. Suspense fallback flash — the fallback is briefly visible between + // the shell commit and the resolved-content commit. + // 2. Scroll restoration jank — restoreScrollPosition sees a page that + // is too short to reach the saved scroll position on the first attempt. + // + // By fully buffering the response, all RSC rows are available before the + // flight parser runs. createFromFetch returns a fully-resolved React tree + // with no lazy chunks. React renders and commits the complete content in + // a single pass — no Suspense suspension, no partial commits, no flash. + // + // The tradeoff: the old content stays visible for the full server response + // time (e.g. 400ms for a slow async component). This matches Next.js App + // Router's "keep old UI visible until new content is ready" contract. + const responseBody = await navResponse.arrayBuffer(); + const bufferedResponse = new Response(responseBody, { + headers: navResponse.headers, + status: navResponse.status, + statusText: navResponse.statusText, + }); + const rscPayload = await createFromFetch(Promise.resolve(bufferedResponse)); + // Await the transition commit so __VINEXT_RSC_PENDING__ resolves only + // after the new content is painted (needed for scroll restoration). if (_scheduleRscUpdate) { await _scheduleRscUpdate(rscPayload as ReactNode); } else {