[Segment Cache] Add PPR header to segment prefetch #73756

acdlite · 2024-12-10T19:25:09Z

Follow-up to #73715. Since PPR should always result in a cache hit for a prefetch (even in the worst case, there's a fallback cache entry), the only reason a route tree prefetch would not be found in the per-segment cache is because PPR is disabled.

So we should just fallthrough to the old prefetching flow whenever PPR is disabled.

But the client still needs some way to tell whether the response was served from the per-segment cache or by the old implementation.

So I added an explicit response header to every per-segment prefetch. If the header is missing, the client can infer that the route does not support PPR.

ijjk · 2024-12-10T19:38:30Z

Tests Passed

ijjk · 2024-12-10T19:42:46Z

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
buildDuration	18.4s	16.9s	N/A
buildDurationCached	16s	13.5s	N/A
nodeModulesSize	409 MB	409 MB	⚠️ +20.1 kB
nextStartRea..uration (ms)	486ms	481ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
1187-HASH.js gzip	50.7 kB	50.8 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.36 kB	5.36 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	232 B	235 B	N/A
main-HASH.js gzip	34 kB	34 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.49 kB	4.49 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.34 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
_buildManifest.js gzip	747 B	746 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
index.html gzip	524 B	522 B	N/A
link.html gzip	539 B	537 B	N/A
withRouter.html gzip	520 B	519 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
edge-ssr.js gzip	128 kB	128 kB	N/A
page.js gzip	203 kB	203 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
middleware-b..fest.js gzip	671 B	667 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.2 kB	31.2 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
523-experime...dev.js gzip	322 B	322 B	✓
523.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	323 kB	323 kB	N/A
app-page-exp..prod.js gzip	127 kB	127 kB	N/A
app-page-tur..prod.js gzip	140 kB	140 kB	N/A
app-page-tur..prod.js gzip	135 kB	135 kB	N/A
app-page.run...dev.js gzip	313 kB	313 kB	N/A
app-page.run..prod.js gzip	123 kB	123 kB	N/A
app-route-ex...dev.js gzip	37.3 kB	37.3 kB	✓
app-route-ex..prod.js gzip	25.4 kB	25.4 kB	✓
app-route-tu..prod.js gzip	25.4 kB	25.4 kB	✓
app-route-tu..prod.js gzip	25.2 kB	25.2 kB	✓
app-route.ru...dev.js gzip	38.9 kB	38.9 kB	✓
app-route.ru..prod.js gzip	25.2 kB	25.2 kB	✓
pages-api-tu..prod.js gzip	9.67 kB	9.67 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.66 kB	9.66 kB	✓
pages-turbo...prod.js gzip	21.7 kB	21.7 kB	✓
pages.runtim...dev.js gzip	27.4 kB	27.4 kB	✓
pages.runtim..prod.js gzip	21.7 kB	21.7 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	N/A
Overall change	279 kB	279 kB	✓

build cache

	vercel/next.js canary	acdlite/next.js segment-cache-ppr-header	Change
0.pack gzip	2.05 MB	2.05 MB	N/A
index.pack gzip	72.4 kB	72.1 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for edge-ssr.js

Diff too large to display

Diff for 1187-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 1b54cfd

ztanner · 2024-12-11T21:29:44Z

packages/next/src/server/base-server.ts

+      // to indicate that the response contains dynamic data, but here we're
+      // only using it to indicate that the feature is enabled — the segment
+      // response itself contains whether the data is dynamic.
+      res.setHeader(NEXT_DID_POSTPONE_HEADER, '2')


this distinction feels very subtle but I suppose the end-goal will be to remove this case all together and not worth adding a new header for

Yeah just didn't feel worth adding another header

ztanner · 2024-12-11T21:37:31Z

packages/next/src/server/base-server.ts

@@ -3044,10 +3044,19 @@ export default abstract class Server<
      }
    )

-    if (isPrefetchRSCRequest && typeof segmentPrefetchHeader === 'string') {
+    if (isRoutePPREnabled && typeof segmentPrefetchHeader === 'string') {


woops, this always should have been gated on PPR right?

It used to not have the check so that it would 404/204 if PPR was disabled (since why not, nothing should be issuing these requests and it's better than it falling through to a old-style prefetch) but then it turned out I need it to fallthrough to an old-style prefetch anyway.

ztanner · 2024-12-11T21:41:30Z

test/e2e/app-dir/segment-cache/incremental-opt-in/segment-cache-incremental-opt-in.test.ts

  // TODO: Replace with e2e test once the client part is implemented
-  it('prefetch responds with 204 if PPR is disabled for a route', async () => {
+  it('route tree prefetch falls through to pre-Segment Cache implementation if PPR is disabled for a route', async () => {


Suggested change

it('route tree prefetch falls through to pre-Segment Cache implementation if PPR is disabled for a route', async () => {

it('route tree prefetch falls through to per-segment cache implementation if PPR is disabled for a route', async () => {

that was actually the intended wording. I meant the "old" (before Segment Cache) implementation. I'll rephrase 😆

Follow-up to vercel#73715. Since PPR should always result in a cache hit for a prefetch (even in the worst case, there's a fallback cache entry), the only reason a route tree prefetch would not be found in the per-segment cache is because PPR is disabled. So we should just fallthrough to the old prefetching flow whenever PPR is disabled. But the client still needs some way to tell whether the response was served from the per-segment cache or by the old implementation. So I added an explicit response header to every per-segment prefetch. If the header is missing, the client can infer that the route does not support PPR.

ijjk added created-by: Next.js team PRs by the Next.js team. tests type: next labels Dec 10, 2024

acdlite force-pushed the segment-cache-ppr-header branch 2 times, most recently from a561377 to b56bfd2 Compare December 10, 2024 22:25

ztanner approved these changes Dec 11, 2024

View reviewed changes

acdlite force-pushed the segment-cache-ppr-header branch from b56bfd2 to 1b54cfd Compare December 12, 2024 04:47

acdlite merged commit 363a7d4 into vercel:canary Dec 12, 2024
86 checks passed

github-actions bot added the locked label Dec 26, 2024

github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Cache] Add PPR header to segment prefetch #73756

[Segment Cache] Add PPR header to segment prefetch #73756

acdlite commented Dec 10, 2024

ijjk commented Dec 10, 2024 •

edited

Loading

ijjk commented Dec 10, 2024 •

edited

Loading

ztanner Dec 11, 2024

acdlite Dec 12, 2024

ztanner Dec 11, 2024

acdlite Dec 12, 2024

ztanner Dec 11, 2024

acdlite Dec 12, 2024

	it('route tree prefetch falls through to pre-Segment Cache implementation if PPR is disabled for a route', async () => {
	it('route tree prefetch falls through to per-segment cache implementation if PPR is disabled for a route', async () => {

[Segment Cache] Add PPR header to segment prefetch #73756

[Segment Cache] Add PPR header to segment prefetch #73756

Conversation

acdlite commented Dec 10, 2024

ijjk commented Dec 10, 2024 • edited Loading

Tests Passed

ijjk commented Dec 10, 2024 • edited Loading

Stats from current PR

ztanner Dec 11, 2024

Choose a reason for hiding this comment

acdlite Dec 12, 2024

Choose a reason for hiding this comment

ztanner Dec 11, 2024

Choose a reason for hiding this comment

acdlite Dec 12, 2024

Choose a reason for hiding this comment

ztanner Dec 11, 2024

Choose a reason for hiding this comment

acdlite Dec 12, 2024

Choose a reason for hiding this comment

ijjk commented Dec 10, 2024 •

edited

Loading

ijjk commented Dec 10, 2024 •

edited

Loading