Polish instant fix cards and validation messages#93894
Draft
aurorascharff wants to merge 32 commits into
Draft
Conversation
Adds two pieces of framework correctness for `unstable_instant` validation:
1. **Navigation body factories** in `blocking-route-messages.ts`:
- `createRuntimeBodyErrorInNavigation` (E1247)
- `createDynamicBodyErrorInNavigation` (E1246)
Wired into `trackDynamicHoleInNavigation` so the in-navigation case emits
its own message ("accessed under `<Suspense>`" + nav-aware "Ways to fix")
instead of reusing the initial-render copy ("accessed outside of
`<Suspense>`"). Phase-neutral phrasing ("during the initial render or a
navigation") accommodates the path firing from either prerender or
client-nav validation.
2. **Clear-on-nav reducer** in the dev overlay:
- `ACTION_INSTANT_ERRORS_CLEAR` reducer case in `shared.ts`.
- `useClearInstantErrorsOnNav` hook in `dev-overlay.tsx` watches
`usePathname()` and dispatches when the route changes.
Only errors fired during a client navigation are eligible to clear:
`*InNavigation` factories carry an `__nextInstantNav` marker, and wrapper
headlines (E1082 / E1112 / E1113 / E1118) are detected via the stable
"Could not validate `unstable_instant`" substring. Initial-render / SSR
errors lack both signals and persist, so a page that fails to prerender
stays in the overlay until the source is fixed.
Co-authored-by: Cursor <[email protected]>
Reads the `__nextInstantNav` marker from the error to pick the right overlay copy for the blocking-route family: - Headline: "Next.js encountered runtime/uncached data during a navigation." (in-nav) vs. "during the initial render." (initial). - Explanation: "This prevents the navigation from being instant..." vs. the default "prevents the route from being prerendered..." string. Adds `BLOCKING_ROUTE_NAVIGATION_EXPLANATION` next to `EXPLANATIONS` in `instant-guidance-data.ts` so all overlay copy stays in one place. Also renames the legacy `'navigation'` value of `GuidanceVariant` to `'dynamic'` — the original name meant "uncached data" (from #92638) and collides with the new in-navigation concept. The new shape is `'runtime' | 'dynamic'` for what kind of data is accessed, plus an orthogonal `inNavigation: boolean` for the phase. Co-authored-by: Cursor <[email protected]>
Object markers on the Error don't survive RSC serialization (`__NEXT_ERROR_CODE` already has a side-channel Map for this same reason), so the previous `__nextInstantNav` property was getting stripped before the dev overlay ever saw it. Switches detection to two structural substrings already present in the user-facing copy: - \`accessed under \`<Suspense>\`\` — only emitted by the in-navigation body factories. The mirrored SSR factories say \`accessed outside of \`<Suspense>\`\`, so the two are unambiguous. - \`Could not validate \`unstable_instant\`\` — already used to detect wrapper errors (E1082 / E1112 / E1113 / E1118), tied to the API name rather than phase wording. Drops the marker helper from `blocking-route-messages.ts`. Co-authored-by: Cursor <[email protected]>
The dev overlay renders in a separate React root mounted at the document body (via `createRoot(container)` in `dev-overlay.browser.tsx`), so the App Router's `PathnameContext` isn't in scope and `usePathname()` returns `null` — making the previous `useClearInstantErrorsOnNav` effect a no-op. Switches to a browser-level subscription: listen for `popstate` (back / forward) and patch `history.pushState` / `history.replaceState` to detect client navigations triggered by `next/link` and `router.push()`. Cleanup restores the originals so the patch is scoped to overlay lifetime. This matches the pattern other dev-overlay code already uses for current path (e.g. `route-info.tsx` reads `window.location.pathname` directly). Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
…/instant-nav-error-clear
The new `createRuntimeBodyErrorInNavigation` / `createDynamicBodyErrorInNavigation` factories produce "encountered runtime data during a navigation" wording in the dev overlay (via inNavigation substring detection in errors.tsx). Seven inline snapshots in instant-validation-parallel-slots.test.ts locked in the older "during the initial render" + E1221 / E1220 wording — flipped to the new "during a navigation" + E1247 / E1246. The eighth test in this file is left at canary state: it relies on PR #93770's "expected segment was not rendered" message which currently bakes in a test/tmp/next-test-… absolute path. That test is broken on canary HEAD too (verified directly) — not a regression from this PR. Co-authored-by: Cursor <[email protected]>
`instant-validation.test.ts` (28 snapshots) and
`instant-validation-parallel-slots.test.ts` (7 snapshots) had build-mode
inline snapshots that pinned the older `createRuntimeBodyError` /
`createDynamicBodyError` wording. Switched to the new in-navigation
factory output:
- "encountered runtime data during the initial render."
+ "encountered runtime data during the initial render or a navigation."
- "accessed outside of `<Suspense>` prevents the route from being
prerendered, blocking navigation and leading to a slower user
experience."
+ "accessed under `<Suspense>` prevents the route from being
prerendered or the navigation from being instant, leading to a
slower user experience."
Captured without NEXT_SKIP_ISOLATE so the test packed next.js as a
tarball (matching the CI build path); avoids leaking local
`../../../packages/next/dist/esm/...` stack frames into the snapshots.
Co-authored-by: Cursor <[email protected]>
The in-navigation body factories said "accessed under \`<Suspense>\`", which contradicts the fix bullet that recommends wrapping in Suspense. The actual problem is the same as the SSR factories: data accessed *outside* a Suspense boundary, where the prerender / prefetchable shell can't proceed without waiting. Aligned both InNavigation factories to "accessed outside of \`<Suspense>\`" so the diagnostic matches the SSR factories — only the consequence clause differs (still includes "or the navigation from being instant"). Refactored the substring detection that distinguishes in-navigation vs SSR variants into a single `isBlockingRouteInNavError` helper in `shared.ts`, used by both `getInstantErrorRoute` (clear-on-nav) and `getBlockingRouteErrorDetails` (overlay headline phase). Helper keys off "or a navigation" which is still unique to the InNavigation factory pair (the diagnostic clause is now shared). Also narrowed `getInstantErrorRoute` to detect *only* body factory errors — wrapper errors like "Could not validate \`unstable_instant\`" are no longer cleared on navigation. They describe a validation infrastructure failure rather than a fixable route-level mistake and should stay in the redbox stack until the user addresses the cause. Snapshot updates across `instant-validation.test.ts` (dev + build modes) and `instant-validation-parallel-slots.test.ts` (dev + build modes) reflect the new wording. Build-mode snapshots captured without NEXT_SKIP_ISOLATE so the test packs Next.js as a tarball and produces the same stack frame shape CI sees (avoids leaking local dist paths). Co-authored-by: Cursor <[email protected]>
…nt-nav-error-clear
- Drop history.pushState/replaceState patching in favor of state.page
from ACTION_DEVTOOL_UPDATE_ROUTE_STATE (cooperates with App Router's
own history patching).
- Update errors.test.ts for new {inNavigation, variant: 'dynamic'} shape
and add InNavigation factory cases.
- Bump E1246/E1247 -> E1249/E1250 in level-* and causes tests after the
factory wording change.
Co-authored-by: Cursor <[email protected]>
- Restore the 13-space indent on `Unrendered segment(s):` paths in dev inline snapshots (got stripped during the earlier wording sweep). - Use the stable `app/...` prefix instead of the non-deterministic `test/tmp/next-test-<timestamp>-<rand>/...` path that crept into one parallel-slots snapshot. Co-authored-by: Cursor <[email protected]>
`state.page` is the resolved URL (e.g. `/foo/123`), but an instant error's embedded `Route "..."` uses the route pattern (`/foo/[param]`). Firing the clear action on the initial '' → page transition would always wipe dynamic-route errors before the redbox could open. Treat the first non-empty page as the baseline and only dispatch on subsequent changes. Co-authored-by: Cursor <[email protected]>
The SSR `createRuntimeBodyError` lists the Suspense placeholder first (the most general fix) followed by `generateStaticParams`. The in-navigation variant had them flipped, so the top-of-list suggestion was the narrower one (only applies to dynamic-param routes). Reorder the in-nav factory to match SSR, and refresh the build-mode CLI snapshots and the runtime-body in-nav error code (E1250 → E1251). Co-authored-by: Cursor <[email protected]>
The earlier wording sweep on the level-* tests preserved the pre-existing 2-space-shy indent on the `code` and `description` lines, so Jest's diff reported a whitespace-only mismatch even though the values agreed. Match them to the sibling JSON keys (13 spaces). Co-authored-by: Cursor <[email protected]>
The previous patch overshot to 13 spaces in tests where the snapshot braces sit two columns shallower (so sibling keys are at 11). Auto-align each `code`/`description` line to the indent of the next sibling key. Co-authored-by: Cursor <[email protected]>
`trackDynamicHoleInNavigation` is also reached during the build-time prerender pass, so the CLI build error now reads "during the initial render or a navigation" with the matching consequence clause. Update the build-mode snapshots in `instant-validation-build` and the four `instant-validation-level-*` tests to match. Co-authored-by: Cursor <[email protected]>
…ording The earlier wording sweep also touched the 4 \`without-root-suspense\` E1220 snapshots, which come from the SSR factory (initial-render path) and should keep "during the initial render." Restore the SSR wording on those four. Co-authored-by: Cursor <[email protected]>
…/instant-nav-error-clear
`Route "<path>":` in the error message is the route template (e.g. `/foo/[slug]`), while `state.page` is the resolved URL (e.g. `/foo/123`). Equality matching dropped SSR-streamed errors on the destination page during navigation for any dynamic route. Convert the template to a regex so the clear-on-nav reducer keeps errors whose template matches the page the user just landed on.
Contributor
Failing test suitesCommit: 1900b73 | About building and testing Next.js
Expand output● instant validation - level error › dev › bare page: implicit validation surfaces a redbox (error level fires) ● instant validation - level error › dev › explicit-error page: explicit override at the configured level, instant redbox in dev ● instant validation - level error › dev › explicit-true page: aliases to error level, instant redbox in dev ● instant validation - level error › dev › explicit-warning page: per-segment de-escalation still validates in dev ● instant validation - level error › dev › layered: bare page under layout-with-instant-false still validates
Expand output● instant validation - level manual-error › dev › explicit-error page: explicit override at the configured level, instant redbox in dev ● instant validation - level manual-error › dev › explicit-true page: aliases to error level, instant redbox in dev ● instant validation - level manual-error › dev › explicit-warning page: per-segment de-escalation still validates in dev
Expand output● instant validation - level manual-warning › with-root-suspense › dev › explicit-error page: instant redbox surfaces under manual-warning ● instant validation - level manual-warning › with-root-suspense › dev › explicit-true page: opt-in falls back to warning level, instant redbox in dev ● instant validation - level manual-warning › with-root-suspense › dev › explicit-warning page: instant redbox in dev ● instant validation - level manual-warning › without-root-suspense › dev › bare page: static-shell error surfaces but is not labelled Instant ● instant validation - level manual-warning › without-root-suspense › dev › explicit-error page: instant redbox surfaces (instant subsumes static-shell when active) ● instant validation - level manual-warning › without-root-suspense › dev › explicit-true page: instant redbox surfaces in dev ● instant validation - level manual-warning › without-root-suspense › dev › explicit-warning page: instant redbox surfaces in dev
Expand output● instant validation › requires a static shell if a below a static layout page is configured as blocking › errors in dev
Expand output● instant validation causes › named export - export { unstable_instant } ● instant validation causes › aliased export - export { instant as unstable_instant } ● instant validation causes › re-export - export { unstable_instant } from "./config" ● instant validation causes › indirect export - const instant = _instant; export { instant as unstable_instant }
Expand output● instant validation › requires a static shell if a below a static layout page is configured as blocking › errors in dev
Expand output● Cache Components Dev Errors › should show a red box error on the SSR render ● Cache Components Dev Errors › should not show a red box error on client navigations ● Cache Components Dev Errors › should display error when component accessed data without suspense boundary
Expand output● Cache Components Errors › Dev › Dynamic Metadata - Error Route › should show a collapsed redbox error ● Cache Components Errors › Dev › Dynamic Viewport - Static Route › should show a collapsed redbox error ... truncated to fit in one GitHub comment ... |
…/revamp-fix-cards # Conflicts: # packages/next/src/next-devtools/dev-overlay/container/errors.tsx # test/e2e/app-dir/instant-validation-build/instant-validation-build.test.ts # test/e2e/app-dir/instant-validation-level-error/instant-validation-level-error.test.ts # test/e2e/app-dir/instant-validation-level-manual-error/instant-validation-level-manual-error.test.ts # test/e2e/app-dir/instant-validation-level-manual-warning/instant-validation-level-manual-warning.test.ts # test/e2e/app-dir/instant-validation-level-warning/instant-validation-level-warning.test.ts # test/e2e/app-dir/instant-validation/instant-validation-parallel-slots.test.ts # test/e2e/app-dir/instant-validation/instant-validation.test.ts
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.
What?
Polish pass on the instant validation overlay and CLI messages.
TODO add new block icon (browser spinner)
Why?
Feedback from testing.
Demo
How
Prerender params if known,Mark the route as dynamic), simplified client sync IO snippets to drop hooks, renamed the Profile card (For telemetry, use a timing API), removed viewportWrap body in Suspensefrom both variants.during prerenderingandblocking the page load; server sync IO namesthe unstable value …; client sync IO dropsfixed at build time; bullets aligned with the cards.scrollbar-gutter: stableto stop the dialog scrollbar flashing on error swap.E2E snapshots refreshed across
instant-validation*,cache-components-*, andbuild-output-prerender.