Apply optimization for unused actions (#69178)#4
Apply optimization for unused actions (#69178)#4MitchLewis930 wants to merge 1 commit intopr_034_beforefrom
Conversation
📝 WalkthroughWalkthroughIntroduces per-entry, per-runtime tracking of server actions in the Flight client entry plugin to enable tree-shaking of unused actions. Adds comprehensive test infrastructure for validating actions tree-shaking across basic, mixed-module, reexport, and shared-module scenarios on both Node and Edge runtimes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts`:
- Around line 964-995: The loop that filters actions (using
getUsedActionsInEntry and isInlineActionIdentifier) can leave Map entries with
an empty array which later creates empty action bundles; after computing
filteredNames in the branch that currently calls actions.set(filePath,
filteredNames), if filteredNames.length === 0 delete the entry
(actions.delete(filePath)) instead of setting an empty array, and also move the
creation of actionsArray = Array.from(actions.entries()) to after this entire
filtering loop so it reflects the cleaned Map; reference getUsedActionsInEntry,
isInlineActionIdentifier, actions Map, createdActions, and entryName when making
the change.
- Around line 210-229: setUsedActionsInEntry currently uses actionNames.forEach
with an arrow callback that implicitly returns the value of Set.add(),
triggering the useIterableCallbackReturn rule; replace that forEach with a
for...of loop over actionNames and call actionsMap[modResource].add(name) inside
the loop to avoid returning the add result while preserving the same behavior
(refer to setUsedActionsInEntry, usedActions, actionsMap, and modResource to
locate the change).
In `@test/production/app-dir/actions-tree-shaking/_testing/utils.ts`:
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
In `@test/production/app-dir/actions-tree-shaking/basic/basic-edge.test.ts`:
- Around line 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.
In
`@test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js`:
- Line 6: Update the heading text in server/two/page.js: change the <h3> content
currently reading "One" to "Two" so the displayed heading matches the file's
location (server/two/page.js) and avoids the copy-paste mismatch from
server/one/page.js.
🧹 Nitpick comments (3)
test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts (1)
12-14: Conditional hook registration at describe body level.The
markLayoutAsEdge(next)call registers abeforeAllhook conditionally based onTEST_EDGE. While this works because Jest/test runners evaluate the describe body during test collection, this pattern can be fragile. If the environment variable check timing changes, the hook may not register as expected.Consider moving the conditional inside the
beforeAllcallback withinmarkLayoutAsEdge, or documenting that this pattern relies on synchronous describe-body evaluation.test/production/app-dir/actions-tree-shaking/basic/basic.test.ts (1)
7-31: Tighten the assertion to catch extra (unused) actions.
toMatchObjectallows additional keys, so unused actions could slip in without failing the test. Consider a strict equality check.🔧 Suggested fix
- expect(actionsRoutesState).toMatchObject({ + expect(actionsRoutesState).toEqual({ // 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, }, })packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts (1)
586-610: Consider recording used actions on first encounter.Used actions are only recorded on revisits today, which can skip single-path imports and reduce tree-shaking. Recording when
actionsexist and identifiers are present would make tracking more complete.💡 Possible refinement
- if (visitedModule.has(modResource) && actions) { - this.setUsedActionsInEntry(entryName, modResource, ids) - } - - if (visitedModule.has(modResource)) return + if (actions && ids.length > 0) { + this.setUsedActionsInEntry(entryName, modResource, ids) + } + + if (visitedModule.has(modResource)) return
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
packages/next/src/build/webpack/plugins/flight-client-entry-plugin.tstest/production/app-dir/actions-tree-shaking/_testing/utils.tstest/production/app-dir/actions-tree-shaking/basic/app/actions.jstest/production/app-dir/actions-tree-shaking/basic/app/client/page.jstest/production/app-dir/actions-tree-shaking/basic/app/inline/page.jstest/production/app-dir/actions-tree-shaking/basic/app/layout.jstest/production/app-dir/actions-tree-shaking/basic/app/server/page.jstest/production/app-dir/actions-tree-shaking/basic/basic-edge.test.tstest/production/app-dir/actions-tree-shaking/basic/basic.test.tstest/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.jstest/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.jstest/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.jstest/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.jstest/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/page.jstest/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions-edge.test.tstest/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.tstest/production/app-dir/actions-tree-shaking/reexport/app/layout.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/page.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/reexport-action.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.jstest/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/reexport-action.jstest/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.jstest/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/page.jstest/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.jstest/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.jstest/production/app-dir/actions-tree-shaking/reexport/reexport-edge.test.tstest/production/app-dir/actions-tree-shaking/reexport/reexport.test.tstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/one/page.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/two/page.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.jstest/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions-edge.test.tstest/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.tstest/turbopack-build-tests-manifest.json
🧰 Additional context used
🧬 Code graph analysis (31)
test/production/app-dir/actions-tree-shaking/reexport/app/layout.js (3)
test/production/app-dir/actions-tree-shaking/basic/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.js (1)
Layout(1-7)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts (1)
test/production/app-dir/actions-tree-shaking/_testing/utils.ts (2)
markLayoutAsEdge(14-24)getActionsRoutesStateByRuntime(72-78)
test/production/app-dir/actions-tree-shaking/basic/basic-edge.test.ts (1)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js (1)
require(1-1)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.js (3)
test/production/app-dir/actions-tree-shaking/basic/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/reexport/app/layout.js (1)
Layout(1-7)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/two/page.js (4)
test/production/app-dir/actions-tree-shaking/basic/app/client/page.js (2)
Page(6-21)text(7-7)test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js (1)
sharedClientLayerAction(3-5)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js (1)
sharedClientLayerAction(3-5)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (1)
sharedClientLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions-edge.test.ts (1)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js (1)
require(1-1)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/one/page.js (1)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (1)
sharedClientLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js (6)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (1)
sharedServerLayerAction(3-5)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (1)
sharedServerLayerAction(3-5)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (1)
sharedServerLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.js (1)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.js (3)
esmModuleTypeAction(3-5)cjsModuleTypeAction(7-9)unusedModuleTypeAction1(11-13)
test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (2)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)
test/production/app-dir/actions-tree-shaking/basic/app/server/page.js (1)
test/production/app-dir/actions-tree-shaking/basic/app/actions.js (1)
serverComponentAction(3-5)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js (5)
test/production/app-dir/actions-tree-shaking/basic/app/server/page.js (1)
Page(3-10)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (1)
sharedServerLayerAction(3-5)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (1)
sharedServerLayerAction(3-5)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (1)
sharedServerLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (2)
test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (2)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js (3)
sharedClientLayerAction(3-5)unusedClientLayerAction1(7-9)unusedClientLayerAction2(11-13)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js (3)
sharedClientLayerAction(3-5)unusedClientLayerAction1(7-9)unusedClientLayerAction2(11-13)
test/production/app-dir/actions-tree-shaking/basic/app/layout.js (3)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/reexport/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.js (1)
Layout(1-7)
test/production/app-dir/actions-tree-shaking/_testing/utils.ts (1)
test/lib/next-modes/base.ts (1)
NextInstance(52-605)
test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions-edge.test.ts (1)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js (1)
require(1-1)
test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js (2)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js (3)
sharedClientLayerAction(3-5)unusedClientLayerAction1(7-9)unusedClientLayerAction2(11-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (3)
sharedClientLayerAction(3-5)unusedClientLayerAction1(7-9)unusedClientLayerAction2(11-13)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/page.js (2)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.js (1)
esmModuleTypeAction(3-5)test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.js (1)
esmModuleTypeAction(3-5)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js (3)
test/production/app-dir/actions-tree-shaking/basic/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/reexport/app/layout.js (1)
Layout(1-7)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.js (1)
Layout(1-7)
test/production/app-dir/actions-tree-shaking/basic/app/inline/page.js (7)
test/production/app-dir/actions-tree-shaking/basic/app/server/page.js (1)
Page(3-10)test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js (1)
Page(3-13)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/page.js (3)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js (1)
sharedClientLayerAction(3-5)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js (1)
sharedClientLayerAction(3-5)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (1)
sharedClientLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts (1)
test/production/app-dir/actions-tree-shaking/_testing/utils.ts (2)
markLayoutAsEdge(14-24)getActionsRoutesStateByRuntime(72-78)
test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/page.js (3)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/page.js (2)
Page(6-21)text(7-7)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/one/page.js (2)
Page(6-22)text(7-7)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/two/page.js (2)
Page(6-22)text(7-7)
test/production/app-dir/actions-tree-shaking/basic/app/client/page.js (1)
test/production/app-dir/actions-tree-shaking/basic/app/actions.js (1)
clientComponentAction(7-9)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.js (2)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (1)
sharedServerLayerAction(3-5)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.js (1)
test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.js (3)
esmModuleTypeAction(3-5)cjsModuleTypeAction(7-9)unusedModuleTypeAction1(11-13)
test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts (1)
test/production/app-dir/actions-tree-shaking/_testing/utils.ts (2)
markLayoutAsEdge(14-24)getActionsRoutesStateByRuntime(72-78)
test/production/app-dir/actions-tree-shaking/basic/basic.test.ts (1)
test/production/app-dir/actions-tree-shaking/_testing/utils.ts (2)
markLayoutAsEdge(14-24)getActionsRoutesStateByRuntime(72-78)
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (2)
test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (3)
sharedServerLayerAction(3-5)unusedServerLayerAction1(7-9)unusedServerLayerAction2(11-13)
test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.js (4)
test/production/app-dir/actions-tree-shaking/basic/app/server/page.js (1)
Page(3-10)test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js (1)
Page(3-12)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.js (1)
Page(3-13)test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js (1)
Page(3-13)
🪛 Biome (2.3.13)
packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
[error] 229-229: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (40)
test/production/app-dir/actions-tree-shaking/basic/app/server/page.js (1)
1-10: LGTM!The server component correctly imports and uses
serverComponentActionvia theformActionprop on the button, which is the standard pattern for binding server actions in Next.js App Router.test/production/app-dir/actions-tree-shaking/basic/app/layout.js (1)
1-7: LGTM!This is an appropriate minimal root layout for the test fixture. The structure is consistent with other layout files in the actions-tree-shaking test suite.
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/layout.js (1)
1-7: LGTM!Minimal root layout appropriate for test fixture purposes, consistent with the other layout files in this test suite.
test/production/app-dir/actions-tree-shaking/basic/app/inline/page.js (1)
1-13: LGTM!The inline server action pattern is correctly implemented with the
'use server'directive inside the function body. This provides good test coverage for the tree-shaking behavior of inline server actions alongside the externally-defined actions in other test pages.test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts (1)
16-34: LGTM!The test correctly validates the actions manifest state for both namespace and named re-export scenarios. The comment on lines 26-32 appropriately documents the known limitation that named re-exports aren't tree-shaken, which is valuable for understanding expected behavior.
test/production/app-dir/actions-tree-shaking/reexport/reexport-edge.test.ts (1)
1-3: LGTM!This follows the established pattern in the test suite for edge runtime variants, setting
TEST_EDGEbefore requiring the base test module to ensure the environment variable is available during test collection.test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/layout.js (1)
1-7: LGTM!Minimal root layout consistent with other test fixtures in the actions-tree-shaking test suite.
test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/actions.js (1)
1-13: Looks good for test coverage.
Simple, deterministic server actions align with the shared-module-actions fixture intent.test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/two/page.js (1)
1-22: LGTM for client action usage.
The client component correctly invokes the shared action and updates state, matching the test intent.test/production/app-dir/actions-tree-shaking/basic/app/actions.js (1)
1-13: Fixture actions are clear and consistent.
These exports are minimal and suitable for the tree-shaking tests.test/production/app-dir/actions-tree-shaking/basic/app/client/page.js (1)
1-21: Client action wiring looks good.
State updates and async action invocation follow the established fixture pattern.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/actions.js (1)
1-13: Good fixture module for reexport tests.
Action definitions are minimal and consistent with the suite.test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/actions.js (1)
1-13: ESM actions fixture is solid.
Definitions are clear and consistent with mixed-module test expectations.test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/esm/page.js (1)
1-13: Form action usage is correct.
The server component cleanly wires the ESM action viaformAction, fitting the test scenario.test/production/app-dir/actions-tree-shaking/shared-module-actions/app/client/one/page.js (1)
1-22: LGTM!The client component correctly uses the
'use client'directive, properly imports and invokes the server action, and handles state updates appropriately. This serves as a clean test fixture for validating tree-shaking behavior.test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/actions.js (1)
1-13: LGTM!The server actions module correctly uses the
'use server'directive and follows the established pattern for tree-shaking test fixtures. The unused actions (unusedServerLayerAction1,unusedServerLayerAction2) are intentionally exported to validate that they get tree-shaken when not imported by any page.test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/one/page.js (1)
1-13: LGTM!The server component correctly imports and binds the server action via
formAction. The structure is appropriate for testing shared-module action tree-shaking.test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts (1)
1-34: LGTM!The test correctly validates tree-shaking behavior by asserting that:
- Server pages (
app/server/one/page,app/server/two/page) have exactly 1 RSC action- Client pages (
app/client/one/page,app/client/two/page) have exactly 1 action-browser entryThis confirms that the unused server/client layer actions are properly tree-shaken from the manifest. The conditional
markLayoutAsEdgecall follows the established pattern in the test suite.test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions-edge.test.ts (1)
1-3: LGTM!Follows the established pattern for edge-runtime test variants in the suite. Setting
TEST_EDGEbefore requiring the main test ensures the edge-specific setup is triggered.test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/actions.js (1)
1-13: LGTM!The
'use server'directive is correct here. Despite the "ClientLayer" naming and location in theclient/directory, these are server actions designed to be called from client components. The naming reflects the layer from which they're invoked, not where they execute. This follows the established pattern in the test suite.test/production/app-dir/actions-tree-shaking/_testing/utils.ts (2)
31-45: Types look clear and appropriately scoped.
72-77: LGTM. The runtime selection logic is straightforward and consistent with the test setup.test/turbopack-build-tests-manifest.json (1)
16569-16640: Verify the intended status of these actions-tree-shaking tests in the Turbopack manifest.All eight newly added entries list their assertions under
failed. Since these tests are part of the actions optimization feature, marking them as expected failures will silently allow regressions if the optimization is supposed to work under Turbopack. Either move them topassedonce they succeed, or add a reference to a tracking issue explaining why they're expected to fail.packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts (4)
75-83: Per-runtime used-actions state looks good.Nice addition for tracking used actions by runtime; aligns well with the new per-entry tracking flow.
319-555: Entry-scoped action wiring and dedupe look solid.Passing
entryNamethrough collection and trackingcreatedActionsshould prevent duplicate action entries across server/client layers.
689-811: Action traversal split + runtime-aware CSS usage looks good.The dedicated action traversal alongside runtime-specific module usage checks is clean and cohesive.
1221-1224: Inline-action identifier helper looks good.test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions-edge.test.ts (1)
1-3: Edge test harness wrapper looks good.test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/page.js (1)
1-13: CJS action page wiring looks good.test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts (1)
1-29: Mixed-module tree-shaking assertions look good.test/production/app-dir/actions-tree-shaking/reexport/app/layout.js (1)
1-7: Layout fixture matches the other suites.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/page.js (1)
1-12: Server page wiring to reexported action looks good.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/reexport-action.js (1)
1-5: Re-export barrel aligns with the test intent.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/page.js (1)
1-21: LGTM — clear client action wiring.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/server/reexport-action.js (1)
1-5: LGTM — clean re-export surface.test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/page.js (1)
1-12: LGTM — correct namespace action wiring.test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/client/page.js (1)
1-21: LGTM — consistent client action usage.test/production/app-dir/actions-tree-shaking/reexport/app/namespace-reexport/server/actions.js (1)
1-13: LGTM — straightforward server actions.test/production/app-dir/actions-tree-shaking/mixed-module-actions/app/mixed-module/cjs/actions.js (1)
1-13: LGTM — mixed-module actions look good.test/production/app-dir/actions-tree-shaking/reexport/app/named-reexport/client/actions.js (1)
1-13: LGTM — clean action stubs for tests.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| setUsedActionsInEntry( | ||
| entryName: string, | ||
| modResource: string, | ||
| actionNames: string[] | ||
| ) { | ||
| const runtime = this.isEdgeServer ? 'edge' : 'node' | ||
| if (!this.usedActions[entryName]) { | ||
| this.usedActions[entryName] = { | ||
| node: {}, | ||
| edge: {}, | ||
| } | ||
| } | ||
| if (!this.usedActions[entryName][runtime]) { | ||
| this.usedActions[entryName][runtime] = {} | ||
| } | ||
| const actionsMap = this.usedActions[entryName][runtime] | ||
| if (!actionsMap[modResource]) { | ||
| actionsMap[modResource] = new Set() | ||
| } | ||
| actionNames.forEach((name) => actionsMap[modResource].add(name)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "flight-client-entry-plugin.ts" -type fRepository: Signal65/next.js-CodeRabbit
Length of output: 142
🏁 Script executed:
sed -n '210,229p' ./packages/next/src/build/webpack/plugins/flight-client-entry-plugin.tsRepository: Signal65/next.js-CodeRabbit
Length of output: 675
🏁 Script executed:
find . -name "biome.json" -o -name ".biomerc*" -o -name "biome.jsonc" | head -20Repository: Signal65/next.js-CodeRabbit
Length of output: 83
🏁 Script executed:
cat ./biome.jsonRepository: Signal65/next.js-CodeRabbit
Length of output: 1846
🌐 Web query:
Biome linter rule arrow function callback return value forEach Set.add
💡 Result:
In Biome, the rule that flags this pattern is lint/suspicious/useIterableCallbackReturn (useIterableCallbackReturn). It enforces “return required vs return disallowed” for iterable callbacks, and specifically disallows returning a value from forEach callbacks. (biomejs.dev)
So code like:
items.forEach((x) => set.add(x)); // returns the Set from Set.add(...)is reported because the arrow expression implicitly returns a value (here, Set.prototype.add returns the set).
Fixes (keep side-effect, return nothing):
items.forEach((x) => { set.add(x); });or
items.forEach((x) => void set.add(x));If you actually want to discourage forEach entirely, Biome also has lint/complexity/noForEach. (biomejs.dev)
Citations:
- 1: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 2: https://biomejs.dev/linter/rules/no-for-each/?utm_source=openai
🌐 Web query:
Biome recommended rules list useIterableCallbackReturn
💡 Result:
lint/suspicious/useIterableCallbackReturn is a recommended Biome rule, which means it’s enabled by default (default severity: error) starting in Biome v2.0.0. [1]
To see the full “recommended rules” list, use Biome’s rules catalog page—rules marked as “recommended” are indicated there (Biome also notes the linter runs only recommended rules by default). [2] [3]
If you want to explicitly set it in biome.json:
{
"linter": {
"rules": {
"suspicious": {
"useIterableCallbackReturn": "error"
}
}
}
}[1]
Sources: Biome rule docs and rules catalog. [1] [2] [3]
Use a for...of loop instead of forEach to avoid implicitly returning the result of Set.add().
The arrow function callback implicitly returns the value of Set.add(), which Biome's useIterableCallbackReturn rule flags. A block body or for...of loop avoids the return value.
🔧 Suggested fix
- actionNames.forEach((name) => actionsMap[modResource].add(name))
+ for (const name of actionNames) {
+ actionsMap[modResource].add(name)
+ }📝 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.
| setUsedActionsInEntry( | |
| entryName: string, | |
| modResource: string, | |
| actionNames: string[] | |
| ) { | |
| const runtime = this.isEdgeServer ? 'edge' : 'node' | |
| if (!this.usedActions[entryName]) { | |
| this.usedActions[entryName] = { | |
| node: {}, | |
| edge: {}, | |
| } | |
| } | |
| if (!this.usedActions[entryName][runtime]) { | |
| this.usedActions[entryName][runtime] = {} | |
| } | |
| const actionsMap = this.usedActions[entryName][runtime] | |
| if (!actionsMap[modResource]) { | |
| actionsMap[modResource] = new Set() | |
| } | |
| actionNames.forEach((name) => actionsMap[modResource].add(name)) | |
| setUsedActionsInEntry( | |
| entryName: string, | |
| modResource: string, | |
| actionNames: string[] | |
| ) { | |
| const runtime = this.isEdgeServer ? 'edge' : 'node' | |
| if (!this.usedActions[entryName]) { | |
| this.usedActions[entryName] = { | |
| node: {}, | |
| edge: {}, | |
| } | |
| } | |
| if (!this.usedActions[entryName][runtime]) { | |
| this.usedActions[entryName][runtime] = {} | |
| } | |
| const actionsMap = this.usedActions[entryName][runtime] | |
| if (!actionsMap[modResource]) { | |
| actionsMap[modResource] = new Set() | |
| } | |
| for (const name of actionNames) { | |
| actionsMap[modResource].add(name) | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 229-229: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts` around
lines 210 - 229, setUsedActionsInEntry currently uses actionNames.forEach with
an arrow callback that implicitly returns the value of Set.add(), triggering the
useIterableCallbackReturn rule; replace that forEach with a for...of loop over
actionNames and call actionsMap[modResource].add(name) inside the loop to avoid
returning the add result while preserving the same behavior (refer to
setUsedActionsInEntry, usedActions, actionsMap, and modResource to locate the
change).
| // Filter out the unused actions before create action entry. | ||
| for (const [filePath, names] of actions.entries()) { | ||
| const usedActionNames = this.getUsedActionsInEntry(entryName, filePath) | ||
| if (!usedActionNames) continue | ||
| const containsAll = usedActionNames.has('*') | ||
| if (usedActionNames && !containsAll) { | ||
| const filteredNames = names.filter( | ||
| (name) => usedActionNames.has(name) || isInlineActionIdentifier(name) | ||
| ) | ||
| actions.set(filePath, filteredNames) | ||
| } else if (!containsAll) { | ||
| // If we didn't collect the used, we erase them from the collected actions | ||
| // to avoid creating the action entry. | ||
| if ( | ||
| names.filter((name) => !isInlineActionIdentifier(name)).length === 0 | ||
| ) { | ||
| actions.delete(filePath) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const actionsArray = Array.from(actions.entries()) | ||
|
|
||
| for (const [dep, actionNames] of actions) { | ||
| for (const actionName of actionNames) { | ||
| createdActions.add(entryName + '@' + dep + '@' + actionName) | ||
| } | ||
| } | ||
|
|
||
| if (actionsArray.length === 0) { | ||
| return Promise.resolve() | ||
| } |
There was a problem hiding this comment.
Drop file entries when filtering removes all actions.
After filtering, a file path can remain with an empty action list, which still creates an entry. Consider deleting those entries to avoid empty action bundles.
🧹 Suggested guard
- const filteredNames = names.filter(
+ const filteredNames = names.filter(
(name) => usedActionNames.has(name) || isInlineActionIdentifier(name)
)
- actions.set(filePath, filteredNames)
+ if (filteredNames.length === 0) {
+ actions.delete(filePath)
+ } else {
+ actions.set(filePath, filteredNames)
+ }📝 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.
| // Filter out the unused actions before create action entry. | |
| for (const [filePath, names] of actions.entries()) { | |
| const usedActionNames = this.getUsedActionsInEntry(entryName, filePath) | |
| if (!usedActionNames) continue | |
| const containsAll = usedActionNames.has('*') | |
| if (usedActionNames && !containsAll) { | |
| const filteredNames = names.filter( | |
| (name) => usedActionNames.has(name) || isInlineActionIdentifier(name) | |
| ) | |
| actions.set(filePath, filteredNames) | |
| } else if (!containsAll) { | |
| // If we didn't collect the used, we erase them from the collected actions | |
| // to avoid creating the action entry. | |
| if ( | |
| names.filter((name) => !isInlineActionIdentifier(name)).length === 0 | |
| ) { | |
| actions.delete(filePath) | |
| } | |
| } | |
| } | |
| const actionsArray = Array.from(actions.entries()) | |
| for (const [dep, actionNames] of actions) { | |
| for (const actionName of actionNames) { | |
| createdActions.add(entryName + '@' + dep + '@' + actionName) | |
| } | |
| } | |
| if (actionsArray.length === 0) { | |
| return Promise.resolve() | |
| } | |
| // Filter out the unused actions before create action entry. | |
| for (const [filePath, names] of actions.entries()) { | |
| const usedActionNames = this.getUsedActionsInEntry(entryName, filePath) | |
| if (!usedActionNames) continue | |
| const containsAll = usedActionNames.has('*') | |
| if (usedActionNames && !containsAll) { | |
| const filteredNames = names.filter( | |
| (name) => usedActionNames.has(name) || isInlineActionIdentifier(name) | |
| ) | |
| if (filteredNames.length === 0) { | |
| actions.delete(filePath) | |
| } else { | |
| actions.set(filePath, filteredNames) | |
| } | |
| } else if (!containsAll) { | |
| // If we didn't collect the used, we erase them from the collected actions | |
| // to avoid creating the action entry. | |
| if ( | |
| names.filter((name) => !isInlineActionIdentifier(name)).length === 0 | |
| ) { | |
| actions.delete(filePath) | |
| } | |
| } | |
| } | |
| const actionsArray = Array.from(actions.entries()) | |
| for (const [dep, actionNames] of actions) { | |
| for (const actionName of actionNames) { | |
| createdActions.add(entryName + '@' + dep + '@' + actionName) | |
| } | |
| } | |
| if (actionsArray.length === 0) { | |
| return Promise.resolve() | |
| } |
🤖 Prompt for AI Agents
In `@packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts` around
lines 964 - 995, The loop that filters actions (using getUsedActionsInEntry and
isInlineActionIdentifier) can leave Map entries with an empty array which later
creates empty action bundles; after computing filteredNames in the branch that
currently calls actions.set(filePath, filteredNames), if filteredNames.length
=== 0 delete the entry (actions.delete(filePath)) instead of setting an empty
array, and also move the creation of actionsArray =
Array.from(actions.entries()) to after this entire filtering loop so it reflects
the cleaned Map; reference getUsedActionsInEntry, isInlineActionIdentifier,
actions Map, createdActions, and entryName when making the 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] | ||
| } |
There was a problem hiding this comment.
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.
| 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() | ||
| }) |
There was a problem hiding this comment.
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.
| 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> ] | ||
| } | ||
| */ |
There was a problem hiding this comment.
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.
| /* | |
| { | |
| [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.
| 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 | ||
| } |
There was a problem hiding this comment.
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.
| process.env.TEST_EDGE = '1' | ||
|
|
||
| require('./basic.test') |
There was a problem hiding this comment.
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.
| 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.
| export default function Page() { | ||
| return ( | ||
| <div> | ||
| <h3>One</h3> |
There was a problem hiding this comment.
Heading text doesn't match the page location.
The heading displays "One" but this file is located in the two/ directory. This appears to be a copy-paste oversight from server/one/page.js. While this doesn't affect tree-shaking test functionality, updating it to "Two" would improve clarity and consistency.
Proposed fix
- <h3>One</h3>
+ <h3>Two</h3>📝 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.
| <h3>One</h3> | |
| <h3>Two</h3> |
🤖 Prompt for AI Agents
In
`@test/production/app-dir/actions-tree-shaking/shared-module-actions/app/server/two/page.js`
at line 6, Update the heading text in server/two/page.js: change the <h3>
content currently reading "One" to "Two" so the displayed heading matches the
file's location (server/two/page.js) and avoids the copy-paste mismatch from
server/one/page.js.
2 participants
PR_034
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.