Avoid unnecessary resolveExternal calls (#52053)

vercel · Jul 3, 2023 · 8eb9730 · 8eb9730
1 parent 967b876
commit 8eb9730
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 58 deletions.
diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -1425,6 +1425,64 @@ export default async function getBaseWebpackConfig(
       }
     }
 
+    // Don't bundle @vercel/og nodejs bundle for nodejs runtime.
+    // TODO-APP: bundle route.js with different layer that externals common node_module deps.
+    if (
+      layer === WEBPACK_LAYERS.server &&
+      request === 'next/dist/compiled/@vercel/og/index.node.js'
+    ) {
+      return `module ${request}`
+    }
+
+    // Specific Next.js imports that should remain external
+    // TODO-APP: Investigate if we can remove this.
+    if (request.startsWith('next/dist/')) {
+      // Image loader needs to be transpiled
+      if (/^next\/dist\/shared\/lib\/image-loader/.test(request)) {
+        return
+      }
+
+      if (
+        /^next\/dist\/shared\/(?!lib\/router\/router)/.test(request) ||
+        /^next\/dist\/compiled\/.*\.c?js$/.test(request)
+      ) {
+        return `commonjs ${request}`
+      }
+
+      if (
+        /^next\/dist\/esm\/shared\/(?!lib\/router\/router)/.test(request) ||
+        /^next\/dist\/compiled\/.*\.mjs$/.test(request)
+      ) {
+        return `module ${request}`
+      }
+
+      // Other Next.js internals need to be transpiled.
+      return
+    }
+
+    // Early return if the request needs to be bundled, such as in the client layer.
+    // Treat react packages and next internals as external for SSR layer,
+    // also map react to builtin ones with require-hook.
+    if (layer === WEBPACK_LAYERS.client) {
+      if (reactPackagesRegex.test(request)) {
+        return `commonjs next/dist/compiled/${request.replace(
+          /^(react-server-dom-webpack|react-dom|react)/,
+          (name) => {
+            return name + bundledReactChannel
+          }
+        )}`
+      }
+
+      const isRelative = request.startsWith('.')
+      const fullRequest = isRelative
+        ? path.join(context, request).replace(/\\/g, '/')
+        : request
+      const resolveNextExternal = isLocalCallback(fullRequest)
+
+      return resolveNextExternal
+    }
+
+    // TODO-APP: Let's avoid this resolve call as much as possible, and eventually get rid of it.
     const resolveResult = await resolveExternal(
       dir,
       config.experimental.esmExternals,
@@ -1446,15 +1504,6 @@ export default async function getBaseWebpackConfig(
       resolveResult.res = require.resolve(request)
     }
 
-    // Don't bundle @vercel/og nodejs bundle for nodejs runtime.
-    // TODO-APP: bundle route.js with different layer that externals common node_module deps.
-    if (
-      layer === WEBPACK_LAYERS.server &&
-      request === 'next/dist/compiled/@vercel/og/index.node.js'
-    ) {
-      return `module ${request}`
-    }
-
     const { res, isEsm } = resolveResult
 
     // If the request cannot be resolved we need to have
@@ -1473,18 +1522,8 @@ export default async function getBaseWebpackConfig(
 
     const externalType = isEsm ? 'module' : 'commonjs'
 
-    if (
-      /next[/\\]dist[/\\](esm[\\/])?shared[/\\](?!lib[/\\]router[/\\]router)/.test(
-        res
-      ) ||
-      /next[/\\]dist[/\\]compiled[/\\].*\.[mc]?js$/.test(res)
-    ) {
-      return `${externalType} ${request}`
-    }
-
     // Default pages have to be transpiled
     if (
-      /[/\\]next[/\\]dist[/\\]/.test(res) ||
       // This is the @babel/plugin-transform-runtime "helpers: true" option
       /node_modules[/\\]@babel[/\\]runtime[/\\]/.test(res)
     ) {
@@ -1544,20 +1583,6 @@ export default async function getBaseWebpackConfig(
         return
       }
 
-      // Treat react packages and next internals as external for SSR layer,
-      // also map react to builtin ones with require-hook.
-      if (layer === WEBPACK_LAYERS.client) {
-        if (reactPackagesRegex.test(request)) {
-          return `commonjs next/dist/compiled/${request.replace(
-            /^(react-server-dom-webpack|react-dom|react)/,
-            (name) => {
-              return name + bundledReactChannel
-            }
-          )}`
-        }
-        return
-      }
-
       if (shouldBeBundled) return
 
       // Anything else that is standard JavaScript within `node_modules`

diff --git a/test/e2e/app-dir/app-external/app-external.test.ts b/test/e2e/app-dir/app-external/app-external.test.ts
@@ -214,31 +214,6 @@ createNextDescribe(
       expect(html).toContain('Foo')
     })
 
-    if (isNextDev) {
-      it('should error for require ESM package in CJS package', async () => {
-        const page = 'app/cjs-import-esm/page.js'
-        // reuse esm-client-ref/page.js
-        const pageSource = await next.readFile('app/esm-client-ref/page.js')
-
-        try {
-          await next.patchFile(
-            page,
-            pageSource.replace(
-              "'client-esm-module'",
-              "'client-cjs-import-esm-wildcard'"
-            )
-          )
-          await next.render('/cjs-import-esm')
-        } finally {
-          await next.patchFile(page, pageSource)
-        }
-
-        expect(next.cliOutput).toInclude(
-          `ESM packages (client-esm-module-wildcard) need to be imported`
-        )
-      })
-    }
-
     it('should have proper tree-shaking for known modules in CJS', async () => {
       const html = await next.render('/test-middleware')
       expect(html).toContain('it works')