Skip to content

Apply optimization for unused actions (#69178) #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MitchLewis930 wants to merge 1 commit into
base: pr_034_before
Choose a base branch
from
Open

Apply optimization for unused actions (#69178) #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 187 additions & 24 deletions packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
View file
Open in desktop

Large diffs are not rendered by default.

78 changes: 78 additions & 0 deletions test/production/app-dir/actions-tree-shaking/_testing/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { type NextInstance } from 'e2e-utils'

async function getActionsMappingByRuntime(
next: NextInstance,
runtime: 'node' | 'edge'
) {
const manifest = JSON.parse(
await next.readFile('.next/server/server-reference-manifest.json')
)

return manifest[runtime]
}
Comment on lines +3 to +12
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a guard for missing runtime mapping.

If the manifest doesn’t contain the requested runtime key, the later code will throw a cryptic error. Failing fast with a clear message makes test failures much easier to diagnose.

🔧 Suggested fix 
 async function getActionsMappingByRuntime(
   next: NextInstance,
   runtime: 'node' | 'edge'
 ) {
   const manifest = JSON.parse(
     await next.readFile('.next/server/server-reference-manifest.json')
   )
-
-  return manifest[runtime]
+  const mapping = manifest?.[runtime]
+  if (!mapping) {
+    throw new Error(`Missing server-reference-manifest runtime: ${runtime}`)
+  }
+  return mapping
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function getActionsMappingByRuntime(
next: NextInstance,
runtime: 'node' | 'edge'
) {
const manifest = JSON.parse(
await next.readFile('.next/server/server-reference-manifest.json')
)
return manifest[runtime]
}
async function getActionsMappingByRuntime(
next: NextInstance,
runtime: 'node' | 'edge'
) {
const manifest = JSON.parse(
await next.readFile('.next/server/server-reference-manifest.json')
)
const mapping = manifest?.[runtime]
if (!mapping) {
throw new Error(`Missing server-reference-manifest runtime: ${runtime}`)
}
return mapping
}
🤖 Prompt for AI Agents 
In `@test/production/app-dir/actions-tree-shaking/_testing/utils.ts` around lines
3 - 12, The function getActionsMappingByRuntime reads the
server-reference-manifest.json but doesn’t guard against a missing runtime key,
which leads to cryptic errors later; update getActionsMappingByRuntime to check
that manifest[runtime] is defined after parsing and, if not, throw a clear Error
(including the runtime value and maybe a short hint that the manifest is missing
that key) so callers fail fast and tests show an actionable message.


export function markLayoutAsEdge(next: NextInstance) {
beforeAll(async () => {
await next.stop()
const layoutContent = await next.readFile('app/layout.js')
await next.patchFile(
'app/layout.js',
layoutContent + `\nexport const runtime = 'edge'`
)
await next.start()
})
Comment on lines +14 to +23
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make layout patching idempotent to avoid duplicate exports.

If the layout already defines runtime or the helper is called twice, appending another export will cause a syntax error.

🔧 Suggested fix 
 export function markLayoutAsEdge(next: NextInstance) {
   beforeAll(async () => {
     await next.stop()
-    const layoutContent = await next.readFile('app/layout.js')
     await next.patchFile(
       'app/layout.js',
-      layoutContent + `\nexport const runtime = 'edge'`
+      (content) => {
+        if (/export const runtime\s*=/.test(content)) {
+          return content.replace(
+            /export const runtime\s*=\s*['"][^'"]+['"]/,
+            `export const runtime = 'edge'`
+          )
+        }
+        return content + `\nexport const runtime = 'edge'`
+      }
     )
     await next.start()
   })
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function markLayoutAsEdge(next: NextInstance) {
beforeAll(async () => {
await next.stop()
const layoutContent = await next.readFile('app/layout.js')
await next.patchFile(
'app/layout.js',
layoutContent + `\nexport const runtime = 'edge'`
)
await next.start()
})
export function markLayoutAsEdge(next: NextInstance) {
beforeAll(async () => {
await next.stop()
await next.patchFile(
'app/layout.js',
(content) => {
if (/export const runtime\s*=/.test(content)) {
return content.replace(
/export const runtime\s*=\s*['"][^'"]+['"]/,
`export const runtime = 'edge'`
)
}
return content + `\nexport const runtime = 'edge'`
}
)
await next.start()
})
}
🤖 Prompt for AI Agents 
In `@test/production/app-dir/actions-tree-shaking/_testing/utils.ts` around lines
14 - 23, The helper markLayoutAsEdge currently appends "export const runtime =
'edge'" unconditionally which can create duplicate exports and syntax errors;
modify the function to read the layout content (in markLayoutAsEdge), check
whether it already contains a runtime export (e.g., match
/export\s+const\s+runtime\s*=/ or runtime\s*=/) or the runtime assignment, and
only call next.patchFile to append the export when no existing runtime
export/assignment is found, ensuring idempotent behavior across multiple calls.

}

/*
{
[route path]: { [layer]: Set<workerId> ]
}
*/
Comment on lines +26 to +30
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify the mapping comment (bracket mismatch + outdated structure).

The comment currently mismatches brackets and doesn’t reflect the actual shape used in ActionsMappingOfRuntime.

✏️ Suggested fix 
-/* 
-{ 
-  [route path]: { [layer]: Set<workerId> ]
-}
-*/
+/*
+{
+  [actionId]: {
+    workers: { [route]: string },
+    layer: { [route]: string }
+  }
+}
+*/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/*
{
[route path]: { [layer]: Set<workerId> ]
}
*/
/*
{
[actionId]: {
workers: { [route]: string },
layer: { [route]: string }
}
}
*/
🤖 Prompt for AI Agents 
In `@test/production/app-dir/actions-tree-shaking/_testing/utils.ts` around lines
26 - 30, The comment above the mapping has mismatched brackets and does not
match the actual shape used by ActionsMappingOfRuntime; update the comment to a
correct, readable mapping that mirrors ActionsMappingOfRuntime's structure (use
the same key names and nesting: route path -> layer -> Set of worker IDs), fix
the bracket/brace placement and delimiters so it is valid and unambiguous, and
ensure the comment text references ActionsMappingOfRuntime so future readers can
cross-check the type.

type ActionsMappingOfRuntime = {
[actionId: string]: {
workers: {
[route: string]: string
}
layer: {
[route: string]: string
}
}
}
type ActionState = {
[route: string]: {
[layer: string]: number
}
}

function getActionsRoutesState(
actionsMappingOfRuntime: ActionsMappingOfRuntime
): ActionState {
const state: ActionState = {}
Object.keys(actionsMappingOfRuntime).forEach((actionId) => {
const action = actionsMappingOfRuntime[actionId]
const routePaths = Object.keys(action.workers)

routePaths.forEach((routePath) => {
if (!state[routePath]) {
state[routePath] = {}
}
const layer = action.layer[routePath]

if (!state[routePath][layer]) {
state[routePath][layer] = 0
}

state[routePath][layer]++
})
})

return state
}
Comment on lines +47 to +70
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate layer presence to avoid undefined buckets.

If action.layer[routePath] is missing, the state gets an undefined key, hiding a manifest inconsistency.

🔧 Suggested fix 
     routePaths.forEach((routePath) => {
       if (!state[routePath]) {
         state[routePath] = {}
       }
       const layer = action.layer[routePath]
+      if (!layer) {
+        throw new Error(
+          `Missing layer for action "${actionId}" on route "${routePath}"`
+        )
+      }
 
       if (!state[routePath][layer]) {
         state[routePath][layer] = 0
       }
🤖 Prompt for AI Agents 
In `@test/production/app-dir/actions-tree-shaking/_testing/utils.ts` around lines
47 - 70, In getActionsRoutesState, validate that action.layer[routePath] is
defined before using it to index state: retrieve const layer =
action.layer?.[routePath]; if layer is undefined, throw or otherwise surface an
explicit error mentioning the offending actionId and routePath (to highlight the
manifest inconsistency) instead of allowing state[routePath][undefined] to be
created; otherwise proceed to increment state[routePath][layer] as before.


export async function getActionsRoutesStateByRuntime(next: NextInstance) {
const actionsMappingOfRuntime = await getActionsMappingByRuntime(
next,
process.env.TEST_EDGE ? 'edge' : 'node'
)
return getActionsRoutesState(actionsMappingOfRuntime)
}
13 changes: 13 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/app/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function serverComponentAction() {
return 'server-action'
}

export async function clientComponentAction() {
return 'client-action'
}

export async function unusedExportedAction() {
return 'unused-exported-action'
}
21 changes: 21 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/app/client/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use client'

import { useState } from 'react'
import { clientComponentAction } from '../actions'

export default function Page() {
const [text, setText] = useState('initial')
return (
<div>
<button
id="action-1"
onClick={async () => {
setText(await clientComponentAction())
}}
>
Action 1
</button>
<span>{text}</span>
</div>
)
}
13 changes: 13 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/app/inline/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export default function Page() {
// Inline Server Action
async function inlineServerAction() {
'use server'
return 'inline-server-action'
}

return (
<form action={inlineServerAction}>
<button type="submit">Submit</button>
</form>
)
}
7 changes: 7 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/app/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
10 changes: 10 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/app/server/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { serverComponentAction } from '../actions'

export default function Page() {
return (
<form>
<input type="text" placeholder="input" />
<button formAction={serverComponentAction}>submit</button>
</form>
)
}
3 changes: 3 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/basic-edge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
process.env.TEST_EDGE = '1'

require('./basic.test')
Comment on lines +1 to +3
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Restore TEST_EDGE to avoid cross-test contamination.

Setting process.env.TEST_EDGE can leak into other tests running in the same worker. Capture and restore after requiring the module.

🔧 Suggested fix 
-process.env.TEST_EDGE = '1'
-
-require('./basic.test')
+const prevTestEdge = process.env.TEST_EDGE
+process.env.TEST_EDGE = '1'
+
+require('./basic.test')
+
+if (prevTestEdge === undefined) {
+  delete process.env.TEST_EDGE
+} else {
+  process.env.TEST_EDGE = prevTestEdge
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
process.env.TEST_EDGE = '1'
require('./basic.test')
const prevTestEdge = process.env.TEST_EDGE
process.env.TEST_EDGE = '1'
require('./basic.test')
if (prevTestEdge === undefined) {
delete process.env.TEST_EDGE
} else {
process.env.TEST_EDGE = prevTestEdge
}
🤖 Prompt for AI Agents 
In `@test/production/app-dir/actions-tree-shaking/basic/basic-edge.test.ts` around
lines 1 - 3, The test sets process.env.TEST_EDGE = '1' and requires
'./basic.test' which can leak into other tests; to fix, capture the original
value (e.g., const prev = process.env.TEST_EDGE), set process.env.TEST_EDGE =
'1', require('./basic.test'), then restore the original value after the require
by setting process.env.TEST_EDGE = prev or deleting it if prev is undefined so
the environment is returned to its prior state.

33 changes: 33 additions & 0 deletions test/production/app-dir/actions-tree-shaking/basic/basic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { nextTestSetup } from 'e2e-utils'
import {
getActionsRoutesStateByRuntime,
markLayoutAsEdge,
} from '../_testing/utils'

describe('actions-tree-shaking - basic', () => {
const { next } = nextTestSetup({
files: __dirname,
})

if (process.env.TEST_EDGE) {
markLayoutAsEdge(next)
}

it('should not have the unused action in the manifest', async () => {
const actionsRoutesState = await getActionsRoutesStateByRuntime(next)

expect(actionsRoutesState).toMatchObject({
// only one server layer action
'app/server/page': {
rsc: 1,
},
// only one browser layer action
'app/client/page': {
'action-browser': 1,
},
'app/inline/page': {
rsc: 1,
},
})
})
})
7 changes: 7 additions & 0 deletions test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
13 changes: 13 additions & 0 deletions ...duction/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function esmModuleTypeAction() {
return 'esm-module-type-action'
}

export async function cjsModuleTypeAction() {
return 'cjs-module-type-action'
}

export async function unusedModuleTypeAction1() {
return 'unused-module-type-action-1'
}
13 changes: 13 additions & 0 deletions ...production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const { cjsModuleTypeAction } = require('./actions')

export default function Page() {
return (
<div>
<h3>One</h3>
<form>
<input type="text" placeholder="input" />
<button formAction={cjsModuleTypeAction}>submit</button>
</form>
</div>
)
}
13 changes: 13 additions & 0 deletions ...duction/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function esmModuleTypeAction() {
return 'esm-module-type-action'
}

export async function cjsModuleTypeAction() {
return 'cjs-module-type-action'
}

export async function unusedModuleTypeAction1() {
return 'unused-module-type-action-1'
}
13 changes: 13 additions & 0 deletions ...production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { esmModuleTypeAction } from './actions'

export default function Page() {
return (
<div>
<h3>One</h3>
<form>
<input type="text" placeholder="input" />
<button formAction={esmModuleTypeAction}>submit</button>
</form>
</div>
)
}
3 changes: 3 additions & 0 deletions ...ction/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions-edge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
process.env.TEST_EDGE = '1'

require('./mixed-module-actions.test')
29 changes: 29 additions & 0 deletions ...production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { nextTestSetup } from 'e2e-utils'
import {
getActionsRoutesStateByRuntime,
markLayoutAsEdge,
} from '../_testing/utils'

describe('actions-tree-shaking - mixed-module-actions', () => {
const { next } = nextTestSetup({
files: __dirname,
})

if (process.env.TEST_EDGE) {
markLayoutAsEdge(next)
}

it('should not do tree shake for cjs module when import server actions', async () => {
const actionsRoutesState = await getActionsRoutesStateByRuntime(next)

expect(actionsRoutesState).toMatchObject({
'app/mixed-module/esm/page': {
rsc: 1,
},
// CJS import is not able to tree shake, so it will include all actions
'app/mixed-module/cjs/page': {
rsc: 3,
},
})
})
})
7 changes: 7 additions & 0 deletions test/production/app-dir/actions-tree-shaking/reexport/app/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
13 changes: 13 additions & 0 deletions test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function sharedClientLayerAction() {
return 'shared-client-layer-action'
}

export async function unusedClientLayerAction1() {
return 'unused-client-layer-action-1'
}

export async function unusedClientLayerAction2() {
return 'unused-client-layer-action-2'
}
21 changes: 21 additions & 0 deletions test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use client'

import { useState } from 'react'
import { sharedClientLayerAction } from './reexport-action'

export default function Page() {
const [text, setText] = useState('initial')
return (
<div>
<button
id="action-1"
onClick={async () => {
setText(await sharedClientLayerAction())
}}
>
Action 1
</button>
<span>{text}</span>
</div>
)
}
5 changes: 5 additions & 0 deletions ...uction/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/reexport-action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export {
sharedClientLayerAction,
unusedClientLayerAction1,
unusedClientLayerAction2,
} from './actions'
13 changes: 13 additions & 0 deletions test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function sharedServerLayerAction() {
return 'shared-server-layer-action'
}

export async function unusedServerLayerAction1() {
return 'unused-server-layer-action-1'
}

export async function unusedServerLayerAction2() {
return 'unused-server-layer-action-2'
}
12 changes: 12 additions & 0 deletions test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { sharedServerLayerAction } from './reexport-action'

export default function Page() {
return (
<div>
<form>
<input type="text" placeholder="input" />
<button formAction={sharedServerLayerAction}>submit</button>
</form>
</div>
)
}
5 changes: 5 additions & 0 deletions ...uction/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/reexport-action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export {
sharedServerLayerAction,
unusedServerLayerAction1,
unusedServerLayerAction2,
} from './actions'
13 changes: 13 additions & 0 deletions ...production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use server'

export async function sharedClientLayerAction() {
return 'shared-client-layer-action'
}

export async function unusedClientLayerAction1() {
return 'unused-client-layer-action-1'
}

export async function unusedClientLayerAction2() {
return 'unused-client-layer-action-2'
}
Loading