Skip to content

Commit

Permalink
fix: mixing namespace import and named import client components (#64809)
Browse files Browse the repository at this point in the history
### What

Reported by @MaxLeiter, when you mixing named import and wildcard import
to a client component, and if you clone the module it will missed others
exports except the named ones. This lead to an issue that rendered React
element type is `undefined`.

### Why

We're using a tree-shaking strategy that collects the imported
identifiers from client components on server components side. But in our
code `connection.dependency.ids` can be undefined when you're using
`import *`. So for that case we include all the exports.

In the flight client entry plugin, if we found there's named imports
that collected later, and the module is already being marked as
namespace import `*`, we merge the ids into "*", so the whole module and
all exports are respected.

Now there're few possible cases for a client component import:

During webpack build, in the outout going connections, there're
connection with empty imported ids (`[]`), cannot unable to detect the
imported ids (`['*']`) and detected named imported ids (`['a', 'b',
..]`). First two represnt to include the whole module and all exports,
but we might collect the named imports could come later than the whole
module. So if we found the existing collection already has `['*']` then
we keep using that regardless the collected named imports. This can
avoid the collected named imports cover "exports all" case, where we
will expose less exports for that collection module lead to the
undefined component error.

Closes NEXT-3177
  • Loading branch information
huozhi committed Apr 23, 2024
1 parent de84e3a commit 8d01d49
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,8 @@ export class FlightClientEntryPlugin {
mod,
modRequest,
clientComponentImports,
importedIdentifiers
importedIdentifiers,
false
)
}
return
Expand Down Expand Up @@ -707,19 +708,27 @@ export class FlightClientEntryPlugin {
mod,
modRequest,
clientComponentImports,
importedIdentifiers
importedIdentifiers,
true
)

return
}

getModuleReferencesInOrder(mod, compilation.moduleGraph).forEach(
(connection: any) => {
const dependencyIds: string[] = []
if (connection.dependency?.ids?.length) {
let dependencyIds: string[] = []
const depModule = connection.resolvedModule

// `ids` are the identifiers that are imported from the dependency,
// if it's present, it's an array of strings.
if (connection.dependency?.ids) {
dependencyIds.push(...connection.dependency.ids)
} else {
dependencyIds = ['*']
}
filterClientComponents(connection.resolvedModule, dependencyIds)

filterClientComponents(depModule, dependencyIds)
}
)
}
Expand Down Expand Up @@ -1030,7 +1039,8 @@ function addClientImport(
mod: webpack.NormalModule,
modRequest: string,
clientComponentImports: ClientComponentImports,
importedIdentifiers: string[]
importedIdentifiers: string[],
isFirstImport: boolean
) {
const clientEntryType = getModuleBuildInfo(mod).rsc?.clientEntryType
const isCjsModule = clientEntryType === 'cjs'
Expand All @@ -1039,23 +1049,34 @@ function addClientImport(
isCjsModule ? 'commonjs' : 'auto'
)

const isAutoModuleSourceType = assumedSourceType === 'auto'
if (isAutoModuleSourceType) {
clientComponentImports[modRequest] = new Set(['*'])
const clientImportsSet = clientComponentImports[modRequest]

if (importedIdentifiers[0] === '*') {
// If there's collected import path with named import identifiers,
// or there's nothing in collected imports are empty.
// we should include the whole module.
if (!isFirstImport && [...clientImportsSet][0] !== '*') {
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')
}
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)
clientComponentImports[modRequest].add(name)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

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

export function ClientModExportB() {
return 'client-mod:export-b'
}

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

export {
ClientModExportA,
ClientModExportB,
ClientModExportC,
} from './client-module'
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use client'

export function ClientMod2ExportA() {
return 'client-mod2:export-a'
}

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

export { ClientMod2ExportA, ClientMod2ExportB } from './client-module'
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as clientMod from './client-module'
import { ClientModExportC } from './client-module'
import * as clientMod2 from './client-module2'

const mod1Map = {
...clientMod,
}

const mod2Map = {
...clientMod2,
}

const A = mod1Map.ClientModExportA
const B = mod1Map.ClientModExportB
const C = mod1Map.ClientModExportC
const A2 = mod2Map.ClientMod2ExportA
const B2 = mod2Map.ClientMod2ExportB

export default function Page() {
return (
<div>
<p id="a">
<A />
</p>
<p id="b">
<B />
</p>
<p id="c">
<C />
</p>
<p id="named-c">
<ClientModExportC />
</p>
<p id="a2">
<A2 />
</p>
<p id="b2">
<B2 />
</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ImportedComp } from './client'
import { ImportedComp } from './client-relative-dep'

export default function Page() {
return <ImportedComp />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,5 +103,19 @@ createNextDescribe(

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

it('should handle mixing namespace imports and named imports from client components', async () => {
const $ = await next.render$('/client-import-namespace')

// mixing namespace imports and named imports
expect($('#a').text()).toContain('client-mod:export-a')
expect($('#b').text()).toContain('client-mod:export-b')
expect($('#c').text()).toContain('client-mod:export-c')
expect($('#named-c').text()).toContain('client-mod:export-c')

// only named exports
expect($('#a2').text()).toContain('client-mod2:export-a')
expect($('#b2').text()).toContain('client-mod2:export-b')
})
}
)

0 comments on commit 8d01d49

Please sign in to comment.