Skip to content

Commit

Permalink
Fix cjs client components tree-shaking (#64558)
Browse files Browse the repository at this point in the history
## What

Determine if the client module is a CJS file and `default` export is
imported, then we include the whole module instead of using webpack
magic comments to only extract `default` export.

## Why

Unlike ESM, The `default` export of CJS module is not just `.default`
property, we need to include `__esModule` mark along with `default`
export to make it function properly with React client module proxy


Fixes #64518
Closes NEXT-3119
  • Loading branch information
huozhi authored and ztanner committed Apr 17, 2024
1 parent 282de73 commit 6838f57
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { normalizePathSep } from '../../../shared/lib/page-path/normalize-path-s
import { getProxiedPluginState } from '../../build-context'
import { PAGE_TYPES } from '../../../lib/page-types'
import { isWebpackServerOnlyLayer } from '../../utils'
import { getModuleBuildInfo } from '../loaders/get-module-build-info'

interface Options {
dev: boolean
Expand Down Expand Up @@ -664,7 +665,16 @@ export class FlightClientEntryPlugin {
if (!modRequest) return
if (visited.has(modRequest)) {
if (clientComponentImports[modRequest]) {
const isCjsModule =
getModuleBuildInfo(mod).rsc?.clientEntryType === 'cjs'
for (const name of importedIdentifiers) {
// For cjs module default import, we include the whole module since
const isCjsDefaultImport = isCjsModule && name === 'default'
// Always include __esModule along with cjs module default export,
// to make sure it work with client module proxy from React.
if (isCjsDefaultImport) {
clientComponentImports[modRequest].add('__esModule')
}
clientComponentImports[modRequest].add(name)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import CjsClientDefault from 'cjs-client-module'

export default function Page() {
return <CjsClientDefault />
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,32 @@ createNextDescribe(
)
).toBe(false)
})

it('should only include the imported identifier of CJS module in browser bundle', async () => {
const clientChunksDir = join(
next.testDir,
'.next',
'static',
'chunks',
'app',
'cjs-dep'
)

const chunkContents = fs
.readdirSync(clientChunksDir, {
withFileTypes: true,
})
.filter((dirent) => dirent.isFile())
.map((chunkDirent) =>
fs.readFileSync(join(chunkDirent.path, chunkDirent.name), 'utf8')
)

expect(
chunkContents.some((content) => content.includes('cjs-client:default'))
).toBe(true)
expect(
chunkContents.every((content) => content.includes('cjs-client:foo'))
).toBe(false)
})
}
)

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.

3 changes: 2 additions & 1 deletion test/turbopack-build-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -14593,7 +14593,8 @@
"passed": [],
"failed": [
"app-dir client-components-tree-shaking should only include imported components 3rd party package in browser bundle with direct imports",
"app-dir client-components-tree-shaking should only include imported relative components in browser bundle with direct imports"
"app-dir client-components-tree-shaking should only include imported relative components in browser bundle with direct imports",
"app-dir client-components-tree-shaking should only include the imported identifier of CJS module in browser bundle"
],
"pending": [],
"flakey": [],
Expand Down

0 comments on commit 6838f57

Please sign in to comment.