Skip to content

Commit

Permalink
Eagerly bundle external ESM dependencies for pages (#42741)
Browse files Browse the repository at this point in the history
One potential risk is ESM dependencies that can't be bundled will cause
a build error. This also means that the `esmExternals` configuration
will be affected.

Closes #42249, closes #42588. Related: #1395.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
shuding and kodiakhq[bot] authored Nov 10, 2022
1 parent 6ab52ed commit 276533d
Show file tree
Hide file tree
Showing 52 changed files with 94 additions and 7 deletions.
15 changes: 10 additions & 5 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1256,11 +1256,16 @@ export default async function getBaseWebpackConfig(
}
}

const shouldBeBundled = isResourceInPackages(
res,
config.experimental.transpilePackages,
resolvedExternalPackageDirs
)
// If a package is included in `transpilePackages`, we don't want to make it external.
// And also, if that resource is an ES module, we bundle it too because we can't
// rely on the require hook to alias `react` to our precompiled version.
const shouldBeBundled =
isResourceInPackages(
res,
config.experimental.transpilePackages,
resolvedExternalPackageDirs
) ||
(isEsm && config.experimental.appDir)

if (/node_modules[/\\].*\.[mc]?js$/.test(res)) {
if (layer === WEBPACK_LAYERS.server) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async function resolveStreamResponse(response: any, onData?: any) {
return result
}

describe('app dir - rsc external dependency', () => {
describe('app dir - external dependency', () => {
let next: NextInstance

if ((global as any).isNextDeploy) {
Expand All @@ -28,7 +28,7 @@ describe('app dir - rsc external dependency', () => {

beforeAll(async () => {
next = await createNext({
files: new FileRef(path.join(__dirname, './rsc-external')),
files: new FileRef(path.join(__dirname, './app-external')),
dependencies: {
'@next/font': 'canary',
react: 'latest',
Expand Down Expand Up @@ -166,4 +166,30 @@ describe('app dir - rsc external dependency', () => {
)
).toMatch(/^__myFont_.{6}, __myFont_Fallback_.{6}$/)
})

describe('react in external esm packages', () => {
it('should use the same react in client app', async () => {
const html = await renderViaHTTP(next.url, '/esm/client')

const v1 = html.match(/App React Version: ([^<]+)</)[1]
const v2 = html.match(/External React Version: ([^<]+)</)[1]
expect(v1).toBe(v2)
})

it('should use the same react in server app', async () => {
const html = await renderViaHTTP(next.url, '/esm/server')

const v1 = html.match(/App React Version: ([^<]+)</)[1]
const v2 = html.match(/External React Version: ([^<]+)</)[1]
expect(v1).toBe(v2)
})

it('should use the same react in pages', async () => {
const html = await renderViaHTTP(next.url, '/test-pages-esm')

const v1 = html.match(/App React Version: ([^<]+)</)[1]
const v2 = html.match(/External React Version: ([^<]+)</)[1]
expect(v1).toBe(v2)
})
})
})
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app-external/app/esm/client/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use client'
import { version, useValue } from 'esm-with-react'

import React from 'react'

export default function Index() {
const value = useValue()
return (
<div>
<h2>{'App React Version: ' + React.version}</h2>
<h2>{'External React Version: ' + version}</h2>
<h2>{'Test: ' + value}</h2>
</div>
)
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/app-external/app/esm/server/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { version } from 'esm-with-react'

import React from 'react'

export default function Index() {
return (
<div>
<h2>{'App React Version: ' + React.version}</h2>
<h2>{'External React Version: ' + version}</h2>
</div>
)
}
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import React from 'react'

const Context = React.createContext ? React.createContext('hello') : null

export const version = React.version
export const useValue = () => React.useContext(Context)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "esm-with-react",
"type": "module",
"exports": "./index.js",
"dependencies": {
"react": "^18",
"react-dom": "^18"
}
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/app-external/pages/test-pages-esm.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { version, useValue } from 'esm-with-react'

import React from 'react'

export default function Index() {
const value = useValue()
return (
<div>
<h2>{'App React Version: ' + React.version}</h2>
<h2>{'External React Version: ' + version}</h2>
<h2>{'Test: ' + value}</h2>
</div>
)
}

0 comments on commit 276533d

Please sign in to comment.