Skip to content

Commit

Permalink
Fix nested esm package default import resolving mismatch (#57784)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
huozhi authored Oct 31, 2023
1 parent 5eb6607 commit cd821c8
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 14 deletions.
37 changes: 23 additions & 14 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 = {
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-external/app-external.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/app-dir/app-external/app/esm/client/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -14,6 +15,7 @@ export default function Index() {
<h2>{'Test: ' + value}</h2>
<h2>{`CJS-ESM Compat package: ${compatPackageEntry}`}</h2>
<h2>{`CJS package: ${packageName}`}</h2>
<h2>{`Nested imports: ${nestedImportExportDefaultValue}`}</h2>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const value = require('cjs-esm-compat').packageEntry

exports.default = 'nested-import:cjs:' + value
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { packageEntry as value } from 'cjs-esm-compat'

const packageEntry = 'nested-import:esm:' + value

export default packageEntry
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"exports": {
"import": "./index.mjs",
"require": "./index.cjs"
}
}

0 comments on commit cd821c8

Please sign in to comment.