Remove legacy PPR codepaths#94955
Conversation
Stats cancelledCommit: 81ffcd6 |
Tests PassedCommit: 81ffcd6 |
![vercel[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Suggestion:
Removing route-level isRoutePPREnabled plumbing routes the Instant Navigation Testing API / debug static shell into app-render's legacy prerender branch when cacheComponents is disabled, which the in-code comment says "hangs in dev mode."
eps1lon
force-pushed
the
sebbie/remove-legacy-ppr
branch
4 times, most recently
from
3210e66 to
2868ad9
Compare
| } | ||
|
|
||
| if (typeof dynamic === 'string') { | ||
| if (cacheComponents) { |
There was a problem hiding this comment.
After the output: 'export' branch synthesizes dynamic = 'error' for pages with no authored dynamic, the second if (typeof dynamic === 'string') { if (cacheComponents) throw } block still fires, so every page/layout in an output: 'export' + cacheComponents: true project throws a misleading "Please remove it" error even when no dynamic config was authored.
eps1lon
force-pushed
the
sebbie/remove-legacy-ppr
branch
from
2868ad9 to
13959c3
Compare
eps1lon
force-pushed
the
sebbie/remove-legacy-ppr
branch
from
13959c3 to
9b19d8c
Compare
| // TODO: Remove this since this should be caught during compilation. | ||
| // Left as a guard in case usage of Postpone was attempted | ||
| throw new Error( | ||
| `Route segment config "dynamic" is not compatible with \`nextConfig.cacheComponents\`. Please remove it.` | ||
| ) |
There was a problem hiding this comment.
Nit: In other similar cases we're throwing an invariant error like this:
next.js/packages/next/src/server/request/cookies.ts
Lines 83 to 85 in f66c8a6
There was a problem hiding this comment.
Not sure yet if this is an invariant yet or an actual export const dynamic that we didn't catch at build time.
| ) | ||
| } | ||
|
|
||
| // TODO: vroom |
There was a problem hiding this comment.
🚗 Did you plan to revisit this?
There was a problem hiding this comment.
Yeah. When inlining the flag, I wasn't sure yet if this is actually just the CC flag. But it's also based on the whether this is app dir or pages dir so I kept it. Removing the comment.
| } | ||
| export function Postpone({ reason, prerenderState, pathname }) { | ||
| postponeWithTracking(prerenderState, reason, pathname); | ||
| export function ApiThatShallNotBeNamed({ reason, prerenderState, pathname }) { |
eps1lon
force-pushed
the
sebbie/remove-legacy-ppr
branch
from
9b19d8c to
47e5f6c
Compare
eps1lon
force-pushed
the
sebbie/remove-legacy-ppr
branch
from
47e5f6c to
7552a45
Compare
| if (result.cacheComponents) { | ||
| // TODO: remove once we've finished migrating internally to cacheComponents. | ||
| result.experimental.ppr = true | ||
| } | ||
|
|
There was a problem hiding this comment.
Broke legacy builders that require changes like vercel/vercel#16706.
Fixing in #95111
There was a problem hiding this comment.
Reverting for now (#95113) and then thinking about forward fix.
2 participants
These should be unreachable since we removed support for
config.experimental.ppr.Had to be previously reverted in #90948.
No new findings from my side. I added new errors where in
forceDynamic && staticGeneration && cacheComponentbranches. These should not be reachable but now we throw explicitly instead of trying to reconcile that defect.We'll check in closely with the people previously seeing regressions.