Add warn and telemetry for customized esmExternals (vercel#67339)

### What * Warn with next.js when users customized `experimental.esmExternals` value * Add telemetry tracking on the customization usage for that flag. 0 for no customization, 1 for used non-default customized value ### Why `esmExternals` ideally can just remain as default value `true` which Next.js can handle the customization properly. Since next.js app router also supports it on canary now we're adding a warning to users that don't modify `esmExternals` option as it could affect module resolution on ESM packages.
Gand0r · Jul 1, 2024 · 0e6e83c · 0e6e83c
1 parent 0115636
commit 0e6e83c
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 0 deletions.
diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -1953,6 +1953,8 @@ export default async function getBaseWebpackConfig(
               ],
               ['skipTrailingSlashRedirect', !!config.skipTrailingSlashRedirect],
               ['modularizeImports', !!config.modularizeImports],
+              // If esmExternals is not same as default value, it represents customized usage
+              ['esmExternals', config.experimental.esmExternals !== true],
               SWCBinaryTarget,
             ].filter<[Feature, boolean]>(Boolean as any)
           )

diff --git a/packages/next/src/build/webpack/plugins/telemetry-plugin.ts b/packages/next/src/build/webpack/plugins/telemetry-plugin.ts
@@ -43,6 +43,7 @@ export type Feature =
   | 'skipMiddlewareUrlNormalize'
   | 'skipTrailingSlashRedirect'
   | 'modularizeImports'
+  | 'esmExternals'
 
 interface FeatureUsage {
   featureName: Feature
@@ -107,6 +108,7 @@ const BUILD_FEATURES: Array<Feature> = [
   'skipMiddlewareUrlNormalize',
   'skipTrailingSlashRedirect',
   'modularizeImports',
+  'esmExternals',
 ]
 
 const eliminatedPackages = new Set<string>()

diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts
@@ -158,6 +158,32 @@ export function warnOptionHasBeenMovedOutOfExperimental(
   return config
 }
 
+function warnCustomizedOption(
+  config: NextConfig,
+  key: string,
+  defaultValue: any,
+  customMessage: string,
+  configFileName: string,
+  silent: boolean
+) {
+  const segs = key.split('.')
+  let current = config
+
+  while (segs.length >= 1) {
+    const seg = segs.shift()!
+    if (!(seg in current)) {
+      return
+    }
+    current = current[seg]
+  }
+
+  if (!silent && current !== defaultValue) {
+    Log.warn(
+      `The "${key}" option has been modified. ${customMessage ? customMessage + '. ' : ''}Please update your ${configFileName}.`
+    )
+  }
+}
+
 function assignDefaults(
   dir: string,
   userConfig: { [key: string]: any },
@@ -454,6 +480,15 @@ function assignDefaults(
     }
   }
 
+  warnCustomizedOption(
+    result,
+    'experimental.esmExternals',
+    true,
+    'experimental.esmExternals is not recommended to be modified as it may disrupt module resolution',
+    configFileName,
+    silent
+  )
+
   warnOptionHasBeenMovedOutOfExperimental(
     result,
     'bundlePagesExternals',

diff --git a/packages/next/src/telemetry/events/build.ts b/packages/next/src/telemetry/events/build.ts
@@ -172,6 +172,7 @@ export type EventBuildFeatureUsage = {
     | 'skipMiddlewareUrlNormalize'
     | 'skipTrailingSlashRedirect'
     | 'modularizeImports'
+    | 'esmExternals'
   invocationCount: number
 }
 export function eventBuildFeatureUsage(

diff --git a/test/e2e/next-config-warnings/esm-externals-false/app/layout.js b/test/e2e/next-config-warnings/esm-externals-false/app/layout.js
@@ -0,0 +1,7 @@
+export default function Layout({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/next-config-warnings/esm-externals-false/app/page.js b/test/e2e/next-config-warnings/esm-externals-false/app/page.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Page</div>
+}
diff --git a/test/e2e/next-config-warnings/esm-externals-false/esm-externals-false.test.ts b/test/e2e/next-config-warnings/esm-externals-false/esm-externals-false.test.ts
@@ -0,0 +1,36 @@
+import { nextTestSetup } from 'e2e-utils'
+import { findAllTelemetryEvents } from 'next-test-utils'
+
+// Turbopack hasn't fully enabled this option yet
+;(process.env.TURBOPACK ? describe.skip : describe)(
+  'next-config-warnings - esm-externals-false',
+  () => {
+    const { next, isNextStart } = nextTestSetup({
+      files: __dirname,
+      env: {
+        NEXT_TELEMETRY_DEBUG: '1',
+      },
+    })
+
+    it('should warn when using ESM externals: false', async () => {
+      await next.fetch('/')
+
+      expect(next.cliOutput).toContain(
+        `The "experimental.esmExternals" option has been modified. experimental.esmExternals is not recommended to be modified as it may disrupt module resolution. Please update your next.config.js`
+      )
+    })
+
+    if (isNextStart) {
+      it('should contain esmExternals feature usage in telemetry', async () => {
+        const featureUsageEvents = findAllTelemetryEvents(
+          next.cliOutput,
+          'NEXT_BUILD_FEATURE_USAGE'
+        )
+        expect(featureUsageEvents).toContainEqual({
+          featureName: 'esmExternals',
+          invocationCount: 1,
+        })
+      })
+    }
+  }
+)
diff --git a/test/e2e/next-config-warnings/esm-externals-false/next.config.js b/test/e2e/next-config-warnings/esm-externals-false/next.config.js
@@ -0,0 +1,8 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+module.exports = {
+  experimental: {
+    esmExternals: false,
+  },
+}