Refactor require hooks (#48506)

Same purpose as #48297, but without the React channel branching logic to make it easier to land. Since we have #48478 reverted, we only need to consider `pages` and `app` inside the require hook. > This PR aims to improve the current require hook by implementing two key changes. Firstly, it ensures that the initialization occurs at the top of the module level for correctness. Secondly, we now set the NEXT_PREBUNDLED_REACT environment variable at the process level to ensure that we don't mix the two types of rendering processes and we always resolve the correct React package. > > These improvements are made possible by the changes introduced in PR #47857. > > Closes [NEXT-231](https://linear.app/vercel/issue/NEXT-231). This will likely fix #45258 too. --------- Co-authored-by: JJ Kasper <[email protected]>
vercel · Apr 21, 2023 · b5f7f84 · b5f7f84
1 parent 6423285
commit b5f7f84
Show file tree

Hide file tree

Showing 35 changed files with 345 additions and 306 deletions.
diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -280,30 +280,10 @@ export default async function build(
 
       const publicDir = path.join(dir, 'public')
       const isAppDirEnabled = !!config.experimental.appDir
-      const initialRequireHookFilePath = require.resolve(
-        'next/dist/server/initialize-require-hook'
-      )
-      const content = await promises.readFile(
-        initialRequireHookFilePath,
-        'utf8'
-      )
 
       if (isAppDirEnabled) {
         process.env.NEXT_PREBUNDLED_REACT = '1'
       }
-      await promises
-        .writeFile(
-          initialRequireHookFilePath,
-          content.replace(
-            /isPrebundled = (true|false)/,
-            `isPrebundled = ${isAppDirEnabled}`
-          )
-        )
-        .catch((err) => {
-          if (isAppDirEnabled) {
-            throw err
-          }
-        })
 
       const { pagesDir, appDir } = findPagesDir(dir, isAppDirEnabled)
       NextBuildContext.pagesDir = pagesDir
@@ -1067,10 +1047,9 @@ export default async function build(
 
       const timeout = config.staticPageGenerationTimeout || 0
       const sharedPool = config.experimental.sharedPool || false
-      const staticWorker = sharedPool
+      const staticWorkerPath = sharedPool
         ? require.resolve('./worker')
         : require.resolve('./utils')
-      let infoPrinted = false
 
       let appPathsManifest: Record<string, string> = {}
       const appPathRoutes: Record<string, string> = {}
@@ -1111,69 +1090,89 @@ export default async function build(
         4
       )
 
-      const staticWorkers = new Worker(staticWorker, {
-        timeout: timeout * 1000,
-        onRestart: (method, [arg], attempts) => {
-          if (method === 'exportPage') {
-            const { path: pagePath } = arg
-            if (attempts >= 3) {
-              throw new Error(
-                `Static page generation for ${pagePath} is still timing out after 3 attempts. See more info here https://nextjs.org/docs/messages/static-page-generation-timeout`
+      function createStaticWorker(type: 'app' | 'pages') {
+        const numWorkersPerType = isAppDirEnabled
+          ? Math.max(1, ~~(numWorkers / 2))
+          : numWorkers
+
+        let infoPrinted = false
+
+        return new Worker(staticWorkerPath, {
+          timeout: timeout * 1000,
+          onRestart: (method, [arg], attempts) => {
+            if (method === 'exportPage') {
+              const { path: pagePath } = arg
+              if (attempts >= 3) {
+                throw new Error(
+                  `Static page generation for ${pagePath} is still timing out after 3 attempts. See more info here https://nextjs.org/docs/messages/static-page-generation-timeout`
+                )
+              }
+              Log.warn(
+                `Restarted static page generation for ${pagePath} because it took more than ${timeout} seconds`
+              )
+            } else {
+              const pagePath = arg
+              if (attempts >= 2) {
+                throw new Error(
+                  `Collecting page data for ${pagePath} is still timing out after 2 attempts. See more info here https://nextjs.org/docs/messages/page-data-collection-timeout`
+                )
+              }
+              Log.warn(
+                `Restarted collecting page data for ${pagePath} because it took more than ${timeout} seconds`
               )
             }
-            Log.warn(
-              `Restarted static page generation for ${pagePath} because it took more than ${timeout} seconds`
-            )
-          } else {
-            const pagePath = arg
-            if (attempts >= 2) {
-              throw new Error(
-                `Collecting page data for ${pagePath} is still timing out after 2 attempts. See more info here https://nextjs.org/docs/messages/page-data-collection-timeout`
+            if (!infoPrinted) {
+              Log.warn(
+                'See more info here https://nextjs.org/docs/messages/static-page-generation-timeout'
               )
+              infoPrinted = true
             }
-            Log.warn(
-              `Restarted collecting page data for ${pagePath} because it took more than ${timeout} seconds`
-            )
-          }
-          if (!infoPrinted) {
-            Log.warn(
-              'See more info here https://nextjs.org/docs/messages/static-page-generation-timeout'
-            )
-            infoPrinted = true
-          }
-        },
-        numWorkers,
-        enableWorkerThreads: config.experimental.workerThreads,
-        computeWorkerKey(method, ...args) {
-          if (method === 'exportPage') {
-            const typedArgs = args as Parameters<
-              typeof import('./worker').exportPage
-            >
-            return typedArgs[0].pathMap.page
-          } else if (method === 'isPageStatic') {
-            const typedArgs = args as Parameters<
-              typeof import('./worker').isPageStatic
-            >
-            return typedArgs[0].originalAppPath || typedArgs[0].page
-          }
-          return method
-        },
-        exposedMethods: sharedPool
-          ? [
-              'hasCustomGetInitialProps',
-              'isPageStatic',
-              'getNamedExports',
-              'exportPage',
-            ]
-          : ['hasCustomGetInitialProps', 'isPageStatic', 'getNamedExports'],
-      }) as Worker &
-        Pick<
-          typeof import('./worker'),
-          | 'hasCustomGetInitialProps'
-          | 'isPageStatic'
-          | 'getNamedExports'
-          | 'exportPage'
-        >
+          },
+          numWorkers: numWorkersPerType,
+          forkOptions: {
+            env: {
+              ...process.env,
+              NEXT_PREBUNDLED_REACT_WORKER: type === 'app' ? '1' : '',
+              __NEXT_PRIVATE_PREBUNDLED_REACT: type === 'app' ? '1' : '',
+            },
+          },
+          enableWorkerThreads: config.experimental.workerThreads,
+          computeWorkerKey(method, ...args) {
+            if (method === 'exportPage') {
+              const typedArgs = args as Parameters<
+                typeof import('./worker').exportPage
+              >
+              return typedArgs[0].pathMap.page
+            } else if (method === 'isPageStatic') {
+              const typedArgs = args as Parameters<
+                typeof import('./worker').isPageStatic
+              >
+              return typedArgs[0].originalAppPath || typedArgs[0].page
+            }
+            return method
+          },
+          exposedMethods: sharedPool
+            ? [
+                'hasCustomGetInitialProps',
+                'isPageStatic',
+                'getNamedExports',
+                'exportPage',
+              ]
+            : ['hasCustomGetInitialProps', 'isPageStatic', 'getNamedExports'],
+        }) as Worker &
+          Pick<
+            typeof import('./worker'),
+            | 'hasCustomGetInitialProps'
+            | 'isPageStatic'
+            | 'getNamedExports'
+            | 'exportPage'
+          >
+      }
+
+      const pagesStaticWorkers = createStaticWorker('pages')
+      const appStaticWorkers = isAppDirEnabled
+        ? createStaticWorker('app')
+        : undefined
 
       const analysisBegin = process.hrtime()
       const staticCheckSpan = nextBuildSpan.traceChild('static-check')
@@ -1195,7 +1194,7 @@ export default async function build(
           nonStaticErrorPageSpan.traceAsyncFn(
             async () =>
               hasCustomErrorPage &&
-              (await staticWorkers.hasCustomGetInitialProps(
+              (await pagesStaticWorkers.hasCustomGetInitialProps(
                 '/_error',
                 distDir,
                 runtimeEnvConfig,
@@ -1206,7 +1205,7 @@ export default async function build(
         const errorPageStaticResult = nonStaticErrorPageSpan.traceAsyncFn(
           async () =>
             hasCustomErrorPage &&
-            staticWorkers.isPageStatic({
+            pagesStaticWorkers.isPageStatic({
               page: '/_error',
               distDir,
               configFileName,
@@ -1222,14 +1221,14 @@ export default async function build(
         const appPageToCheck = '/_app'
 
         const customAppGetInitialPropsPromise =
-          staticWorkers.hasCustomGetInitialProps(
+          pagesStaticWorkers.hasCustomGetInitialProps(
             appPageToCheck,
             distDir,
             runtimeEnvConfig,
             true
           )
 
-        const namedExportsPromise = staticWorkers.getNamedExports(
+        const namedExportsPromise = pagesStaticWorkers.getNamedExports(
           appPageToCheck,
           distDir,
           runtimeEnvConfig
@@ -1385,7 +1384,11 @@ export default async function build(
                       checkPageSpan.traceChild('is-page-static')
                     let workerResult = await isPageStaticSpan.traceAsyncFn(
                       () => {
-                        return staticWorkers.isPageStatic({
+                        return (
+                          pageType === 'app'
+                            ? appStaticWorkers
+                            : pagesStaticWorkers
+                        )!.isPageStatic({
                           page,
                           originalAppPath,
                           distDir,
@@ -1621,7 +1624,11 @@ export default async function build(
           hasNonStaticErrorPage: nonStaticErrorPage,
         }
 
-        if (!sharedPool) staticWorkers.end()
+        if (!sharedPool) {
+          pagesStaticWorkers.end()
+          appStaticWorkers?.end()
+        }
+
         return returnValue
       })
 
@@ -2290,12 +2297,16 @@ export default async function build(
             pages: combinedPages,
             outdir: path.join(distDir, 'export'),
             statusMessage: 'Generating static pages',
+            exportAppPageWorker: sharedPool
+              ? appStaticWorkers?.exportPage.bind(appStaticWorkers)
+              : undefined,
             exportPageWorker: sharedPool
-              ? staticWorkers.exportPage.bind(staticWorkers)
+              ? pagesStaticWorkers.exportPage.bind(pagesStaticWorkers)
               : undefined,
             endWorker: sharedPool
               ? async () => {
-                  await staticWorkers.end()
+                  await pagesStaticWorkers.end()
+                  await appStaticWorkers?.end()
                 }
               : undefined,
           }
@@ -2723,7 +2734,8 @@ export default async function build(
       }
 
       // ensure the worker is not left hanging
-      staticWorkers.close()
+      pagesStaticWorkers.close()
+      appStaticWorkers?.close()
 
       const analysisEnd = process.hrtime(analysisBegin)
       telemetry.record(

diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts
@@ -14,6 +14,7 @@ import type { MiddlewareManifest } from './webpack/plugins/middleware-plugin'
 import type { AppRouteUserlandModule } from '../server/future/route-modules/app-route/module'
 import type { StaticGenerationAsyncStorage } from '../client/components/static-generation-async-storage'
 
+import '../server/require-hook'
 import '../server/node-polyfill-fetch'
 import chalk from 'next/dist/compiled/chalk'
 import getGzipSize from 'next/dist/compiled/gzip-size'
@@ -52,21 +53,12 @@ import { Sema } from 'next/dist/compiled/async-sema'
 import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
 import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
 import { getRuntimeContext } from '../server/web/sandbox'
-import {
-  loadRequireHook,
-  overrideBuiltInReactPackages,
-} from './webpack/require-hook'
 import { isClientReference } from '../lib/client-reference'
 import { StaticGenerationAsyncStorageWrapper } from '../server/async-storage/static-generation-async-storage-wrapper'
 import { IncrementalCache } from '../server/lib/incremental-cache'
 import { patchFetch } from '../server/lib/patch-fetch'
 import { nodeFs } from '../server/lib/node-fs-methods'
 
-loadRequireHook()
-if (process.env.NEXT_PREBUNDLED_REACT) {
-  overrideBuiltInReactPackages()
-}
-
 // expose AsyncLocalStorage on global for react usage
 const { AsyncLocalStorage } = require('async_hooks')
 ;(globalThis as any).AsyncLocalStorage = AsyncLocalStorage