Fix server output bundling packages module resolving (#59369)

Author: huozhi

packages/next/src/build/handle-externals.ts

@@ -47,6 +47,7 @@ export async function resolveExternal(
   context: string,
   request: string,
   isEsmRequested: boolean,
+  optOutBundlingPackages: string[],
   getResolve: (
     options: any
   ) => (
@@ -66,8 +67,15 @@ export async function resolveExternal(
   let res: string | null = null
   let isEsm: boolean = false
 
-  let preferEsmOptions =
-    esmExternals && isEsmRequested ? [true, false] : [false]
+  const preferEsmOptions =
+    esmExternals &&
+    isEsmRequested &&
+    // For package that marked as externals that should be not bundled,
+    // we don't resolve them as ESM since it could be resolved as async module,
+    // such as `import(external package)` in the bundle, valued as a `Promise`.
+    !optOutBundlingPackages.some((optOut) => request.startsWith(optOut))
+      ? [true, false]
+      : [false]
 
   for (const preferEsm of preferEsmOptions) {
     const resolve = getResolve(
@@ -131,10 +139,12 @@ export async function resolveExternal(
 
 export function makeExternalHandler({
   config,
+  optOutBundlingPackages,
   optOutBundlingPackageRegex,
   dir,
 }: {
   config: NextConfigComplete
+  optOutBundlingPackages: string[]
   optOutBundlingPackageRegex: RegExp
   dir: string
 }) {
@@ -289,6 +299,7 @@ export function makeExternalHandler({
       context,
       request,
       isEsmRequested,
+      optOutBundlingPackages,
       getResolve,
       isLocal ? resolveNextExternal : undefined
     )
@@ -349,6 +360,7 @@ export function makeExternalHandler({
           context,
           pkg + '/package.json',
           isEsmRequested,
+          optOutBundlingPackages,
           getResolve,
           isLocal ? resolveNextExternal : undefined
         )

packages/next/src/build/webpack-config.ts

@@ -727,9 +727,11 @@ export default async function getBaseWebpackConfig(
 
   const crossOrigin = config.crossOrigin
 
+  // For original request, such as `package name`
   const optOutBundlingPackages = EXTERNAL_PACKAGES.concat(
     ...(config.experimental.serverComponentsExternalPackages || [])
   )
+  // For resolved request, such as `absolute path/package name/foo/bar.js`
   const optOutBundlingPackageRegex = new RegExp(
     `[/\\\\]node_modules[/\\\\](${optOutBundlingPackages
       .map((p) => p.replace(/\//g, '[/\\\\]'))
@@ -738,6 +740,7 @@ export default async function getBaseWebpackConfig(
 
   const handleExternals = makeExternalHandler({
     config,
+    optOutBundlingPackages,
     optOutBundlingPackageRegex,
     dir,
   })
@@ -1662,6 +1665,7 @@ export default async function getBaseWebpackConfig(
             outputFileTracingRoot: config.experimental.outputFileTracingRoot,
             appDirEnabled: hasAppDir,
             turbotrace: config.experimental.turbotrace,
+            optOutBundlingPackages,
             traceIgnores: config.experimental.outputFileTracingIgnores || [],
           }
         ),

packages/next/src/build/webpack/plugins/next-trace-entrypoints-plugin.ts

@@ -133,6 +133,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
   private rootDir: string
   private appDir: string | undefined
   private pagesDir: string | undefined
+  private optOutBundlingPackages: string[]
   private appDirEnabled?: boolean
   private tracingRoot: string
   private entryTraces: Map<string, Set<string>>
@@ -144,6 +145,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
     rootDir,
     appDir,
     pagesDir,
+    optOutBundlingPackages,
     appDirEnabled,
     traceIgnores,
     esmExternals,
@@ -153,6 +155,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
     rootDir: string
     appDir: string | undefined
     pagesDir: string | undefined
+    optOutBundlingPackages: string[]
     appDirEnabled?: boolean
     traceIgnores?: string[]
     outputFileTracingRoot?: string
@@ -168,6 +171,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
     this.traceIgnores = traceIgnores || []
     this.tracingRoot = outputFileTracingRoot || rootDir
     this.turbotrace = turbotrace
+    this.optOutBundlingPackages = optOutBundlingPackages
   }
 
   // Here we output all traced assets and webpack chunks to a
@@ -743,6 +747,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
             context,
             request,
             isEsmRequested,
+            this.optOutBundlingPackages,
             (options) => (_: string, resRequest: string) => {
               return getResolve(options)(parent, resRequest, job)
             },

test/e2e/app-dir/app-external/app-external.test.ts

@@ -1,5 +1,5 @@
 import { createNextDescribe } from 'e2e-utils'
-import { shouldRunTurboDevTest } from '../../../lib/next-test-utils'
+import { check, shouldRunTurboDevTest } from 'next-test-utils'
 
 async function resolveStreamResponse(response: any, onData?: any) {
   let result = ''
@@ -250,5 +250,22 @@ createNextDescribe(
       const html = await next.render('/async-storage')
       expect(html).toContain('success')
     })
+
+    it('should not prefer to resolve esm over cjs for bundling optout packages', async () => {
+      const browser = await next.browser('/optout/action')
+      expect(await browser.elementByCss('#dual-pkg-outout p').text()).toBe('')
+
+      browser.elementByCss('#dual-pkg-outout button').click()
+      await check(async () => {
+        const text = await browser.elementByCss('#dual-pkg-outout p').text()
+        if (process.env.TURBOPACK) {
+          // The prefer esm won't effect turbopack resolving
+          expect(text).toBe('dual-pkg-optout:mjs')
+        } else {
+          expect(text).toBe('dual-pkg-optout:cjs')
+        }
+        return 'success'
+      }, /success/)
+    })
   }
 )

test/e2e/app-dir/app-external/app/optout/action/actions.js

@@ -0,0 +1,7 @@
+'use server'
+
+import { value as dualPkgOptoutValue } from 'dual-pkg-optout'
+
+export async function getDualOptoutValue() {
+  return dualPkgOptoutValue
+}

test/e2e/app-dir/app-external/app/optout/action/page.js

@@ -0,0 +1,20 @@
+'use client'
+
+import { useState } from 'react'
+import { getDualOptoutValue } from './actions'
+
+export default function Page() {
+  const [optoutDisplayValue, setOptoutDisplayValue] = useState('')
+  return (
+    <div id="dual-pkg-outout">
+      <p>{optoutDisplayValue}</p>
+      <button
+        onClick={async () => {
+          setOptoutDisplayValue(await getDualOptoutValue())
+        }}
+      >
+        dual-pkg-optout
+      </button>
+    </div>
+  )
+}

test/e2e/app-dir/app-external/next.config.js

@@ -2,6 +2,9 @@ module.exports = {
   reactStrictMode: true,
   transpilePackages: ['untranspiled-module', 'css', 'font'],
   experimental: {
-    serverComponentsExternalPackages: ['conditional-exports-optout'],
+    serverComponentsExternalPackages: [
+      'conditional-exports-optout',
+      'dual-pkg-optout',
+    ],
   },
 }

test/e2e/app-dir/app-external/node_modules_bak/dual-pkg-optout/index.cjs

@@ -0,0 +1 @@
+exports.value = 'dual-pkg-optout:cjs'

test/e2e/app-dir/app-external/node_modules_bak/dual-pkg-optout/index.mjs

@@ -0,0 +1 @@
+export const value = 'dual-pkg-optout:mjs'

test/e2e/app-dir/app-external/node_modules_bak/dual-pkg-optout/package.json

@@ -0,0 +1,6 @@
+{
+  "exports": {
+    "import": "./index.mjs",
+    "require": "./index.cjs"
+  }
+}