Insights: drop irrelevant fix cards from instant errors#94926
Conversation
…rrors; flag cache as N/A for connection()
Stats cancelledCommit: af1ebc4 |
Tests PassedCommit: af1ebc4 |
There was a problem hiding this comment.
Pull request overview
This PR refines the “Ways to fix this” guidance for instant-prerender validation errors across the dev overlay, CLI/build error messages, docs, and e2e tests by removing misleading fix cards and adding a connection()-specific cache caveat.
Changes:
- Remove the
generateStaticParams/ “For known params, prerender” guidance from relevant instant-prerender error families (runtime + client-hook). - Suppress the
"use cache"fix card when the dynamic error is triggered byconnection(), including deriving theconnectioncause from the highlighted code-frame line in the overlay. - Update error strings (including
errors.json) and test snapshots to reflect the new guidance.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/app-dir/instant-validation/instant-validation.test.ts | Updates expected fix-bullets to drop GSP and qualify cache for connection(). |
| test/e2e/app-dir/instant-validation/instant-validation-parallel-slots.test.ts | Removes expected GSP fix-bullets from runtime variants. |
| test/e2e/app-dir/instant-validation-level-warning/instant-validation-level-warning.test.ts | Updates expected cache bullet with connection() caveat. |
| test/e2e/app-dir/instant-validation-level-manual-warning/instant-validation-level-manual-warning.test.ts | Updates expected cache bullets and removes GSP bullet expectations. |
| test/e2e/app-dir/instant-validation-level-manual-error/instant-validation-level-manual-error.test.ts | Updates expected cache bullet with connection() caveat. |
| test/e2e/app-dir/instant-validation-level-error/instant-validation-level-error.test.ts | Updates expected cache bullet with connection() caveat. |
| test/e2e/app-dir/instant-validation-build/instant-validation-build.test.ts | Updates expected cache bullet with connection() caveat. |
| test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts | Updates expected metadata/viewport cache bullets and removes GSP bullet expectations. |
| test/e2e/app-dir/cache-components-errors/cache-components-errors.client.test.ts | Removes expected GSP fix-bullets from client-component variants. |
| test/e2e/app-dir/cache-components-errors/cache-components-client-hook-abort-reasons.test.ts | Removes client-hook GSP expectations; needs snapshot updates for removed ParamClientHookDynamicError. |
| packages/next/src/server/dynamic-rendering-utils.ts | Removes ParamClientHookDynamicError and narrows related type usage. |
| packages/next/src/server/app-render/dynamic-rendering.ts | Switches client hook dynamic error construction to ClientHookDynamicError. |
| packages/next/src/server/app-render/blocking-route-messages.ts | Removes GSP bullet and adds (does not apply to \connection()`)` to cache guidance. |
| packages/next/src/next-devtools/dev-overlay/container/errors.tsx | Adds deriveCauseFromCodeFrame() to detect connection() on the highlighted code-frame line. |
| packages/next/src/next-devtools/dev-overlay/container/errors.test.ts | Updates guidance tests for removed GSP and adds coverage for connection() cause derivation. |
| packages/next/src/next-devtools/dev-overlay/components/instant/instant-guidance-data.ts | Removes GSP cards and filters cache cards when cause === 'connection'. |
| packages/next/errors.json | Adds/updates error-message entries reflecting the new guidance text. |
| errors/blocking-prerender-viewport-dynamic.mdx | Documents that cache guidance does not apply to connection(). |
| errors/blocking-prerender-runtime.mdx | Removes “For known params, prerender” section and references. |
| errors/blocking-prerender-metadata-dynamic.mdx | Documents that cache guidance does not apply to connection(). |
| errors/blocking-prerender-dynamic.mdx | Documents that cache guidance does not apply to connection(). |
| errors/blocking-prerender-client-hook.mdx | Removes “For known params, prerender” section and updates framing for client hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Additional Suggestions:
- E2E snapshots in
cache-components-errors.test.tswere not updated for the new(does not apply toconnection())caveat added to metadata[cache]bullets, so the dev redbox code and build prerender output assertions will fail CI.
- Dev collapsed-redbox snapshots for dynamic
generateViewport()assert error codeE1352, but the source message now matches errors.json codeE1369(the new message with theconnection()caveat), so the assertions will fail CI.
… + connection caveat
Button label is 'Copy as prompt'; the jsdoc was inaccurate.
…gsp-fix-card-from-runtime-and-connection # Conflicts: # test/e2e/app-dir/cache-components-errors/cache-components-client-hook-abort-reasons.test.ts
There was a problem hiding this comment.
Additional Suggestion:
Build-mode (--debug-prerender) inline code-frame snapshots in cache-components-client-hook-abort-reasons.test.ts are stale: line numbers are +1 too high vs. source and 6 frames reference the removed class ParamClientHookDynamicError, so the build-mode CI jobs fail the toMatchInlineSnapshot assertions.
Both line numbers and class name were stale: - Line numbers were +1 too high vs current source. - 6 frames referenced the removed ParamClientHookDynamicError class. Per Roselbot's review on #94926.
The PR's own snapshots in cache-components-client-hook-abort-reasons.test.ts
still had:
- the '[cache] For `params`: if the params are known, prerender them with
`generateStaticParams`' bullet that this PR removes
- the old '[cache] For uncached data' bullet without the new
'(does not apply to `connection()`)' qualifier
28 occurrences fixed across both turbopack/webpack and debug-prerender/normal
variants.
Whitespace-only changes from cargo fmt --all.
The viewport error's [cache] bullet was missing the '(does not apply to `connection()`)' qualifier that this PR added to the other [cache] bullets. Adds it for consistency and updates the 4 matching snapshot occurrences in cache-components-errors.test.ts. Per vercel[bot] review.
`createDynamicOrRuntimeViewportError`'s message changed in the previous commit. Adds the new entry to errors.json so check-error-codes passes.
8 more useParams() build-mode inline snapshots referenced the removed '[cache] For known params, prerender them with `generateStaticParams`' bullet from blocking-prerender-client-hook. These were the client-hook analog of the runtime/dynamic GSP bullet I removed earlier. Per vercel[bot] review.
Why?
Two Insight fix cards were misleading:
generateStaticParamsshowed up on every runtime/client-hook insight. One error coverscookies()/headers()/params/searchParams. GSP only applies toparams; for the others it's noise. Even forparamsit nudges devs to make the route static instead of fixing the immediate error."use cache"showed up onconnection()triggers. Cachingconnection()is contradictory.Both manifest on initial load and in-navigation (the fix-card sets are shared).
What?
"use cache"when the cause isconnection(). Affects 05-connection-body, 08-connection-body-dynamic, 31-connection-in-metadata, 33-connection-in-viewport. 06-uncached-fetch-body keeps the Cache card.How?
getCards()filters the cache card whencause === 'connection'.deriveCauseFromCodeFrame()helper detectsconnection(on the highlighted code-frame line.ParamClientHookDynamicErrorcollapsed intoClientHookDynamicError.(does not apply to \connection()`)` on the cache bullet.For known params, prerendersections; added a connection caveat on dynamic/metadata-dynamic/viewport-dynamic pages.