Skip to content

Commit

Permalink
fix esm externals edge case with "module" condition (#28612)
Browse files Browse the repository at this point in the history
prefer commonjs externals even if "module" condition is used in package.json



## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
sokra committed Aug 31, 2021
1 parent 71771df commit f6d8a11
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 30 deletions.
22 changes: 4 additions & 18 deletions packages/next/build/webpack-config.ts
Expand Up @@ -173,28 +173,14 @@ export function attachReactRefresh(
}
}

const WEBPACK_RESOLVE_OPTIONS = {
// This always uses commonjs resolving, assuming API is identical
// between ESM and CJS in a package
// Otherwise combined ESM+CJS packages will never be external
// as resolving mismatch would lead to opt-out from being external.
dependencyType: 'commonjs',
symlinks: true,
}

const WEBPACK_ESM_RESOLVE_OPTIONS = {
dependencyType: 'esm',
symlinks: true,
}

const NODE_RESOLVE_OPTIONS = {
dependencyType: 'commonjs',
modules: ['node_modules'],
alias: false,
fallback: false,
exportsFields: ['exports'],
importsFields: ['imports'],
conditionNames: ['node', 'require', 'module'],
conditionNames: ['node', 'require'],
descriptionFiles: ['package.json'],
extensions: ['.js', '.json', '.node'],
enforceExtensions: false,
Expand All @@ -211,7 +197,7 @@ const NODE_RESOLVE_OPTIONS = {
const NODE_ESM_RESOLVE_OPTIONS = {
...NODE_RESOLVE_OPTIONS,
dependencyType: 'esm',
conditionNames: ['node', 'import', 'module'],
conditionNames: ['node', 'import'],
fullySpecified: true,
}

Expand Down Expand Up @@ -754,7 +740,7 @@ export default async function getBaseWebpackConfig(
const preferEsm = esmExternals && isEsmRequested

const resolve = getResolve(
preferEsm ? WEBPACK_ESM_RESOLVE_OPTIONS : WEBPACK_RESOLVE_OPTIONS
preferEsm ? NODE_ESM_RESOLVE_OPTIONS : NODE_RESOLVE_OPTIONS
)

// Resolve the import with the webpack provided context, this
Expand All @@ -772,7 +758,7 @@ export default async function getBaseWebpackConfig(
// try the alternative resolving options.
if (!res && (isEsmRequested || looseEsmExternals)) {
const resolveAlternative = getResolve(
preferEsm ? WEBPACK_RESOLVE_OPTIONS : WEBPACK_ESM_RESOLVE_OPTIONS
preferEsm ? NODE_RESOLVE_OPTIONS : NODE_ESM_RESOLVE_OPTIONS
)
try {
;[res, isEsm] = await resolveAlternative(context, request)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/externals-esm-loose/test/index.test.js
Expand Up @@ -15,7 +15,7 @@ const appDir = join(__dirname, '../')
let appPort
let app

describe('Valid resolve alias', () => {
describe("Handle ESM externals with esmExternals: 'loose'", () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/externals-esm/test/index.test.js
Expand Up @@ -15,7 +15,7 @@ const appDir = join(__dirname, '../')
let appPort
let app

describe('Valid resolve alias', () => {
describe('Handle ESM externals with esmExternals: true', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions test/integration/externals/pages/ssg.js
@@ -1,5 +1,6 @@
import React from 'preact/compat'
import World from 'esm-package/entry'
import World1 from 'esm-package/entry'
import World2 from 'module-package'

export async function getStaticProps() {
return {
Expand All @@ -8,5 +9,9 @@ export async function getStaticProps() {
}

export default function Index(props) {
return <div>Hello {World}</div>
return (
<div>
Hello {World1}+{World2}
</div>
)
}
9 changes: 7 additions & 2 deletions test/integration/externals/pages/ssr.js
@@ -1,10 +1,15 @@
import React from 'preact/compat'
import World from 'esm-package/entry'
import World1 from 'esm-package/entry'
import World2 from 'module-package'

export function getServerSideProps() {
return {}
}

export default function Index(props) {
return <div>Hello {World}</div>
return (
<div>
Hello {World1}+{World2}
</div>
)
}
9 changes: 7 additions & 2 deletions test/integration/externals/pages/static.js
@@ -1,6 +1,11 @@
import React from 'preact/compat'
import World from 'esm-package/entry'
import World1 from 'esm-package/entry'
import World2 from 'module-package'

export default function Index(props) {
return <div>Hello {World}</div>
return (
<div>
Hello {World1}+{World2}
</div>
)
}
10 changes: 6 additions & 4 deletions test/integration/externals/test/index.test.js
Expand Up @@ -15,7 +15,7 @@ const appDir = join(__dirname, '../')
let appPort
let app

describe('Valid resolve alias', () => {
describe('Handle ESM externals', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
Expand All @@ -24,18 +24,20 @@ describe('Valid resolve alias', () => {
})
afterAll(() => killApp(app))

const expected = /Hello <!-- -->World<!-- -->\+<!-- -->World/

it('should render the static page', async () => {
const html = await renderViaHTTP(appPort, '/static')
expect(html).toMatch(/Hello <!-- -->World/)
expect(html).toMatch(expected)
})

it('should render the ssr page', async () => {
const html = await renderViaHTTP(appPort, '/ssr')
expect(html).toMatch(/Hello <!-- -->World/)
expect(html).toMatch(expected)
})

it('should render the ssg page', async () => {
const html = await renderViaHTTP(appPort, '/ssg')
expect(html).toMatch(/Hello <!-- -->World/)
expect(html).toMatch(expected)
})
})

0 comments on commit f6d8a11

Please sign in to comment.