Repository: vercel/next.js
Pull request: [Cache Components] Reset PrerenderStore after generating RSC payload

The RSC payload ideally would be constructed during the prerender. However it is not currently sync and RSC doesn't provide a top level way to produce an object child from a function at the root so we generate the component tree before starting the render.

However we use the workUnitStore type to govern certain behaviors within the RSC payload generation for instance preparing things like searchParams and params props for layouts and pages.

So at the moment we scope the RSC generation inside the same workUnitStore instance that will also scope the prerender. This is fine except there are certain things that apply in the prerender context that do not apply in the component tree generation. Namely we do not want to track Sync IO as need to abort the prerender. This would lead to the entire prerender being aborted before it even begins.

There are a few options. We could create a new workUnitStore type. This isn't great b/c we have so many places where we need to exhaustively check this.

We could use a new phase to similarly track whether we're before render or not however this has the same problem of all places where phase is considered needing to be aware of this additional possible phase.

Instead I think we ought to leave the logic of the code that determines what to do simple and continue to track the sync IO but we just reset the dynamic tracking and abort controllers so that the render starts afresh with these.

Changed files:
filename: packages/next/src/server/app-render/app-render.tsx
patch: @@ -2660,14 +2660,19 @@ async function spawnDynamicValidationInDev(
   // prerendering of an already prerendered page, we should use the passed
   // resume data cache instead.
   const prerenderResumeDataCache = createPrerenderResumeDataCache()
-  const initialServerPrerenderStore: PrerenderStore = {
+  const initialServerPayloadPrerenderStore: PrerenderStore = {
     type: 'prerender',
     phase: 'render',
     rootParams,
     fallbackRouteParams,
     implicitTags,
+    // While this render signal isn't going to be used to abort a React render while getting the RSC payload
+    // various request data APIs bind to this controller to reject after completion.
     renderSignal: initialServerRenderController.signal,
-    controller: initialServerPrerenderController,
+    // When we generate the RSC payload we might abort this controller due to sync IO
+    // but we don't actually care about sync IO in this phase so we use a throw away controller
+    // that isn't connected to anything
+    controller: new AbortController(),
     // During the initial prerender we need to track all cache reads to ensure
     // we render long enough to fill every cache it is possible to visit during
     // the final prerender.
@@ -2687,13 +2692,37 @@ async function spawnDynamicValidationInDev(
   // We're not going to use the result of this render because the only time it could be used
   // is if it completes in a microtask and that's likely very rare for any non-trivial app
   const initialServerPayload = await workUnitAsyncStorage.run(
-    initialServerPrerenderStore,
+    initialServerPayloadPrerenderStore,
     getRSCPayload,
     tree,
     ctx,
     isNotFound
   )
 
+  const initialServerPrerenderStore: PrerenderStore = {
+    type: 'prerender',
+    phase: 'render',
+    rootParams,
+    fallbackRouteParams,
+    implicitTags,
+    renderSignal: initialServerRenderController.signal,
+    controller: initialServerPrerenderController,
+    // During the initial prerender we need to track all cache reads to ensure
+    // we render long enough to fill every cache it is possible to visit during
+    // the final prerender.
+    cacheSignal,
+    dynamicTracking: null,
+    allowEmptyStaticShell,
+    revalidate: INFINITE_CACHE,
+    expire: INFINITE_CACHE,
+    stale: INFINITE_CACHE,
+    tags: [...implicitTags.tags],
+    prerenderResumeDataCache,
+    renderResumeDataCache: null,
+    hmrRefreshHash,
+    captureOwnerStack: captureOwnerStackServer,
+  }
+
   const pendingInitialServerResult = workUnitAsyncStorage.run(
     initialServerPrerenderStore,
     ComponentMod.prerender,
@@ -2897,21 +2926,22 @@ async function spawnDynamicValidationInDev(
   const finalServerReactController = new AbortController()
   const finalServerRenderController = new AbortController()
 
-  const serverDynamicTracking = createDynamicTrackingState(
-    false // isDebugDynamicAccesses
-  )
-
-  const finalServerPrerenderStore: PrerenderStore = {
+  const finalServerPayloadPrerenderStore: PrerenderStore = {
     type: 'prerender',
     phase: 'render',
     rootParams,
     fallbackRouteParams,
     implicitTags,
+    // While this render signal isn't going to be used to abort a React render while getting the RSC payload
+    // various request data APIs bind to this controller to reject after completion.
     renderSignal: finalServerRenderController.signal,
-    controller: finalServerReactController,
+    // When we generate the RSC payload we might abort this controller due to sync IO
+    // but we don't actually care about sync IO in this phase so we use a throw away controller
+    // that isn't connected to anything
+    controller: new AbortController(),
     // All caches we could read must already be filled so no tracking is necessary
     cacheSignal: null,
-    dynamicTracking: serverDynamicTracking,
+    dynamicTracking: null,
     allowEmptyStaticShell,
     revalidate: INFINITE_CACHE,
     expire: INFINITE_CACHE,
@@ -2924,13 +2954,39 @@ async function spawnDynamicValidationInDev(
   }
 
   const finalAttemptRSCPayload = await workUnitAsyncStorage.run(
-    finalServerPrerenderStore,
+    finalServerPayloadPrerenderStore,
     getRSCPayload,
     tree,
     ctx,
     isNotFound
   )
 
+  const serverDynamicTracking = createDynamicTrackingState(
+    false // isDebugDynamicAccesses
+  )
+
+  const finalServerPrerenderStore: PrerenderStore = {
+    type: 'prerender',
+    phase: 'render',
+    rootParams,
+    fallbackRouteParams,
+    implicitTags,
+    renderSignal: finalServerRenderController.signal,
+    controller: finalServerReactController,
+    // All caches we could read must already be filled so no tracking is necessary
+    cacheSignal: null,
+    dynamicTracking: serverDynamicTracking,
+    allowEmptyStaticShell,
+    revalidate: INFINITE_CACHE,
+    expire: INFINITE_CACHE,
+    stale: INFINITE_CACHE,
+    tags: [...implicitTags.tags],
+    prerenderResumeDataCache,
+    renderResumeDataCache: null,
+    hmrRefreshHash,
+    captureOwnerStack: captureOwnerStackServer,
+  }
+
   const reactServerResult = await createReactServerPrerenderResult(
     prerenderAndAbortInSequentialTasks(
       async () => {
@@ -3354,14 +3410,19 @@ async function prerenderToStream(
           createPrerenderResumeDataCache()
       }
 
-      const initialServerPrerenderStore: PrerenderStore = (prerenderStore = {
+      const initialServerPayloadPrerenderStore: PrerenderStore = {
         type: 'prerender',
         phase: 'render',
         rootParams,
         fallbackRouteParams,
         implicitTags,
+        // While this render signal isn't going to be used to abort a React render while getting the RSC payload
+        // various request data APIs bind to this controller to reject after completion.
         renderSignal: initialServerRenderController.signal,
-        controller: initialServerPrerenderController,
+        // When we generate the RSC payload we might abort this controller due to sync IO
+        // but we don't actually care about sync IO in this phase so we use a throw away controller
+        // that isn't connected to anything
+        controller: new AbortController(),
         // During the initial prerender we need to track all cache reads to ensure
         // we render long enough to fill every cache it is possible to visit during
         // the final prerender.
@@ -3376,18 +3437,42 @@ async function prerenderToStream(
         renderResumeDataCache,
         hmrRefreshHash: undefined,
         captureOwnerStack: undefined, // Not available in production.
-      })
+      }
 
       // We're not going to use the result of this render because the only time it could be used
       // is if it completes in a microtask and that's likely very rare for any non-trivial app
       const initialServerPayload = await workUnitAsyncStorage.run(
-        initialServerPrerenderStore,
+        initialServerPayloadPrerenderStore,
         getRSCPayload,
         tree,
         ctx,
         res.statusCode === 404
       )
 
+      const initialServerPrerenderStore: PrerenderStore = (prerenderStore = {
+        type: 'prerender',
+        phase: 'render',
+        rootParams,
+        fallbackRouteParams,
+        implicitTags,
+        renderSignal: initialServerRenderController.signal,
+        controller: initialServerPrerenderController,
+        // During the initial prerender we need to track all cache reads to ensure
+        // we render long enough to fill every cache it is possible to visit during
+        // the final prerender.
+        cacheSignal,
+        dynamicTracking: null,
+        allowEmptyStaticShell,
+        revalidate: INFINITE_CACHE,
+        expire: INFINITE_CACHE,
+        stale: INFINITE_CACHE,
+        tags: [...implicitTags.tags],
+        prerenderResumeDataCache,
+        renderResumeDataCache,
+        hmrRefreshHash: undefined,
+        captureOwnerStack: undefined, // Not available in production.
+      })
+
       const pendingInitialServerResult = workUnitAsyncStorage.run(
         initialServerPrerenderStore,
         ComponentMod.prerender,
@@ -3580,13 +3665,48 @@ async function prerenderToStream(
         initialClientReactController.abort()
       }
 
-      let serverIsDynamic = false
       const finalServerReactController = new AbortController()
       const finalServerRenderController = new AbortController()
 
+      const finalServerPayloadPrerenderStore: PrerenderStore = {
+        type: 'prerender',
+        phase: 'render',
+        rootParams,
+        fallbackRouteParams,
+        implicitTags,
+        // While this render signal isn't going to be used to abort a React render while getting the RSC payload
+        // various request data APIs bind to this controller to reject after completion.
+        renderSignal: finalServerRenderController.signal,
+        // When we generate the RSC payload we might abort this controller due to sync IO
+        // but we don't actually care about sync IO in this phase so we use a throw away controller
+        // that isn't connected to anything
+        controller: new AbortController(),
+        // All caches we could read must already be filled so no tracking is necessary
+        cacheSignal: null,
+        dynamicTracking: null,
+        allowEmptyStaticShell,
+        revalidate: INFINITE_CACHE,
+        expire: INFINITE_CACHE,
+        stale: INFINITE_CACHE,
+        tags: [...implicitTags.tags],
+        prerenderResumeDataCache,
+        renderResumeDataCache,
+        hmrRefreshHash: undefined,
+        captureOwnerStack: undefined, // Not available in production.
+      }
+
+      const finalAttemptRSCPayload = await workUnitAsyncStorage.run(
+        finalServerPayloadPrerenderStore,
+        getRSCPayload,
+        tree,
+        ctx,
+        res.statusCode === 404
+      )
+
       const serverDynamicTracking = createDynamicTrackingState(
         isDebugDynamicAccesses
       )
+      let serverIsDynamic = false
 
       const finalServerPrerenderStore: PrerenderStore = (prerenderStore = {
         type: 'prerender',
@@ -3610,13 +3730,6 @@ async function prerenderToStream(
         captureOwnerStack: undefined, // Not available in production.
       })
 
-      const finalAttemptRSCPayload = await workUnitAsyncStorage.run(
-        finalServerPrerenderStore,
-        getRSCPayload,
-        tree,
-        ctx,
-        res.statusCode === 404
-      )
       let prerenderIsPending = true
       const reactServerResult = (reactServerPrerenderResult =
         await createReactServerPrerenderResult(

filename: test/e2e/app-dir/cache-components-create-component-tree/app/layout.tsx
patch: @@ -0,0 +1,38 @@
+/**
+ * This is a rather contrived test. The trick is to construct a scenario where
+ * Math.random is called during the create-component-tree phase but not during the render.
+ * To do this we patch Symbol.for. When the `react.client.reference` symbol is checked
+ * we know that this is happening in the create-component-tree function. Given the simple
+ * setup of the test page this symbol is not checked during the actual prerender itself.
+ *
+ * This test may flake in the future. It should be replaced ideally with a test that uses
+ * our OTel implementation to ensure that the built-in trace in create-component-tree does not
+ * early abort the prerender. The problem is that we are about to make these functions avoid the
+ * workUnitStore scope so it will not express the regression for other reasons.
+ */
+function patchSymbolFor() {
+  const isPatched = globalThis[Symbol.for('__SYMBOL_FOR_PATCHED__')]
+  if (!isPatched) {
+    console.log('patching')
+    globalThis[Symbol.for('__SYMBOL_FOR_PATCHED__')] = true
+
+    const originalSymbolFor = Symbol.for
+
+    Symbol.for = (...args) => {
+      if (args[0] === 'react.client.reference') {
+        Math.random()
+      }
+      return originalSymbolFor.apply(Symbol, args)
+    }
+  }
+}
+
+patchSymbolFor()
+
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

filename: test/e2e/app-dir/cache-components-create-component-tree/app/page.tsx
patch: @@ -0,0 +1,20 @@
+export default function Page() {
+  return (
+    <>
+      <p>
+        This test does some hacky stuff to cause a `Math.random()` call to
+        happen during the component tree creation which happens before the RSC
+        render. Before the fix that this test accompanied landed this kind of
+        sync IO was reported as a problem in Dev and in runtime during start. In
+        practice this would be because of an OTEL span generation during
+        create-component-tree. With this fix in place the create-component-tree
+        function will not leak any sync IO results into the final prerender.
+      </p>
+      <p>
+        In the long run we ought to move create-component-tree into the render
+        itself and ensure the tree structure comports with CacheComponent rules.
+        This test can be deleted at that time.
+      </p>
+    </>
+  )
+}

filename: test/e2e/app-dir/cache-components-create-component-tree/cache-components-create-component-tree.test.ts
patch: @@ -0,0 +1,31 @@
+import { isNextDev, nextTestSetup } from 'e2e-utils'
+import { assertNoRedbox } from 'next-test-utils'
+
+describe('hello-world', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+    skipStart: !isNextDev,
+    skipDeployment: true,
+  })
+
+  if (isNextDev) {
+    it('should not indicate there is an error when incidental math.random calls occur during component tree generation during dev', async () => {
+      const browser = await next.browser('/')
+      await assertNoRedbox(browser)
+
+      // The redbox assertion is currently unreliable in this test and so this is an additional check to ensure the CLI didn't print anything with `Math.random()` in it.
+      expect(next.cliOutput).not.toContain('Math.random()')
+    })
+  } else {
+    it('should not indicate there is an error when incidental math.random calls occur during component tree generation during build', async () => {
+      try {
+        await next.build()
+      } catch (e) {
+        console.error('Expected build to Succeed, but it failed.')
+        throw e
+      }
+
+      expect(next.cliOutput).not.toContain('Math.random()')
+    })
+  }
+})

filename: test/e2e/app-dir/cache-components-create-component-tree/next.config.js
patch: @@ -0,0 +1,10 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {
+  experimental: {
+    cacheComponents: true,
+  },
+}
+
+module.exports = nextConfig