From bf49f62cda65fae565bbb2b90052de780aa58b53 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Thu, 4 May 2023 23:25:21 +0200 Subject: [PATCH] Fix Server Actions defined in both layers in one entry (#49248) This fixes an existing bug where there're Server Actions defined in both the "server" and "client" layers (client imported Actions). They have the same worker name as they exist in one route entry, but they're built into different modules and compiled differently (server and client layers). Because of that, each route entry can have 2 "action module entries". This PR adds the logic to differentiate that via a "layer" field so they don't conflict. --- .../plugins/flight-client-entry-plugin.ts | 116 ++++++++++++++---- test/e2e/app-dir/actions/app/client/layout.js | 9 ++ 2 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 test/e2e/app-dir/actions/app/client/layout.js diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index 688a52dd3511f..f4a7f9a4a3ab8 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -48,6 +48,10 @@ export type ActionManifest = { workers: { [name: string]: string | number } + // Record which layer the action is in (sc_server or sc_action), in the specific entry. + layer: { + [name: string]: string + } } } } @@ -56,8 +60,21 @@ const pluginState = getProxiedPluginState({ // A map to track "action" -> "list of bundles". serverActions: {} as ActionManifest['node'], edgeServerActions: {} as ActionManifest['edge'], - actionModServerId: {} as Record, - actionModEdgeServerId: {} as Record, + + actionModServerId: {} as Record< + string, + { + server?: string | number + client?: string | number + } + >, + actionModEdgeServerId: {} as Record< + string, + { + server?: string | number + client?: string | number + } + >, // Manifest of CSS entry files for server/edge server. serverCSSManifest: {} as ClientCSSReferenceManifest, @@ -170,6 +187,7 @@ export class ClientReferenceEntryPlugin { const addActionEntryList: Array> = [] + const actionMapsPerEntry: Record> = {} // For each SC server compilation entry, we need to create its corresponding // client component entry. @@ -247,19 +265,31 @@ export class ClientReferenceEntryPlugin { ) ) } else { - addActionEntryList.push( - this.injectActionEntry({ - compiler, - compilation, - actions: actionEntryImports, - entryName: name, - bundlePath: name, - }) - ) + if (!actionMapsPerEntry[name]) { + actionMapsPerEntry[name] = new Map() + } + actionMapsPerEntry[name] = new Map([ + ...actionMapsPerEntry[name], + ...actionEntryImports, + ]) } } }) + for (const [name, actionEntryImports] of Object.entries( + actionMapsPerEntry + )) { + addActionEntryList.push( + this.injectActionEntry({ + compiler, + compilation, + actions: actionEntryImports, + entryName: name, + bundlePath: name, + }) + ) + } + // To collect all CSS imports and action imports for a specific entry // including the ones that are in the client graph, we need to store a // map for client boundary dependencies. @@ -293,6 +323,7 @@ export class ClientReferenceEntryPlugin { // client layer. compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => { const addedClientActionEntryList: Promise[] = [] + const actionMapsPerClientEntry: Record> = {} forEachEntryModule(compilation, ({ name, entryModule }) => { const actionEntryImports = new Map() @@ -324,18 +355,31 @@ export class ClientReferenceEntryPlugin { } if (actionEntryImports.size > 0) { - addedClientActionEntryList.push( - this.injectActionEntry({ - compiler, - compilation, - actions: actionEntryImports, - entryName: name, - bundlePath: name, - fromClient: true, - }) - ) + if (!actionMapsPerClientEntry[name]) { + actionMapsPerClientEntry[name] = new Map() + } + actionMapsPerClientEntry[name] = new Map([ + ...actionMapsPerClientEntry[name], + ...actionEntryImports, + ]) } }) + + for (const [name, actionEntryImports] of Object.entries( + actionMapsPerClientEntry + )) { + addedClientActionEntryList.push( + this.injectActionEntry({ + compiler, + compilation, + actions: actionEntryImports, + entryName: name, + bundlePath: name, + fromClient: true, + }) + ) + } + return Promise.all(addedClientActionEntryList) }) @@ -719,6 +763,7 @@ export class ClientReferenceEntryPlugin { const actionsArray = Array.from(actions.entries()) const actionLoader = `next-flight-action-entry-loader?${stringify({ actions: JSON.stringify(actionsArray), + __client_imported__: fromClient, })}!` const currentCompilerServerActions = this.isEdgeServer @@ -730,9 +775,13 @@ export class ClientReferenceEntryPlugin { if (typeof currentCompilerServerActions[id] === 'undefined') { currentCompilerServerActions[id] = { workers: {}, + layer: {}, } } currentCompilerServerActions[id].workers[bundlePath] = '' + currentCompilerServerActions[id].layer[bundlePath] = fromClient + ? WEBPACK_LAYERS.action + : WEBPACK_LAYERS.server } } @@ -793,11 +842,16 @@ export class ClientReferenceEntryPlugin { mod.request && /next-flight-action-entry-loader/.test(mod.request) ) { - if (this.isEdgeServer) { - pluginState.actionModEdgeServerId[chunkGroup.name] = modId - } else { - pluginState.actionModServerId[chunkGroup.name] = modId + const fromClient = /&__client_imported__=true/.test(mod.request) + + const mapping = this.isEdgeServer + ? pluginState.actionModEdgeServerId + : pluginState.actionModServerId + + if (!mapping[chunkGroup.name]) { + mapping[chunkGroup.name] = {} } + mapping[chunkGroup.name][fromClient ? 'client' : 'server'] = modId } }) @@ -805,7 +859,11 @@ export class ClientReferenceEntryPlugin { for (let id in pluginState.serverActions) { const action = pluginState.serverActions[id] for (let name in action.workers) { - action.workers[name] = pluginState.actionModServerId[name] + const modId = + pluginState.actionModServerId[name][ + action.layer[name] === WEBPACK_LAYERS.action ? 'client' : 'server' + ] + action.workers[name] = modId! } serverActions[id] = action } @@ -814,7 +872,11 @@ export class ClientReferenceEntryPlugin { for (let id in pluginState.edgeServerActions) { const action = pluginState.edgeServerActions[id] for (let name in action.workers) { - action.workers[name] = pluginState.actionModEdgeServerId[name] + const modId = + pluginState.actionModEdgeServerId[name][ + action.layer[name] === WEBPACK_LAYERS.action ? 'client' : 'server' + ] + action.workers[name] = modId! } edgeServerActions[id] = action } diff --git a/test/e2e/app-dir/actions/app/client/layout.js b/test/e2e/app-dir/actions/app/client/layout.js new file mode 100644 index 0000000000000..616d7e9dbc085 --- /dev/null +++ b/test/e2e/app-dir/actions/app/client/layout.js @@ -0,0 +1,9 @@ +async function noopAction() { + 'use server' +} + +console.log(noopAction()) + +export default function Layout({ children }) { + return children +}