Skip to content

Commit

Permalink
Fix: resolve mixed re-exports module as cjs (#64681)
Browse files Browse the repository at this point in the history
### Why

If you have a client entry that mixing `default` re-export and `*`
re-export, atm we cannot statically analyze all the exports from this
the boundary, unless we can apply barrel file optimization for every
import which could slow down speed.

```js
// index.js
'use client'
export * from './client'
export { default } from './client'
```

Before that happen we high recommend you don't mixing that and try to
add the client directive to the leaf level client module. We're not able
to determine what the identifiers are imported from the wildcard import
path. This would work if we resolved the actual file but currently we
can't.

### What

When we found the mixing client entry module like that, we treat it as a
CJS client module and include all the bundle in client like before what
we have the client components import optimization.

Ideally we could warn users don't apply the client directive to these
kinda of barrel file, and only apply them to where we needed.


Fixes #64518 
Closes NEXT-3119
  • Loading branch information
huozhi committed Apr 23, 2024
1 parent c850e4a commit de84e3a
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export default function transformSource(

// When we cannot determine the export names, we use eager mode to include the whole module.
// Otherwise, we use eager mode with webpackExports to only include the necessary exports.
if (ids.length === 0) {
// If we have '*' in the ids, we include all the imports
if (ids.length === 0 || ids.includes('*')) {
return `import(/* webpackMode: "eager" */ ${importPath});\n`
} else {
return `import(/* webpackMode: "eager", webpackExports: ${JSON.stringify(
Expand Down
64 changes: 39 additions & 25 deletions packages/next/src/build/webpack/loaders/next-flight-loader/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'
import { RSC_MOD_REF_PROXY_ALIAS } from '../../../../lib/constants'
import {
BARREL_OPTIMIZATION_PREFIX,
Expand All @@ -13,6 +14,36 @@ const noopHeadPath = require.resolve('next/dist/client/components/noop-head')
const MODULE_PROXY_PATH =
'next/dist/build/webpack/loaders/next-flight-loader/module-proxy'

type SourceType = 'auto' | 'commonjs' | 'module'
export function getAssumedSourceType(
mod: webpack.Module,
sourceType: SourceType
): SourceType {
const buildInfo = getModuleBuildInfo(mod)
const detectedClientEntryType = buildInfo?.rsc?.clientEntryType
const clientRefs = buildInfo?.rsc?.clientRefs || []

// It's tricky to detect the type of a client boundary, but we should always
// use the `module` type when we can, to support `export *` and `export from`
// syntax in other modules that import this client boundary.
let assumedSourceType = sourceType
if (assumedSourceType === 'auto' && detectedClientEntryType === 'auto') {
if (
clientRefs.length === 0 ||
(clientRefs.length === 1 && clientRefs[0] === '')
) {
// If there's zero export detected in the client boundary, and it's the
// `auto` type, we can safely assume it's a CJS module because it doesn't
// have ESM exports.
assumedSourceType = 'commonjs'
} else if (!clientRefs.includes('*')) {
// Otherwise, we assume it's an ESM module.
assumedSourceType = 'module'
}
}
return assumedSourceType
}

export default function transformSource(
this: any,
source: string,
Expand Down Expand Up @@ -50,29 +81,12 @@ export default function transformSource(

// A client boundary.
if (buildInfo.rsc?.type === RSC_MODULE_TYPES.client) {
const sourceType = this._module?.parser?.sourceType
const detectedClientEntryType = buildInfo.rsc.clientEntryType
const assumedSourceType = getAssumedSourceType(
this._module,
this._module?.parser?.sourceType
)
const clientRefs = buildInfo.rsc.clientRefs!

// It's tricky to detect the type of a client boundary, but we should always
// use the `module` type when we can, to support `export *` and `export from`
// syntax in other modules that import this client boundary.
let assumedSourceType = sourceType
if (assumedSourceType === 'auto' && detectedClientEntryType === 'auto') {
if (
clientRefs.length === 0 ||
(clientRefs.length === 1 && clientRefs[0] === '')
) {
// If there's zero export detected in the client boundary, and it's the
// `auto` type, we can safely assume it's a CJS module because it doesn't
// have ESM exports.
assumedSourceType = 'commonjs'
} else if (!clientRefs.includes('*')) {
// Otherwise, we assume it's an ESM module.
assumedSourceType = 'module'
}
}

if (assumedSourceType === 'module') {
if (clientRefs.includes('*')) {
this.callback(
Expand Down Expand Up @@ -123,9 +137,9 @@ export { e${cnt++} as ${ref} };`
}
}

this.callback(
null,
source.replace(RSC_MOD_REF_PROXY_ALIAS, MODULE_PROXY_PATH),
sourceMap
const replacedSource = source.replace(
RSC_MOD_REF_PROXY_ALIAS,
MODULE_PROXY_PATH
)
this.callback(null, replacedSource, sourceMap)
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { getProxiedPluginState } from '../../build-context'
import { PAGE_TYPES } from '../../../lib/page-types'
import { isWebpackServerOnlyLayer } from '../../utils'
import { getModuleBuildInfo } from '../loaders/get-module-build-info'
import { getAssumedSourceType } from '../loaders/next-flight-loader'

interface Options {
dev: boolean
Expand Down Expand Up @@ -665,18 +666,12 @@ 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)
}
addClientImport(
mod,
modRequest,
clientComponentImports,
importedIdentifiers
)
}
return
}
Expand Down Expand Up @@ -708,9 +703,13 @@ export class FlightClientEntryPlugin {
if (!clientComponentImports[modRequest]) {
clientComponentImports[modRequest] = new Set()
}
for (const name of importedIdentifiers) {
clientComponentImports[modRequest].add(name)
}
addClientImport(
mod,
modRequest,
clientComponentImports,
importedIdentifiers
)

return
}

Expand Down Expand Up @@ -1026,3 +1025,37 @@ export class FlightClientEntryPlugin {
new sources.RawSource(json) as unknown as webpack.sources.RawSource
}
}

function addClientImport(
mod: webpack.NormalModule,
modRequest: string,
clientComponentImports: ClientComponentImports,
importedIdentifiers: string[]
) {
const clientEntryType = getModuleBuildInfo(mod).rsc?.clientEntryType
const isCjsModule = clientEntryType === 'cjs'
const assumedSourceType = getAssumedSourceType(
mod,
isCjsModule ? 'commonjs' : 'auto'
)

const isAutoModuleSourceType = assumedSourceType === 'auto'
if (isAutoModuleSourceType) {
clientComponentImports[modRequest] = new Set(['*'])
} else {
// If it's not analyzed as named ESM exports, e.g. if it's mixing `export *` with named exports,
// We'll include all modules since it's not able to do tree-shaking.
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)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

export default function ClientModExportDefault() {
return 'client:mod-export-default'
}

export function ClientModExportA() {
return 'client:mod-export-a'
}

export function ClientModExportB() {
return 'client:mod-export-b'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
'use client'

export { default } from './client'
export * from './client'
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import ClientDefault from './client-module'

export default function Page() {
return (
<div>
<p>
<ClientDefault />
</p>
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,11 @@ createNextDescribe(
chunkContents.every((content) => content.includes('cjs-client:foo'))
).toBe(false)
})

it('should able to resolve the client module entry with mixing rexports', async () => {
const $ = await next.render$('/client-reexport-index')

expect($('p').text()).toContain('client:mod-export-default')
})
}
)

0 comments on commit de84e3a

Please sign in to comment.