1 Commits

Author	SHA1	Message	Date
Yoinky3000	493e72d256	fix(app-router): catch redirect diges, render loading.tsx during cross-route navigation and invalid with calling refresh() (#1152) ... * fix(app-router): catch server redirect digest, render loading.tsx across navigation * Update page.tsx * fix(app-router): tighten RedirectBoundary digest handling and dedupe nav-signal check Addresses code-review findings on PR #1152: - RedirectBoundary.reset() now clears both `redirect` and `redirectType` so the state never sits in a half-cleared (`null`, "replace") shape. - getDerivedStateFromError re-throws on malformed `NEXT_REDIRECT;` digests (empty URL segment) instead of silently swallowing the error. Mirrors the server-side guard in parseNextRedirectDigest (next-error-digest.ts:51). - Document the `error.handled` branch as a Next.js parity placeholder so it's clear it is not dead code, just unused by vinext today. - Extract `getErrorDigest` / `isNavigationSignalError` into a shared utils/navigation-signal.ts, removing the duplicated digest classification in shims/error-boundary.tsx and server/dev-error-overlay.tsx. - Update stale `snapshotRscResponse` reference in the cached-route comment to the current body.tee + createFromFetch flow. - Strengthen the delayed-redirect-under-loading E2E test to also assert the loading.tsx fallback paints during the 50ms server delay. - Format fix on tests/fixtures/app-basic/app/page.tsx that was failing the CI Check job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(app-router): invalidate client RSC caches on router.refresh() router.refresh() previously only re-fetched the current URL via the "refresh" navigationKind, leaving the visited-response and prefetch caches untouched for sibling routes. After a session change (e.g. authClient.signOut() → router.push("/") → router.refresh()), a subsequent <Link> click to a previously visited auth-gated route would render the stale cached RSC payload and bypass the server's redirect() entirely. Matches Next.js's refresh-reducer.ts, whose header comment is explicit: "During a refresh, we invalidate the segment cache but not the route cache. The segment cache contains the actual RSC data which needs to be re-fetched." Next does this via an O(1) version counter (invalidateSegmentCacheEntries → currentSegmentCacheVersion++); vinext's bounded LRU is small enough that Map.clear() on the visited-response + prefetch caches is equivalent in observable behavior. Pre-existing in upstream vinext (verified against 8edc010); only surfaced after the redirect-digest fix earlier in this PR, because before that the unhandled NEXT_REDIRECT digest aborted the page render outright and masked the stale-cache reuse. - Expose clearClientNavigationCaches as window.__VINEXT_CLEAR_NAV_CACHES__ - router.refresh() calls it synchronously before scheduling the RSC re-fetch inside React.startTransition. Synchronous ordering matters: if the clear ran inside the transition, prefetches kicked off during the transition's renders could repopulate stale entries into the just-cleared cache. - Unit test pins the clear-before-navigate ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(app-router): address review — remove HandleRedirect reset, fix boundary nesting, add malformed digest test Address ask-bonk review comments from all three rounds: 1. Remove reset() from HandleRedirect — calling reset() inside startTransition clears the boundary state, re-rendering children and re-mounting the page that threw redirect(). For deterministic redirects (auth guards) this creates an infinite loop. Next.js intentionally does not call reset() in HandleRedirect (redirect-boundary.tsx); the boundary stays in its 'caught' state rendering null until router.push()/replace() mounts fresh children. 2. Move RedirectBoundary inside Suspense (loading boundary) — Next.js nesting is Error > Loading > AccessFallback > Redirect > content. The previous placement had RedirectBoundary outside Suspense, which unmounted the loading fallback during redirect transitions. Now RedirectBoundary wraps the page slot directly, and Suspense wraps both. 3. Add comment documenting dual-format digest parsing — the parts.length >= 5 branching handles both vinext 3/4-part and Next.js 5-part digests, which was non-obvious to reviewers. 4. Add test for malformed-digest re-throw guard — verifies that getDerivedStateFromError re-throws when the URL segment is empty (e.g. NEXT_REDIRECT;push;) rather than silently swallowing the error. * fix(app-router): correct nesting-order comment, add error.handled test Address ask-bonk round-4 review: 1. Fix the boundary nesting comment in app-page-route-wiring.tsx — it incorrectly claimed the order matches Next.js's Error > Loading > AccessFallback > Redirect > content. The actual vinext order is Error > AccessFallback > Loading > Redirect > content; the AccessFallback positioning is a pre-existing divergence. 2. Add test for the error.handled branch in RedirectErrorBoundary — verifies the Next.js parity placeholder returns null state when a redirect error is already marked as handled by an outer boundary. * refactor: use shared header constants instead of string literals Replace two hardcoded header strings in the visited-response cache snapshot with the already-imported VINEXT_MOUNTED_SLOTS_HEADER and VINEXT_PARAMS_HEADER constants from server/headers.ts. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: James <james@eli.cx>	2026-05-12 16:48:15 +00:00