From 0e6e83c33dd580dee87f86a239d088c82b179d63 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 1 Jul 2024 18:28:11 +0200 Subject: [PATCH] Add warn and telemetry for customized esmExternals (#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. --- packages/next/src/build/webpack-config.ts | 2 ++ .../build/webpack/plugins/telemetry-plugin.ts | 2 ++ packages/next/src/server/config.ts | 35 ++++++++++++++++++ packages/next/src/telemetry/events/build.ts | 1 + .../esm-externals-false/app/layout.js | 7 ++++ .../esm-externals-false/app/page.js | 3 ++ .../esm-externals-false.test.ts | 36 +++++++++++++++++++ .../esm-externals-false/next.config.js | 8 +++++ 8 files changed, 94 insertions(+) create mode 100644 test/e2e/next-config-warnings/esm-externals-false/app/layout.js create mode 100644 test/e2e/next-config-warnings/esm-externals-false/app/page.js create mode 100644 test/e2e/next-config-warnings/esm-externals-false/esm-externals-false.test.ts create mode 100644 test/e2e/next-config-warnings/esm-externals-false/next.config.js diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 44dabe04f6572..3bf30b508707d 100644 --- 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 index 9926f22145df8..8de21f37c33b2 100644 --- 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 = [ 'skipMiddlewareUrlNormalize', 'skipTrailingSlashRedirect', 'modularizeImports', + 'esmExternals', ] const eliminatedPackages = new Set() diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts index 0534ca8dd9444..21689b81ff43b 100644 --- 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 index 2249e82572b44..faedb1d0f1692 100644 --- 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 new file mode 100644 index 0000000000000..750eb927b1980 --- /dev/null +++ b/test/e2e/next-config-warnings/esm-externals-false/app/layout.js @@ -0,0 +1,7 @@ +export default function Layout({ children }) { + return ( + + {children} + + ) +} 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 new file mode 100644 index 0000000000000..04d7e2308b66e --- /dev/null +++ b/test/e2e/next-config-warnings/esm-externals-false/app/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return
Page
+} 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 new file mode 100644 index 0000000000000..4534a14656c23 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..118cbceebbb86 --- /dev/null +++ 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, + }, +}