Skip to content

Commit

Permalink
Fix Server Actions defined in both layers in one entry (#49248)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shuding authored May 4, 2023
1 parent 7fa4946 commit bf49f62
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 27 deletions.
116 changes: 89 additions & 27 deletions packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
Expand All @@ -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<string, string | number>,
actionModEdgeServerId: {} as Record<string, string | number>,

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,
Expand Down Expand Up @@ -170,6 +187,7 @@ export class ClientReferenceEntryPlugin {

const addActionEntryList: Array<ReturnType<typeof this.injectActionEntry>> =
[]
const actionMapsPerEntry: Record<string, Map<string, string[]>> = {}

// For each SC server compilation entry, we need to create its corresponding
// client component entry.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -293,6 +323,7 @@ export class ClientReferenceEntryPlugin {
// client layer.
compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => {
const addedClientActionEntryList: Promise<any>[] = []
const actionMapsPerClientEntry: Record<string, Map<string, string[]>> = {}

forEachEntryModule(compilation, ({ name, entryModule }) => {
const actionEntryImports = new Map<string, string[]>()
Expand Down Expand Up @@ -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)
})

Expand Down Expand Up @@ -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
Expand All @@ -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
}
}

Expand Down Expand Up @@ -793,19 +842,28 @@ 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
}
})

const serverActions: ActionManifest['node'] = {}
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
}
Expand All @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/app-dir/actions/app/client/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
async function noopAction() {
'use server'
}

console.log(noopAction())

export default function Layout({ children }) {
return children
}

0 comments on commit bf49f62

Please sign in to comment.