Support resuming a complete HTML prerender that has dynamic flight data

[gnoff]

followup to: #60645

Background

When prerendering the determination of whether a prerender is fully static or partially static should not be directly related to whether there is a postponed state or not. When rendering RSC it is possible to postpone because a dynamic API was used but then on the client (SSR) the postpone is never encountered. This can happen when a server component is passed to a client component and the client component conditionally renders the server component.

Today if this happens the entire output would be considered static when in fact the flight data encoded into the page and used for bootstrapping the client router contains dynamic holes. Today this is blocked by an error that incorrectly assumes that this case means the user caught the postpone in the client layer but as shown above this may not be the case.

Implementation

A more capable model is to think of the outcome of a prerender as having 3 possible states

1. Dynamic HTML: The HTML produces by the prerender has dynamic holes. we save the static prelude but expect to resume the render later to complete the HTML. This means we will resume the RSC part of the render as well
2. Dynamic Data: The HTML is completely static but the RSC data encoded into the page is dynamic. We don't want to resume the render but we do need to produce new inlined RSC data during a Request.
3. Static: The HTML is completely static and so is the RSC data encoded into the page. We save the entire HTML output and there will be no dynamic continuation when this route is visited.

Really 1 & 3 are the same as today (Partially static & Fully Static respectively) but case 2 which today errors in a confusing way is now supported.

In addition implementing the Dynamic Data case the old warning about catching postpones is removed. The reason we don't want this is that catching postpones is potentially a valid way to do optimistic UI. We probably want a first-party API for it at some point (and maybe we'll add the warning back in once we do) but imagine you do something dynamic like look up a user but during prerender you want to render as if the user is logged out. you could call getUser() in a try catch and render fallback UI if it throws. In this case we'd detect a dynamic API was used but we wouldn't have a corresponding postpone state which would put us in the Dynamic Data case (2).

Another item to note is that we can produce a fully static result even if there is a postponed state because users may call postpone themselves even if they are not calling dynamic APIs like headers or cookies. When this happens we don't want to statically capture a page with postponed boundaries in it. Instead we immediately resume the render and abort it with a postponed abort signal. This will cause the boundaries to immediately enter client render mode which should speed up recovery on the client.

Technical Note

Another note about the implementation is that you'll see that regardless of which case we are in, if there is a postponed state but we consider the page to be Dynamic Data meaning we want to serialize all the HTML and NOT do a resume in the dynamic continuation then we immediately resume the render with and already aborted AbortSignal. The purpose here is to mark any boundaries which have dynamic holes as being client-rendered.

As a general rule if the render produces a postponed state we must do one of the following

1. save the postponed state and ensure there is a dynamic continuation that calls resume
2. immediately resume the render and save the concatenated output and ensure the dynamic continuation does NOT call resume.

or said another way, every postponed state must be resumed (even if it didn't come from Next's dynamic APIs)

Perf considerations

This PR modifies a few key areas to improve perf.

Reduces quantity of *Stream instances where possible as these add significant overhead
Reduces extra closures to lower allocations and keep functions in monomorphic form where possible

Closes NEXT-2164

[ijjk]

Tests Passed

[gnoff]

This isn't really necessary anymore. There may be a legit use case to catch a postpone in the future, such as to support optimistic SSR UI. Perhaps that should have it's own sort of API in Next but since postponing itself is no longer an indication of whether something postponed it's no longer safe to assume any postponed result without a corresponding dynamic API call is an error and vice versa.

[ijjk]

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
buildDuration	11.7s	11.8s	N/A
buildDurationCached	6.4s	5s	N/A
nodeModulesSize	196 MB	196 MB	⚠️ +282 kB
nextStartRea..uration (ms)	425ms	425ms	✓

Client Bundles (main, webpack)

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
3f784ff6-HASH.js gzip	53.5 kB	53.5 kB	N/A
423.HASH.js gzip	185 B	181 B	N/A
68-HASH.js gzip	29.6 kB	29.7 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	238 B	240 B	N/A
main-HASH.js gzip	31.9 kB	31.9 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	✓
Overall change	46.9 kB	46.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
_app-HASH.js gzip	194 B	195 B	N/A
_error-HASH.js gzip	182 B	181 B	N/A
amp-HASH.js gzip	502 B	501 B	N/A
css-HASH.js gzip	320 B	322 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	255 B	256 B	N/A
head-HASH.js gzip	350 B	349 B	N/A
hooks-HASH.js gzip	368 B	369 B	N/A
image-HASH.js gzip	4.2 kB	4.2 kB	N/A
index-HASH.js gzip	257 B	256 B	N/A
link-HASH.js gzip	2.67 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	310 B	311 B	N/A
script-HASH.js gzip	384 B	383 B	N/A
withRouter-HASH.js gzip	306 B	308 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	106 B	106 B	✓

Client Build Manifests

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
_buildManifest.js gzip	483 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
index.html gzip	527 B	527 B	✓
link.html gzip	541 B	539 B	N/A
withRouter.html gzip	523 B	522 B	N/A
Overall change	527 B	527 B	✓

Edge SSR bundle Size

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
edge-ssr.js gzip	94.4 kB	94.5 kB	N/A
page.js gzip	150 kB	150 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
middleware-b..fest.js gzip	619 B	624 B	N/A
middleware-r..fest.js gzip	151 B	149 B	N/A
middleware.js gzip	47.4 kB	47.4 kB	N/A
edge-runtime..pack.js gzip	1.94 kB	1.94 kB	✓
Overall change	1.94 kB	1.94 kB	✓

Next Runtimes

	vercel/next.js canary	gnoff/next.js render-with-dynamic	Change
app-page-exp...dev.js gzip	166 kB	166 kB	N/A
app-page-exp..prod.js gzip	95.4 kB	95.4 kB	N/A
app-page-tur..prod.js gzip	97.2 kB	97.2 kB	N/A
app-page-tur..prod.js gzip	91.6 kB	91.6 kB	N/A
app-page.run...dev.js gzip	136 kB	136 kB	N/A
app-page.run..prod.js gzip	90.2 kB	90.2 kB	N/A
app-route-ex...dev.js gzip	22 kB	22 kB	N/A
app-route-ex..prod.js gzip	14.9 kB	14.9 kB	N/A
app-route-tu..prod.js gzip	14.9 kB	14.9 kB	N/A
app-route-tu..prod.js gzip	14.7 kB	14.6 kB	N/A
app-route.ru...dev.js gzip	21.7 kB	21.7 kB	N/A
app-route.ru..prod.js gzip	14.7 kB	14.6 kB	N/A
pages-api-tu..prod.js gzip	9.43 kB	9.43 kB	✓
pages-api.ru...dev.js gzip	9.7 kB	9.7 kB	✓
pages-api.ru..prod.js gzip	9.43 kB	9.43 kB	✓
pages-turbo...prod.js gzip	22 kB	22.1 kB	N/A
pages.runtim...dev.js gzip	22.7 kB	22.7 kB	N/A
pages.runtim..prod.js gzip	22 kB	22.1 kB	N/A
server.runti..prod.js gzip	49.9 kB	49.9 kB	N/A
Overall change	28.6 kB	28.6 kB	✓

Diff details

Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 68-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 app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: b455536

[ztanner]

wait until VoidRenderer meets foreverStream (I love the name choices in this PR 😆 )

[gnoff]

The space time continuum collapses and we discover if we really live in a simulation or not 😆

[ztanner]

should the ppr-full test suite have some updated assertions for what these new files are testing?

is this PR a good opportunity to test the case of passing something dynamic like headers or cookies through a context provider to ensure we only bail out of static generation at the use callsite?

[gnoff]

We need to change the implementation of cookies and headers first. right now we treat the invocation itself as dynamic not the read. We should do that but I don't want to add more to landing this effort

[ztanner]

would it make sense for postponeWithTracking to assertPostpone? I worry about having to remember to do both in all the relevant places

[ztanner]

When would I want to use this function instead of postponeWithTracking? Or is there really only one spot where we'd want to use this and that's the stream options we pass to the static renderer? I ask mostly because I'm wondering if it should be inlined rather than as a util

[gnoff]

The technique of aborting a render with a postpone probably won't happen elsewhere because it only really makes sense when you are trying to turn a prerender into a fully static result.

The abort signal is really only intended to be used when aborting a render whereas the postponeWithTracking is to create a hole in the current render. I could move it into app-render but i'd have to duplicate the postpone assertion logic and it sort of colocates all the postpone interfaces in a single place. It's probably not the right long term factoring but I don't know if moving it to app-render is either so I want to leave it here for now

[ztanner]

Suggested change

	* about how the handle the render. This function will drain the RSC reader and
	* about how to handle the render. This function will drain the RSC reader and

[wyattjoh]

This linting exemption should include a comment as to why

[wyattjoh]

This is only used below, maybe better to just embed it there? Having the callback site so far from this signal makes it hard to relate.

As the error render phases do not "postpone", maybe having the renderToStream just return encounteredDynamic: boolean | null might make it more semantic (and more easily extracted)?

[gnoff]

I think you had an old comment here. This is no longer part of the PR

[wyattjoh]

Instead, I'd prefer to use structure instead of a magic value that doesn't provide runtime guarantees:

diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
index ec6befc744..aa7c9dcce3 100644
--- a/packages/next/src/server/app-render/app-render.tsx
+++ b/packages/next/src/server/app-render/app-render.tsx
@@ -74,7 +74,11 @@ import { walkTreeWithFlightRouterState } from './walk-tree-with-flight-router-st
 import { createComponentTree } from './create-component-tree'
 import { getAssetQueryString } from './get-asset-query-string'
 import { setReferenceManifestsSingleton } from './action-encryption-utils'
-import { createStaticRenderer, DYNAMIC_DATA } from './static/static-renderer'
+import {
+  createStaticRenderer,
+  deserializePostponedState,
+  serializePostponedState,
+} from './static/static-renderer'
 import { DetachedPromise } from '../../lib/detached-promise'
 import { isDynamicServerError } from '../../client/components/hooks-server-context'
 import {
@@ -895,7 +899,7 @@ async function renderToHTMLOrFlightImpl(
         // If provided, the postpone state should be parsed as JSON so it can be
         // provided to React.
         postponed: renderOpts.postponed
-          ? JSON.parse(renderOpts.postponed)
+          ? deserializePostponedState(renderOpts.postponed)
           : null,
         streamOptions: {
           onError: htmlRendererErrorHandler,
@@ -953,7 +957,7 @@ async function renderToHTMLOrFlightImpl(
                 'Invariant: Prerender resulted in Dynamic HTML but no posptoned state was provided. This is a bug in Next.js'
               )
             }
-            metadata.postponed = JSON.stringify(postponed)
+            metadata.postponed = serializePostponedState(postponed)
             return { stream }
           }
 
@@ -987,7 +991,7 @@ async function renderToHTMLOrFlightImpl(
           // concatenate dynamic flight data to the static HTML on each request.
           // We serialize a special value for our postponed state that indicates the a later
           // dynamic render should skip the SSR resume and just append new inlined flight data
-          metadata.postponed = JSON.stringify(DYNAMIC_DATA)
+          metadata.postponed = serializePostponedState({ type: 'dynamic' })
           return {
             stream: await continueDynamicDataPrerender(stream, {
               getServerInsertedHTML: () =>
diff --git a/packages/next/src/server/app-render/static/static-renderer.ts b/packages/next/src/server/app-render/static/static-renderer.ts
index 7bf5210024..30af502496 100644
--- a/packages/next/src/server/app-render/static/static-renderer.ts
+++ b/packages/next/src/server/app-render/static/static-renderer.ts
@@ -4,9 +4,27 @@ import type {
 } from 'react-dom/server.edge'
 import type { Options as PrerenderOptions } from 'react-dom/static.edge'
 
+type ReactPostponedState = { type: 'react'; postponedState: object }
+type DynamicPostponedState = { type: 'dynamic' }
+type PostponedState = ReactPostponedState | DynamicPostponedState
+
+export function serializePostponedState(postponed: PostponedState) {
+  return JSON.stringify(postponed)
+}
+
+export function deserializePostponedState(
+  postponed: string
+): PostponedState | null {
+  try {
+    return JSON.parse(postponed)
+  } catch {
+    return null
+  }
+}
+
 type RenderResult = {
   stream: ReadableStream<Uint8Array>
-  postponed?: object | null
+  postponed?: PostponedState | null
   resumed?: boolean
 }
 
@@ -25,7 +43,12 @@ class StaticRenderer implements Renderer {
   public async render(children: JSX.Element) {
     const { prelude, postponed } = await this.prerender(children, this.options)
 
-    return { stream: prelude, postponed }
+    return {
+      stream: prelude,
+      postponed: postponed
+        ? ({ type: 'react', postponedState: postponed } as ReactPostponedState)
+        : null,
+    }
   }
 }
 
@@ -34,12 +57,16 @@ class StaticResumeRenderer implements Renderer {
     .resume as typeof import('react-dom/server.edge')['resume']
 
   constructor(
-    private readonly postponed: object,
+    private readonly postponed: ReactPostponedState,
     private readonly options: ResumeOptions
   ) {}
 
   public async render(children: JSX.Element) {
-    const stream = await this.resume(children, this.postponed, this.options)
+    const stream = await this.resume(
+      children,
+      this.postponed.postponedState,
+      this.options
+    )
 
     return { stream, resumed: true }
   }
@@ -89,8 +116,6 @@ type StreamOptions = Pick<
   | 'formState'
 >
 
-export const DYNAMIC_DATA = 1 as const
-
 type Options = {
   /**
    * Whether or not PPR is enabled. This is used to determine which renderer to
@@ -108,7 +133,7 @@ type Options = {
    * The postponed state for the render. This is only used when resuming a
    * prerender that has postponed.
    */
-  postponed: object | typeof DYNAMIC_DATA | null
+  postponed: PostponedState | null
 
   /**
    * The options for any of the renderers. This is a union of all the possible
@@ -144,7 +169,7 @@ export function createStaticRenderer({
       })
     }
 
-    if (postponed === DYNAMIC_DATA) {
+    if (postponed?.type === 'dynamic') {
       return new VoidRenderer()
     } else if (postponed) {
       return new StaticResumeRenderer(postponed, {

[gnoff]

What do you mean about runtime guarantees?

[wyattjoh]

Comments on this would be great! A bit odd on naming too, as it means I call:

await atLeastOneTask()

Which isn't super clear how/what it returns by the name. Maybe something like resolveImmediatelyAfter?

[gnoff]

It's named so that await atLeastOneTask() tells you semantically what you are trying to do. I want the code after this await to guaranteed be after the current microtask queue is flushed. This is also useful if you want to Promise.race something against the completion of the current Task.

added comments

[wyattjoh]

Fundamentally, this would be the same as writing:

Suggested change

	return new Promise(async (resolve, reject) => {
	try {
	while (true) {
	const { done } = await flightReader.read()
	if (done) {
	resolve()
	return
	}
	}
	} catch (error) {
	reject(error)
	}
	})
	while (true) {
	const { done } = await flightReader.read()
	if (done) {
	return
	}
	}

As the contents of the handler for the promise is executed sync, and the wrapping function is already marked as async.

[wyattjoh]

.pipeTo returns a Promise, which should have a .catch attached to it to avoid a UnhandledPromiseRejectionWarning.

Is there a reason why the above chainStreams doesn't do what's required? The upside to chainStreams is that it handles the case where the pipeTo rejects.

[wyattjoh]

I understand that this is clearing the array, but it would be good to comment that this is what we're doing here.

[wyattjoh]

NIT: Maybe cleaner to read if we early-exit?

Suggested change

	if (hasBytes) {
	const insertion = await insert()
	if (insertion) {
	controller.enqueue(encoder.encode(insertion))
	}
	}
	if (!hasBytes) return
	const insertion = await insert()
	if (!insertion) return
	controller.enqueue(encoder.encode(insertion))

[gnoff]

IMO it reads less clear this way. The existence of additional html here is unusual and we're only checking it for correctness / completeness

[wyattjoh]

NIT: Early returns here might make this easier to read:

Suggested change

	if (!pull) {
	pull = startPulling(controller)
	}
	if (pull) return
	pull = startPulling(controller)

[gnoff]

If this code grows to such a complexity that an early return offers clarity by scoping the environment to some invariant I'm all for it but it's so trivial I don't think it reads any clearer