feat: track previousNextUrl for intercepted App Router entries#755
Draft
NathanDrake2406 wants to merge 25 commits intocloudflare:mainfrom
Draft
feat: track previousNextUrl for intercepted App Router entries#755NathanDrake2406 wants to merge 25 commits intocloudflare:mainfrom
NathanDrake2406 wants to merge 25 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: |
NathanDrake2406
changed the title
8 tasks
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 5). This builds on PR 4's interception-aware payload IDs and cache keys by tracking
previousNextUrlin the App Router browser state. Intercepted navigations now remember the URL they came from, so refresh/back-forward behavior can distinguish soft interception from direct loads.The practical effect is that
/feed -> /photos/42modal navigations now behave correctly across the three important cases: browser reload breaks out to the full page, back/forward restores the intercepted modal view from history state, androuter.refresh()keeps the intercepted modal instead of silently switching to the direct page tree.What changed
app-browser-statenow carriespreviousNextUrlalongsideelements,interceptionContext,routeId, androotLayoutTreePath, with helpers for persisting and reading it from history state__vinext_previousNextUrlinstead of the old interception-context field, preserving the URL active before a soft navigation while keeping the existing scroll restoration state intactnavigaterecords the current URL aspreviousNextUrl,traversereconstructs interception context fromevent.state, andrefreshreconstructs it from committed router state so refresh preserves interception without changing hard-load behaviorpreviousNextUrlfrom the active history entry so full document reloads still break out of interception even though browsers preservehistory.stateacross reloadspreviousNextUrlforward so replacement navigations do not lose the source-route provenancepreviousNextUrlstate/history helpers plus E2E coverage for reload, back/forward restoration, androuter.refresh()on intercepted routes. The modal fixture now exposes a smallrouter.refresh()button to exercise the public APIapp-browser-entrycomments so they match the current dispatch/control-flow names after the refactorWhat this does NOT do
Server action interception-request parity is still intentionally deferred. This PR preserves the browser-side
previousNextUrlstate during server-action merges, but it does not add interception headers to the action POST itself.Test plan
vp test run tests/app-browser-entry.test.tsvp test run tests/app-router.test.tsvp run test:e2e tests/e2e/app-router/advanced.spec.ts --grep 'hard reload after intercepted navigation renders the full page|back then forward restores intercepted modal view|router.refresh preserves intercepted modal view'git commitpre-commit hook (vp check --fix)