test: E2E verification for layout persistence flat payload pipeline#752
Draft
NathanDrake2406 wants to merge 21 commits intocloudflare:mainfrom
Draft
test: E2E verification for layout persistence flat payload pipeline#752NathanDrake2406 wants to merge 21 commits intocloudflare:mainfrom
NathanDrake2406 wants to merge 21 commits intocloudflare:mainfrom
Conversation
- Fix stale closure on readBrowserRouterState by using a useRef updated synchronously during render instead of a closure captured in useLayoutEffect. External callers (navigate, server actions, HMR) now always read the current router state. - Restore GlobalErrorBoundary wrapping that was dropped when switching from buildPageElement to buildAppPageElements. Apps with app/global-error.tsx now get their global error boundary back. - Add exhaustive default case to routerReducer so new action types produce a compile error and a runtime throw instead of silent undefined. - Remove dead code: createRouteNodeSnapshot, AppRouteNodeSnapshot, AppRouteNodeValue were defined but never imported. - Remove deprecated buildAppPageRouteElement and its test — no production callers remain after the flat payload cutover. - Short-circuit normalizeAppElements when no slot keys need rewriting to avoid unnecessary allocation on every payload. - Align test data in error boundary RSC payload test (matchedParams slug: "post" -> "missing" to match requestUrl /posts/missing).
createFromReadableStream() returns a React thenable whose .then() returns undefined (not a Promise). Chaining .then(normalizeAppElements) broke SSR by assigning undefined to flightRoot. Fix: call use() on the raw thenable, then normalize synchronously after resolution. Also widen renderAppPageLifecycle element type to accept flat map payloads.
The SSR entry always expects a flat Record<string, ReactNode> with __route and __rootLayout metadata from the RSC stream. Three paths were still producing bare ReactNode payloads: 1. renderAppPageBoundaryElementResponse only created the flat map for isRscRequest=true, but HTML requests also flow through RSC→SSR 2. buildPageElements "no default export" early return 3. Server action "Page not found" fallback All three now produce the flat keyed element map, fixing 17 test failures across 404/not-found, forbidden/unauthorized, error boundary, production build, rewrite, and encoded-slash paths.
- Update renderElementToStream mock to extract the route element from the flat map before rendering to HTML (mirrors real SSR entry flow) - Update entry template snapshots for the buildPageElements changes
createFromReadableStream() returns a React Flight thenable whose .then() returns undefined instead of a new Promise. The browser entry's normalizeAppElementsPromise chained .then() on this raw thenable, producing undefined — which crashed use() during hydration with "An unsupported type was passed to use(): undefined". Wrapping in Promise.resolve() first converts the Flight thenable into a real Promise, making .then() chains work correctly. The same fix was already applied to the SSR entry in 5395efc but was missed in the browser entry.
React 19.2.4's use(Promise) during hydration triggers "async Client Component" because native Promises lack React's internal .status property (set only by Flight thenables). When use() encounters a Promise without .status, it suspends — which React interprets as the component being async, causing a fatal error. Fix: store resolved AppElements directly in ElementsContext and router state instead of Promise<AppElements>. The navigation async flow (createPendingNavigationCommit) awaits the Promise before dispatching, so React state never holds a Promise. - ElementsContext: Promise<AppElements> → AppElements - AppRouterState.elements: Promise<AppElements> → AppElements - mergeElementsPromise → mergeElements (sync object spread) - Slot: useContext only, no use(Promise) - SSR entry: pass resolved elements to context - dispatchBrowserTree: simplified, no async error handler Also fix flaky instrumentation E2E test that read the last error entry instead of finding by path.
- Remove Promise wrappers from ElementsContext test values - mergeElementsPromise → mergeElements (sync) - Replace Suspense streaming test with direct render test - Remove unused createDeferred helper and Suspense import - Update browser state test assertions (no longer async)
P1a: mergeElements preserves previous slot content when the new payload marks a parallel slot as unmatched. On soft navigation, unmatched slots keep their previous subtree instead of triggering notFound(). P1b: renderNavigationPayload now receives navId and checks for superseded navigations after its await. Stale payloads are discarded instead of being dispatched into the React tree. P2: The catch block in renderNavigationPayload only calls commitClientNavigationState() when activateNavigationSnapshot() was actually reached, preventing counter underflow. P3: The no-default-export fallback in buildPageElements now derives the root layout tree path from route.layoutTreePositions and route.routeSegments instead of hardcoding "/".
Prove the flat keyed map architecture works end-to-end: - Layout state persists across sibling navigation (counter survives) - Template remounts on segment boundary change, persists within segment - Error boundary clears on navigate-away-and-back - Back/forward preserves layout state through history - Parallel slots persist on soft nav, show default.tsx on hard nav Zero production code changes — test fixtures and Playwright specs only.
commit: |
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
Part of #726 (PR 3). Pure test code — zero production changes. Proves the flat keyed map payload pipeline from PR 2c actually works end-to-end in a real browser.
Every test answers: "would a real user notice if this broke?" Layout state surviving navigation, template state resetting on segment change, errors clearing on re-navigation — these are observable user behaviors, not implementation details.
What's tested
page.goto()resets counter vs Link click preserving it/→/about(segment changes)/dashboard→/dashboard/settings(same top-level segment)goBack()/goForward()through dashboard history@teamand@analyticsslot content survives/dashboard→/dashboard/settings/dashboard/settingsrendersdefault.tsxinstead of page slot contentWhat's NOT tested here (and why)
shouldHardNavigateintests/app-browser-entry.test.tsFixture changes
Minimal additions to
tests/fixtures/app-basic/to make behaviors observable:app/components/layout-counter.tsx—"use client"counter proving layout persistenceapp/components/template-counter.tsx—"use client"counter proving template remountapp/dashboard/layout.tsx— added LayoutCounter + nav links between dashboard pagesapp/template.tsx— added TemplateCounterapp/error-test/error.tsx— added "Go home" Link for error recovery navigationapp/page.tsx— added "Error Test" LinkAll existing E2E tests (navigation, error-handling, navigation-flows — 25 tests) still pass with these fixture changes.
Test plan
tests/e2e/app-router/layout-persistence.spec.ts— 8 E2E tests, all passingtests/app-router.test.ts -t "not-found|forbidden|unauthorized"— 5 SSR integration tests, all passingvp check— 0 lint/type errors