From cd821c8e0351267b2c044f7f48e8db8918b07ef3 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 31 Oct 2023 01:31:23 +0100 Subject: [PATCH] Fix nested esm package default import resolving mismatch (#57784) For app router bundling layers "SSR rendering" and "browser" layer, which are used for server side rendering and client, we should still apply the module resolving rules to all assets since we bundled everything, removed the default exclude conditions as they'll not apply the rules to node_modules. That could cause the mismatch resolving for package. E.g. You have two dual packages A and B are both compatible for ESM and CJS, and both have default export. B is depent on A, but when you import B in a client component that will be SSR'd, it's picking up the CJS asset like the case described in #57584 . Fixes #57584 Closes NEXT-1702 --- packages/next/src/build/webpack-config.ts | 37 ++++++++++++------- .../app-dir/app-external/app-external.test.ts | 3 ++ .../app-external/app/esm/client/page.js | 2 + .../nested-import-export-default/index.cjs | 3 ++ .../nested-import-export-default/index.mjs | 5 +++ .../nested-import-export-default/package.json | 6 +++ 6 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.cjs create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.mjs create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/package.json diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 8223e506e8037..32559ce10e9af 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -730,13 +730,8 @@ export default async function getBaseWebpackConfig( const shouldIncludeExternalDirs = config.experimental.externalDir || !!config.transpilePackages - const codeCondition = { - test: /\.(tsx|ts|js|cjs|mjs|jsx)$/, - ...(shouldIncludeExternalDirs - ? // Allowing importing TS/TSX files from outside of the root dir. - {} - : { include: [dir, ...babelIncludeRegexes] }), - exclude: (excludePath: string) => { + function createLoaderRuleExclude(skipNodeModules: boolean) { + return (excludePath: string) => { if (babelIncludeRegexes.some((r) => r.test(excludePath))) { return false } @@ -747,8 +742,17 @@ export default async function getBaseWebpackConfig( ) if (shouldBeBundled) return false - return excludePath.includes('node_modules') - }, + return skipNodeModules && excludePath.includes('node_modules') + } + } + + const codeCondition = { + test: /\.(tsx|ts|js|cjs|mjs|jsx)$/, + ...(shouldIncludeExternalDirs + ? // Allowing importing TS/TSX files from outside of the root dir. + {} + : { include: [dir, ...babelIncludeRegexes] }), + exclude: createLoaderRuleExclude(true), } let webpackConfig: webpack.Configuration = { @@ -1401,12 +1405,17 @@ export default async function getBaseWebpackConfig( use: swcLoaderForServerLayer, }, { - ...codeCondition, - issuerLayer: [ - WEBPACK_LAYERS.appPagesBrowser, - WEBPACK_LAYERS.serverSideRendering, - ], + test: codeCondition.test, exclude: codeCondition.exclude, + issuerLayer: [WEBPACK_LAYERS.appPagesBrowser], + use: swcLoaderForClientLayer, + resolve: { + mainFields: getMainField('app', compilerType), + }, + }, + { + test: codeCondition.test, + issuerLayer: [WEBPACK_LAYERS.serverSideRendering], use: swcLoaderForClientLayer, resolve: { mainFields: getMainField('app', compilerType), diff --git a/test/e2e/app-dir/app-external/app-external.test.ts b/test/e2e/app-dir/app-external/app-external.test.ts index b4e66cb205966..dbf044d7324cc 100644 --- a/test/e2e/app-dir/app-external/app-external.test.ts +++ b/test/e2e/app-dir/app-external/app-external.test.ts @@ -168,6 +168,9 @@ createNextDescribe( 'CJS-ESM Compat package: cjs-esm-compat/index.mjs' ) expect(html).toContain('CJS package: cjs-lib') + expect(html).toContain( + 'Nested imports: nested-import:esm:cjs-esm-compat/index.mjs' + ) }) it('should use the same react in server app', async () => { diff --git a/test/e2e/app-dir/app-external/app/esm/client/page.js b/test/e2e/app-dir/app-external/app/esm/client/page.js index eaf90cd08743d..98fd9d66c4999 100644 --- a/test/e2e/app-dir/app-external/app/esm/client/page.js +++ b/test/e2e/app-dir/app-external/app/esm/client/page.js @@ -4,6 +4,7 @@ import React from 'react' import { version, useValue } from 'esm-with-react' import { packageEntry as compatPackageEntry } from 'cjs-esm-compat' import { packageName } from 'cjs-lib' +import nestedImportExportDefaultValue from 'nested-import-export-default' export default function Index() { const value = useValue() @@ -14,6 +15,7 @@ export default function Index() {

{'Test: ' + value}

{`CJS-ESM Compat package: ${compatPackageEntry}`}

{`CJS package: ${packageName}`}

+

{`Nested imports: ${nestedImportExportDefaultValue}`}

) } diff --git a/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.cjs b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.cjs new file mode 100644 index 0000000000000..fb0f843a10a24 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.cjs @@ -0,0 +1,3 @@ +const value = require('cjs-esm-compat').packageEntry + +exports.default = 'nested-import:cjs:' + value diff --git a/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.mjs b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.mjs new file mode 100644 index 0000000000000..5f8eef500e9b1 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/index.mjs @@ -0,0 +1,5 @@ +import { packageEntry as value } from 'cjs-esm-compat' + +const packageEntry = 'nested-import:esm:' + value + +export default packageEntry diff --git a/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/package.json b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/package.json new file mode 100644 index 0000000000000..3646ad4bdb5f5 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/nested-import-export-default/package.json @@ -0,0 +1,6 @@ +{ + "exports": { + "import": "./index.mjs", + "require": "./index.cjs" + } +}