Skip to content

fix: chained call - #157

Open
syd03098 wants to merge 1 commit into
unplugin:mainfrom
syd03098:chained-call
Open

fix: chained call#157
syd03098 wants to merge 1 commit into
unplugin:mainfrom
syd03098:chained-call

Conversation

@syd03098

Copy link
Copy Markdown

Description

  • To be honest I used an AI to help me understand the core code.
  • The issue was that when compiler encountered a CallExpression, it was immediately skipped before checking whether it's a macro function or not.
  • So I made a small change to skip only after it's confirmed to be a macro function call.

Linked Issues

  • 🤔

@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@ariesclark

Copy link
Copy Markdown

Confirming this fixes a real bug. The fixture doesn't cover enough of it, though.

Here's the case I hit, on unplugin-macros 0.20.2 with Vite 8.1.4 and Node 24.18.0:

// macros.js
export const answer = () => 42
// main.js
import { answer } from './macros.js' with { type: 'macro' }

export default [answer()].filter(Boolean)
// vite.config.mjs
import macros from 'unplugin-macros/vite'

export default {
  plugins: [macros()],
  build: { minify: false, lib: { entry: 'main.js', formats: ['es'], fileName: 'out' } },
}

The macro is silently ignored. vite build succeeds and emits:

var main_default = [answer()].filter(Boolean);

The import is stripped but the call is left behind, so importing that module throws ReferenceError: answer is not defined. With this PR applied I get [42].filter(Boolean), which is correct.

The reason chained-call.js wouldn't have caught this: it puts the macro in an argument of the chained call's object (defineRoute(() => getRandom()).meta(...)), whereas here the macro sits in the object expression itself, the array in [answer()].filter(...). Both are subtrees of node.callee, so skip.add(node.callee) discards either one, but neither fixture implies the other. Anything of the form [...].filter(), [...].map(), or [...].join() wrapped around a macro call hits this. I'd suggest adding a tests/fixtures/chained-call-object.js:

import { getRandom } from './macros/rand' with { type: 'macro' }

const values = [getRandom()].filter(Boolean)

const joined = [getRandom(), getRandom()].map((value) => value).join(',')

I checked that it fails on the original ordering and passes with your change.

One other thing worth raising. The silence is what made this expensive to track down. recordImports() writes the import removal into the MagicString before collectMacros() runs, so when the walk comes back empty the early return at if (!macros.length && !macroExports.length) has already dropped the import while leaving every call site intact. Nothing warns at build time.

@ariesclark

ariesclark commented Jul 20, 2026

Copy link
Copy Markdown

If anyone's blocked on this, I published this branch as @ariesclark/unplugin-macros@0.20.3, drop-in until it lands upstream:

pnpm add -D "unplugin-macros@npm:@ariesclark/unplugin-macros@0.20.3"

ariesclark added a commit to flirtual/flirtual that referenced this pull request Jul 20, 2026
vite-plugin-babel parsed and regenerated every file it matched, including
node_modules, which Babel reported as deoptimised for react-dom and
@formatjs/intl-datetimeformat. unplugin-macros evaluates macros without a
Babel pass, and `ms` can be imported directly instead of through the
ms.macro wrapper, so @types/ms now checks the duration strings.

Import attributes require a module target above ES2022, hence the tsconfig
bump; ESNext is also the correct pairing for moduleResolution: bundler.

babel-plugin-dev-expression is dropped with nothing to replace it: it only
stripped invariant() messages in production, and the codebase has no
__DEV__ or warning() calls.

unplugin-macros is pinned to a fork that carries unplugin/unplugin-macros#157,
which fixes macros being silently dropped inside a chained call's object
(`[ms("30s")].filter(Boolean)`). Revert to the upstream package once that
lands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants