packages/nuxt/src/core/plugins/tree-shake.ts (2)

86-86: Wrap the replacement in parentheses to preserve operator precedence in all contexts.

Helps when the transformed call appears inside larger expressions (arguments, logical chains, ternaries).

Apply:

-            s.overwrite(node.start, node.end, ` false && /*@__PURE__*/ ${functionName}${code.slice(node.callee.end, node.end)}`)
+            s.overwrite(node.start, node.end, `(false && /*@__PURE__*/ ${functionName}${code.slice(node.callee.end, node.end)})`)

---

85-85: Follow-through on the TODO: consult the real auto-imports registry.

Wire an optional predicate into plugin options so we don’t rely solely on name heuristics.

Example API shape:

interface TreeShakeComposablesPluginOptions {
  sourcemap?: boolean
  composables: Record<ImportPath, string[]>
  isAutoImported?: (name: string) => boolean
}

And in the guard (near Line 81):

if (!scopeTrackerNode && !(allComposableNames.has(functionName) || options.isAutoImported?.(functionName))) {
  return
}

packages/nuxt/test/tree-shake.test.ts (1)

71-94: Great regression test for nested composables — confirms new skip behaviour.

Consider adding an async-nested variant to cover the reported pattern with async arrows.

You could duplicate this case with:

onMounted(async () => {
  onMounted(() => {})
})

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/nuxt/src/core/plugins/tree-shake.ts (1)

47-52: Guard against non-statement contexts (potential syntax break).

If a tree-shakable composable were ever used in a non-statement position (e.g., as an argument, in a sequence, or assigned), a raw s.remove(node.start, node.end) could yield invalid JS (e.g., foo(, 1) or const x = ;). Today you only include lifecycle composables, so this is unlikely, but worth hardening.

Suggested approach (optional):

• Operate at ExpressionStatement level: only remove when the parent is an ExpressionStatement whose expression is the target CallExpression.
• Otherwise, neutralise with a benign expression (e.g., void 0) to keep syntax valid.

I can send a follow-up PR to reframe the visitor to match ExpressionStatement and handle the fallback path if you’d like.

Would you confirm that options.composables is guaranteed to list only lifecycle-style, statement-only calls in Nuxt 4.x?

Also applies to: 85-87

packages/nuxt/test/tree-shake.test.ts (1)

5-10: Add a few safety tests for non-statement contexts (optional).

To guard against future additions to composables, consider adding cases like:

• foo(onMounted(() => {}))
• const x = onMounted(() => {})
• [onMounted(() => {}), 1]

These will catch accidental invalid code generation if the plugin ever targets non-statement usages.

Happy to draft these tests if you want.

Also applies to: 66-81

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.

Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.

packages/nuxt/src/core/plugins/tree-shake.ts (2)

86-90: Remove the entire statement to avoid stray semicolons/artefacts

When the parent is an ExpressionStatement, removing only the CallExpression can leave a dangling “;” or whitespace. Remove the whole statement.

-            if (parent?.type === 'ExpressionStatement') {
-              s.remove(node.start, node.end)
+            if (parent?.type === 'ExpressionStatement') {
+              s.remove(parent.start, parent.end)
             } else {
               s.overwrite(node.start, node.end, 'void 0')
             }

---

91-91: Early return after mutation for clarity

After mutation + this.skip(), return to make intent explicit and avoid any accidental fallthrough in future edits.

-            this.skip()
+            this.skip()
+            return

packages/nuxt/test/tree-shake.test.ts (4)

66-79: Good coverage for nested composables

This reproduces the original failure mode and verifies no double-edit; looks solid. Consider also a variant where the inner call uses auto-imports from '#imports' (not via alias) to widen coverage.

---

140-155: Assignment neutralisation verified

a = void 0 maintains runtime shape. Consider adding a semicolon-style variant to ensure no stray “;” when statements end with semicolons.

---

157-173: Logical/conditional contexts covered well

These cases are easy to get wrong; snapshots look right. Please add:

• return onMounted() → return void 0
• await onMounted() → await void 0

---

11-18: Add tests for optional chaining and parenthesised statements

Two extra edge cases frequently appear in user code:

• onMounted?.(() => {}) in expression/argument positions.
• (onMounted()) as a standalone parenthesised expression statement.

These ensure the callee-detection and ExpressionStatement removal behave as intended.

Proposed additions (append near related blocks):

+  it('should handle optional chaining calls', () => {
+    const code = `
+      import { onMounted } from '#imports'
+      onMounted?.()
+      test(onMounted?.())
+    `
+    const { code: result } = transformPlugin.transform.handler(code, 'test.js')
+    expect(clean(result)).toMatchInlineSnapshot(`
+      "import { onMounted } from '#imports'
+      test(void 0)"
+    `)
+  })
+
+  it('should handle parenthesised expression statements', () => {
+    const code = `
+      import { onMounted } from 'vue'
+      (onMounted())
+    `
+    const { code: result } = transformPlugin.transform.handler(code, 'test.js')
+    expect(clean(result)).toMatchInlineSnapshot(`
+      "import { onMounted } from 'vue'"
+    `)
+  })
+
+  it('should handle return/await contexts', () => {
+    const code = `
+      import { onMounted } from 'vue'
+      async function f () {
+        return onMounted()
+      }
+      async function g () {
+        await onMounted()
+      }
+    `
+    const { code: result } = transformPlugin.transform.handler(code, 'test.js')
+    expect(clean(result)).toMatchInlineSnapshot(`
+      "import { onMounted } from 'vue'
+      async function f() {
+        return void 0
+      }
+      async function g() {
+        await void 0
+      }"
+    `)
+  })

Also applies to: 128-137, 140-155, 157-173, 175-185

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro