Skip to content

Commit

Permalink
Use registerClientReference for ESM client component modules
Browse files Browse the repository at this point in the history
When registering a client reference for a client component module, React
offers two distinct functions:

- `registerClientReference`: Register a named or default export of an
  ES module as a client reference.
- `createClientModuleProxy`: Create a client module proxy for a CJS
  module that registers every module export as a client reference.

Until this PR, we were using the client module proxy for both kinds of
modules, which is incorrect. It was especially problematic because in
the Node.js runtime we were always awaiting the client module proxy.
This was a workaround for handling async modules (see #66990), but led
to issues like #71840.

With this PR, we are now using `registerClientReference` if we detect
that a module is an ES module. This aligns the Webpack implementation
with the one in Turbopack.

fixes #71840
  • Loading branch information
unstubbable committed Oct 28, 2024
1 parent 655b808 commit 3713b3a
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 89 deletions.
82 changes: 47 additions & 35 deletions packages/next/src/build/webpack/loaders/next-flight-loader/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,27 @@ export function getAssumedSourceType(
// 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 (sourceType === 'auto') {
if (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.
return 'commonjs'
} else if (!clientRefs.includes('*')) {
// Otherwise, we assume it's an ESM module.
return 'module'
}
} else if (detectedClientEntryType === 'cjs') {
return 'commonjs'
}
}
return assumedSourceType

return sourceType
}

export default function transformSource(
Expand Down Expand Up @@ -107,6 +112,7 @@ export default function transformSource(
)

const clientRefs = buildInfo.rsc.clientRefs!
const stringifiedResourceKey = JSON.stringify(resourceKey)

if (assumedSourceType === 'module') {
if (clientRefs.includes('*')) {
Expand All @@ -130,34 +136,40 @@ export default function transformSource(
)
}

// `proxy` is the module proxy that we treat the module as a client boundary.
// For ESM, we access the property of the module proxy directly for each export.
// This is bit hacky that treating using a CJS like module proxy for ESM's exports,
// but this will avoid creating nested proxies for each export. It will be improved in the future.

// Explanation for: await createProxy(...)
// We need to await the module proxy creation because it can be async module for SSR layer
// due to having async dependencies.
// We only apply `the await` for Node.js as only Edge doesn't have external dependencies.
let esmSource = `\
import { createProxy } from "${MODULE_PROXY_PATH}"
const proxy = ${isEdgeServer ? '' : 'await'} createProxy(String.raw\`${resourceKey}\`)
import { registerClientReference } from "react-server-dom-webpack/server.edge";
`
let cnt = 0
for (const ref of clientRefs) {
if (ref === '') {
esmSource += `exports[''] = proxy['']\n`
} else if (ref === 'default') {
esmSource += `export default proxy.default;\n`
if (ref === 'default') {
esmSource += `export default registerClientReference(
function() { throw new Error(${JSON.stringify(`Attempted to call the default \
export of ${stringifiedResourceKey} from the server, but it's on the client. \
It's not possible to invoke a client function from the server, it can only be \
rendered as a Component or passed to props of a Client Component.`)}); },
${stringifiedResourceKey},
"default",
);\n`
} else {
esmSource += `const e${cnt} = proxy["${ref}"];\n`
esmSource += `export { e${cnt++} as ${ref} };\n`
esmSource += `export const ${ref} = registerClientReference(
function() { throw new Error(${JSON.stringify(`Attempted to call ${ref}() from \
the server but ${ref} is on the client. It's not possible to invoke a client \
function from the server, it can only be rendered as a Component or passed to \
props of a Client Component.`)}); },
${stringifiedResourceKey},
${JSON.stringify(ref)},
);`
}
}

this.callback(null, esmSource, sourceMap)
return
return this.callback(null, esmSource, sourceMap)
} else if (assumedSourceType === 'commonjs') {
let cjsSource = `\
const { createProxy } = require("${MODULE_PROXY_PATH}")
module.exports = createProxy(${stringifiedResourceKey})
`

return this.callback(null, cjsSource, sourceMap)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,32 +72,29 @@ export type ActionManifest = {
edge: Actions
}

export interface ModuleInfo {
moduleId: string | number
async: boolean
}

const pluginState = getProxiedPluginState({
// A map to track "action" -> "list of bundles".
serverActions: {} as ActionManifest['node'],
edgeServerActions: {} as ActionManifest['edge'],

actionModServerId: {} as Record<
string,
{
server?: { moduleId: string | number; async: boolean }
client?: { moduleId: string | number; async: boolean }
}
>,
actionModEdgeServerId: {} as Record<
string,
{
server?: { moduleId: string | number; async: boolean }
client?: { moduleId: string | number; async: boolean }
}
>,
serverActionModules: {} as {
[workerName: string]: { server?: ModuleInfo; client?: ModuleInfo }
},

edgeServerActionModules: {} as {
[workerName: string]: { server?: ModuleInfo; client?: ModuleInfo }
},

// Mapping of resource path to module id for server/edge server.
serverModuleIds: {} as Record<string, string | number>,
edgeServerModuleIds: {} as Record<string, string | number>,
ssrModules: {} as { [ssrModuleId: string]: ModuleInfo },
edgeSsrModules: {} as { [ssrModuleId: string]: ModuleInfo },

rscModuleIds: {} as Record<string, string | number>,
edgeRscModuleIds: {} as Record<string, string | number>,
rscModules: {} as { [rscModuleId: string]: ModuleInfo },
edgeRscModules: {} as { [rscModuleId: string]: ModuleInfo },

injectedClientEntries: {} as Record<string, string>,
})
Expand Down Expand Up @@ -220,10 +217,15 @@ export class FlightClientEntryPlugin {
.relative(compiler.context, modResource)
.replace(/\/next\/dist\/esm\//, '/next/dist/')

const moduleInfo: ModuleInfo = {
moduleId: modId,
async: compilation.moduleGraph.isAsync(mod),
}

if (this.isEdgeServer) {
pluginState.edgeRscModuleIds[key] = modId
pluginState.edgeRscModules[key] = moduleInfo
} else {
pluginState.rscModuleIds[key] = modId
pluginState.rscModules[key] = moduleInfo
}
}
}
Expand All @@ -244,12 +246,17 @@ export class FlightClientEntryPlugin {
ssrNamedModuleId = `./${normalizePathSep(ssrNamedModuleId)}`
}

const moduleInfo: ModuleInfo = {
moduleId: modId,
async: compilation.moduleGraph.isAsync(mod),
}

if (this.isEdgeServer) {
pluginState.edgeServerModuleIds[
pluginState.edgeSsrModules[
ssrNamedModuleId.replace(/\/next\/dist\/esm\//, '/next/dist/')
] = modId
] = moduleInfo
} else {
pluginState.serverModuleIds[ssrNamedModuleId] = modId
pluginState.ssrModules[ssrNamedModuleId] = moduleInfo
}
}
}
Expand Down Expand Up @@ -968,8 +975,8 @@ export class FlightClientEntryPlugin {
const fromClient = /&__client_imported__=true/.test(mod.request)

const mapping = this.isEdgeServer
? pluginState.actionModEdgeServerId
: pluginState.actionModServerId
? pluginState.edgeServerActionModules
: pluginState.serverActionModules

if (!mapping[chunkGroup.name]) {
mapping[chunkGroup.name] = {}
Expand All @@ -985,7 +992,7 @@ export class FlightClientEntryPlugin {
const action = pluginState.serverActions[id]
for (let name in action.workers) {
const modId =
pluginState.actionModServerId[name][
pluginState.serverActionModules[name][
action.layer[name] === WEBPACK_LAYERS.actionBrowser
? 'client'
: 'server'
Expand All @@ -999,7 +1006,7 @@ export class FlightClientEntryPlugin {
const action = pluginState.edgeServerActions[id]
for (let name in action.workers) {
const modId =
pluginState.actionModEdgeServerId[name][
pluginState.edgeServerActionModules[name][
action.layer[name] === WEBPACK_LAYERS.actionBrowser
? 'client'
: 'server'
Expand Down
49 changes: 26 additions & 23 deletions packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
import type { ChunkGroup } from 'webpack'
import { encodeURIPath } from '../../../shared/lib/encode-uri-path'
import { isMetadataRoute } from '../../../lib/metadata/is-metadata-route'
import type { ModuleInfo } from './flight-client-entry-plugin'

interface Options {
dev: boolean
Expand All @@ -43,11 +44,11 @@ type ModuleId = string | number /*| null*/
export type ManifestChunks = Array<string>

const pluginState = getProxiedPluginState({
serverModuleIds: {} as Record<string, string | number>,
edgeServerModuleIds: {} as Record<string, string | number>,
ssrModules: {} as { [ssrModuleId: string]: ModuleInfo },
edgeSsrModules: {} as { [ssrModuleId: string]: ModuleInfo },

rscModuleIds: {} as Record<string, string | number>,
edgeRscModuleIds: {} as Record<string, string | number>,
rscModules: {} as { [rscModuleId: string]: ModuleInfo },
edgeRscModules: {} as { [rscModuleId: string]: ModuleInfo },
})

export interface ManifestNode {
Expand Down Expand Up @@ -361,7 +362,7 @@ export class ClientReferenceManifestPlugin {
id: modId,
name: '*',
chunks: requiredChunks,
async: false,
async: compilation.moduleGraph.isAsync(mod),
}
if (esmResource) {
const edgeExportName = esmResource
Expand All @@ -372,64 +373,66 @@ export class ClientReferenceManifestPlugin {

function addSSRIdMapping() {
const exportName = resource
if (
typeof pluginState.serverModuleIds[ssrNamedModuleId] !== 'undefined'
) {
const moduleInfo = pluginState.ssrModules[ssrNamedModuleId]

if (moduleInfo) {
moduleIdMapping[modId] = moduleIdMapping[modId] || {}
moduleIdMapping[modId]['*'] = {
...manifest.clientModules[exportName],
// During SSR, we don't have external chunks to load on the server
// side with our architecture of Webpack / Turbopack. We can keep
// this field empty to save some bytes.
chunks: [],
id: pluginState.serverModuleIds[ssrNamedModuleId],
id: moduleInfo.moduleId,
async: moduleInfo.async,
}
}

if (
typeof pluginState.edgeServerModuleIds[ssrNamedModuleId] !==
'undefined'
) {
const edgeModuleInfo = pluginState.edgeSsrModules[ssrNamedModuleId]

if (edgeModuleInfo) {
edgeModuleIdMapping[modId] = edgeModuleIdMapping[modId] || {}
edgeModuleIdMapping[modId]['*'] = {
...manifest.clientModules[exportName],
// During SSR, we don't have external chunks to load on the server
// side with our architecture of Webpack / Turbopack. We can keep
// this field empty to save some bytes.
chunks: [],
id: pluginState.edgeServerModuleIds[ssrNamedModuleId],
id: edgeModuleInfo.moduleId,
async: edgeModuleInfo.async,
}
}
}

function addRSCIdMapping() {
const exportName = resource
if (
typeof pluginState.rscModuleIds[rscNamedModuleId] !== 'undefined'
) {
const moduleInfo = pluginState.rscModules[rscNamedModuleId]

if (moduleInfo) {
rscIdMapping[modId] = rscIdMapping[modId] || {}
rscIdMapping[modId]['*'] = {
...manifest.clientModules[exportName],
// During SSR, we don't have external chunks to load on the server
// side with our architecture of Webpack / Turbopack. We can keep
// this field empty to save some bytes.
chunks: [],
id: pluginState.rscModuleIds[rscNamedModuleId],
id: moduleInfo.moduleId,
async: moduleInfo.async,
}
}

if (
typeof pluginState.edgeRscModuleIds[rscNamedModuleId] !==
'undefined'
) {
const edgeModuleInfo = pluginState.ssrModules[rscNamedModuleId]

if (edgeModuleInfo) {
edgeRscIdMapping[modId] = edgeRscIdMapping[modId] || {}
edgeRscIdMapping[modId]['*'] = {
...manifest.clientModules[exportName],
// During SSR, we don't have external chunks to load on the server
// side with our architecture of Webpack / Turbopack. We can keep
// this field empty to save some bytes.
chunks: [],
id: pluginState.edgeRscModuleIds[rscNamedModuleId],
id: edgeModuleInfo.moduleId,
async: edgeModuleInfo.async,
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/dynamic-import/app/button.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

export function Button() {
return <button>submit</button>
}
8 changes: 8 additions & 0 deletions test/e2e/app-dir/dynamic-import/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ReactNode } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
16 changes: 16 additions & 0 deletions test/e2e/app-dir/dynamic-import/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// x-ref: https://github.com/vercel/next.js/issues/71840

import { ElementType } from 'react'

async function getImport(
slug: string,
exportName: string
): Promise<ElementType> {
const moduleExports = await import(`./${slug}`)
return moduleExports[exportName]
}

export default async function Home() {
const Button = await getImport('button', 'Button')
return <Button />
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/dynamic-import/dynamic-import.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { nextTestSetup } from 'e2e-utils'

describe('dynamic-import', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should render the dynamically imported component', async () => {
const browser = await next.browser('/')
expect(await browser.elementByCss('button').text()).toBe('submit')
})
})
6 changes: 6 additions & 0 deletions test/e2e/app-dir/dynamic-import/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Loading

0 comments on commit 3713b3a

Please sign in to comment.