From 376c207e30dc42ae48ecbe3ae20123656e2a95c1 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 15 Dec 2024 13:58:15 -0500 Subject: [PATCH 1/2] [Segment Cache] Properly encode error digests This was an oversight on my part when I originally implemented the per- segment prefetches. `onError` needs to return an error digest. --- .../app-render/collect-segment-data.tsx | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/next/src/server/app-render/collect-segment-data.tsx b/packages/next/src/server/app-render/collect-segment-data.tsx index 15e14f672b3ad..c94e1221d6995 100644 --- a/packages/next/src/server/app-render/collect-segment-data.tsx +++ b/packages/next/src/server/app-render/collect-segment-data.tsx @@ -25,6 +25,7 @@ import { encodeSegment, ROOT_SEGMENT_KEY, } from './segment-value-encoding' +import { getDigestForWellKnownError } from './create-error-handler' // Contains metadata about the route tree. The client must fetch this before // it can fetch any actual segment data. @@ -66,6 +67,15 @@ export type SegmentPrefetch = { isPartial: boolean } +function onSegmentPrerenderError(error: unknown) { + const digest = getDigestForWellKnownError(error) + if (digest) { + return digest + } + // We don't need to log the errors because we would have already done that + // when generating the original Flight stream for the whole page. +} + export async function collectSegmentData( fullPageDataBuffer: Buffer, staleTime: number, @@ -120,10 +130,7 @@ export async function collectSegmentData( clientModules, { signal: abortController.signal, - onError() { - // Ignore any errors. These would have already been reported when - // we created the full page data. - }, + onError: onSegmentPrerenderError, } ) @@ -326,10 +333,7 @@ async function renderSegmentPrefetch( clientModules, { signal: abortController.signal, - onError() { - // Ignore any errors. These would have already been reported when - // we created the full page data. - }, + onError: onSegmentPrerenderError, } ) const segmentBuffer = await streamToBuffer(segmentStream) From 3b7ce7832ee3b42a4d0473a28a9fba8482c5bebb Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 15 Dec 2024 01:06:10 -0500 Subject: [PATCH 2/2] Generate per-segment responses for any static page Originally I gated per-segment prefetch generation on the PPR flag, because I thought the client Segment Cache would require PPR to be enabled on the server. However, since then the strategy has evolved and I do think we can roll out the Segment Cache independently of PPR. Dynamic pages without PPR won't be able to take full advantage of the Segment Cache, but if the page is fully static then there's no reason we can't implement all the same behavior. So during per-segment prerendering, I've changed the feature condition to check for the `clientSegmentCache` flag instead of the PPR one. --- packages/next/src/export/index.ts | 1 + .../next/src/server/app-render/app-render.tsx | 24 +++++++++++++++---- .../app-render/collect-segment-data.tsx | 15 ++++++++++-- packages/next/src/server/app-render/types.ts | 1 + packages/next/src/server/base-server.ts | 2 ++ .../ppr-navigations/simple/next.config.js | 1 + 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index a42495142f798..b593e2410288d 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -357,6 +357,7 @@ async function exportAppImpl( clientTraceMetadata: nextConfig.experimental.clientTraceMetadata, expireTime: nextConfig.expireTime, dynamicIO: nextConfig.experimental.dynamicIO ?? false, + clientSegmentCache: nextConfig.experimental.clientSegmentCache ?? false, inlineCss: nextConfig.experimental.inlineCss ?? false, authInterrupts: !!nextConfig.experimental.authInterrupts, }, diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 182eecb544ce6..84606c6e17785 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -4001,10 +4001,7 @@ async function collectSegmentData( // decomposed into a separate stream per segment. const clientReferenceManifest = renderOpts.clientReferenceManifest - if ( - !clientReferenceManifest || - renderOpts.experimental.isRoutePPREnabled !== true - ) { + if (!clientReferenceManifest || !renderOpts.experimental.clientSegmentCache) { return } @@ -4022,8 +4019,27 @@ async function collectSegmentData( serverModuleMap: null, } + // When dynamicIO is enabled, missing data is encoded to an infinitely hanging + // promise, the absence of which we use to determine if a segment is fully + // static or partially static. However, when dynamicIO is not enabled, this + // trick doesn't work. + // + // So if PPR is enabled, and dynamicIO is not, we have to be conservative and + // assume all segments are partial. + // + // TODO: When PPR is on, we can at least optimize the case where the entire + // page is static. Either by passing that as an argument to this function, or + // by setting a header on the response like the we do for full page RSC + // prefetches today. The latter approach might be simpler since it requires + // less plumbing, and the client has to check the header regardless to see if + // PPR is enabled. + const shouldAssumePartialData = + renderOpts.experimental.isRoutePPREnabled === true && // PPR is enabled + !renderOpts.experimental.dynamicIO // dynamicIO is disabled + const staleTime = prerenderStore.stale return await ComponentMod.collectSegmentData( + shouldAssumePartialData, fullPageDataBuffer, staleTime, clientReferenceManifest.clientModules as ManifestNode, diff --git a/packages/next/src/server/app-render/collect-segment-data.tsx b/packages/next/src/server/app-render/collect-segment-data.tsx index c94e1221d6995..ef5d19d34b907 100644 --- a/packages/next/src/server/app-render/collect-segment-data.tsx +++ b/packages/next/src/server/app-render/collect-segment-data.tsx @@ -77,6 +77,7 @@ function onSegmentPrerenderError(error: unknown) { } export async function collectSegmentData( + shouldAssumePartialData: boolean, fullPageDataBuffer: Buffer, staleTime: number, clientModules: ManifestNode, @@ -120,6 +121,7 @@ export async function collectSegmentData( // inside of it, the side effects are transferred to the new stream. // @ts-expect-error renderSegmentPrefetch( + shouldAssumePartialData, buildId, seedData, key, @@ -306,6 +315,7 @@ async function collectSegmentDataImpl( } async function renderSegmentPrefetch( + shouldAssumePartialData: boolean, buildId: string, seedData: CacheNodeSeedData, key: string, @@ -321,7 +331,8 @@ async function renderSegmentPrefetch( buildId, rsc, loading, - isPartial: await isPartialRSCData(rsc, clientModules), + isPartial: + shouldAssumePartialData || (await isPartialRSCData(rsc, clientModules)), } // Since all we're doing is decoding and re-encoding a cached prerender, if // it takes longer than a microtask, it must because of hanging promises diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index 2b014ae46e634..4e6c8c703d052 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -183,6 +183,7 @@ export interface RenderOptsPartial { expireTime: ExpireTime | undefined clientTraceMetadata: string[] | undefined dynamicIO: boolean + clientSegmentCache: boolean inlineCss: boolean authInterrupts: boolean } diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index cd604095281a8..3c23f8af11522 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -592,6 +592,8 @@ export default abstract class Server< expireTime: this.nextConfig.expireTime, clientTraceMetadata: this.nextConfig.experimental.clientTraceMetadata, dynamicIO: this.nextConfig.experimental.dynamicIO ?? false, + clientSegmentCache: + this.nextConfig.experimental.clientSegmentCache ?? false, inlineCss: this.nextConfig.experimental.inlineCss ?? false, authInterrupts: !!this.nextConfig.experimental.authInterrupts, }, diff --git a/test/e2e/app-dir/ppr-navigations/simple/next.config.js b/test/e2e/app-dir/ppr-navigations/simple/next.config.js index 6013aed786290..1f8082a9ba645 100644 --- a/test/e2e/app-dir/ppr-navigations/simple/next.config.js +++ b/test/e2e/app-dir/ppr-navigations/simple/next.config.js @@ -1,5 +1,6 @@ module.exports = { experimental: { ppr: true, + clientSegmentCache: true, }, }