Skip to content

macros: resolve macro imports with a resolver for Bun, so the "macro" export condition applies - #46802

Open
robobun wants to merge 3 commits into
robobun/076b1765/main-fields-shared-cachefrom
robobun/27655f39/macro-export-condition
Open

robobun wants to merge 3 commits into
robobun/076b1765/main-fields-shared-cachefrom
robobun/27655f39/macro-export-condition

Conversation

@robobun

@robobun robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #8779

Problem

  • A macro import never matches the documented "macro" export condition: it runs index.js, not index.macro.js, from 0.6.7 to 1.4.4.
  • MacroContext::call (src/js_parser_jsc/Macro.rs:146) resolved it with the importing build's resolver, which has the output target's conditions. So bun build ran a browser file as the macro.

Fix

  • A macro import resolves with a second resolver (Transpiler::macro_resolver): Bun's conditions plus macro, Bun's main fields, no externals.
  • Each MacroContext builds it at its first macro call. The docs list "macro" first.
  • A transpile job reads its own copy of the require.extensions list (Notes: a use-after-free).
  • Verified: test/bundler/transpiler/macro-test.test.ts (16 new tests, 14 fail on the base), two dev server tests. Self-reviewed: 77 concerns raised, 69 addressed (Notes).

Background

  • A macro runs in a Bun VM while the importing file is transpiled. MacroContext is one transpiler's macro state.
  • An export condition is a key of exports in package.json. The first key in the resolver's set wins.
  • Stacked on resolver: keep every main field of a package.json in the shared cache, select per resolver #46652, which makes the shared package.json cache safe for a second resolver.
  • Considered one more condition on the importing resolver: the output target still decides 31 of 117 changed cells.

Downsides

  • bun build behaviour change: a macro import ignores the browser condition and field and externals, and takes main before module. A browser-only package is not found.
  • A package with "macro" last, as in the old docs, still resolves index.js.
  • Cost with macros: 90 allocations (12,500 instructions) per bundler thread and per imported macro module under bun run. Without: +0 instructions per resolve, +6,144 text bytes.
Notes

For the maintainer: the scope. #8779 asks for the "macro" key. This PR does more: a macro import resolves for Bun, where the macro runs, at every target. The smaller change is one more condition on the importing resolver (see Alternatives). It fixes the key and leaves the output target in control of everything else. #40750 (merged) already makes the macro VM from the options of the build, so the imports inside a macro module resolve for Bun with the build's --conditions. #33709 proposed to delete the docs section in place of a fix and was closed unmerged.

Repro. node_modules/my-package/package.json is {"name":"my-package","exports":{"macro":"./index.macro.js","default":"./index.js"}}. Each file exports who() that returns its own name. index.ts is:

import { who } from "my-package" with { type: "macro" };
console.log(who());

bun build ./index.ts --target=bun and bun run index.ts print index.js on 0.6.7, 1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.4.3 and on the base. They print index.macro.js on this branch.

Why #8779 is not closed by this PR. The first matching key decides, as in Node. The docs listed "macro" last from 2023-05-31 until now, and the report says its map follows the docs. Such a map still resolves index.js. The package must move "macro" to the top. Older Bun versions ignore the key. The docs now name v1.4.5 as the first version (v1.4.4 was released on 2026-10-22 without this change; change the number if this lands later). They also show a full exports map with a "./macro" subpath for packages that support older versions. Both docs examples were run as written on this branch and on the base.

What a macro import resolves to now. It is the file that bun run --conditions=macro resolves with the same --conditions and --main-fields, plus development where the importing file has it. One test asserts that for every specifier of the matrix.

  1. A "macro" key that is the first match gives its file: exports, nested conditions, subpaths, and the "imports" of the importing package.
  2. The conditions browser and node of the output target no longer apply. The bun condition applies at every target.
  3. The "browser" field and the "browser" map do not apply.
  4. main comes before module, as in Bun's runtime. If the build passes --main-fields, that list applies, as on the base and as inside the macro module.
  5. The user's --conditions apply, also a condition that is the name of a target: with --conditions=browser a macro import takes a browser key again. On the base a page of an HTML import did not get the user's conditions for its macro imports. Now it does, like the macro itself when it runs.
  6. development applies when the importing resolver has it: a build that is not for production, the dev server. It does not apply in Bun's runtime or on a page of an HTML import in bun build, where plain imports do not have it either. react-server of a framework does not apply: with it, the SSR copy and the client copy of one "use client" file inlined different results of one macro call.
  7. --external, --packages=external and Bun.build({ external, packages }) do not apply. Before, the macro VM received the specifier unresolved and resolved it from the working directory. A relative macro import under --external '*' failed with Cannot find module './macro.ts'.
  8. A package that has a file only for the output target (for example exports: { browser }) now fails with Macro "x" not found. So does a "macro" entry that gives no file ("macro": null, a missing file): as in Node, the resolver does not go on to the next key. A "macro" entry that holds conditions of which none matches does fall through. What a user can do: import the file by its path, or name the condition for the whole build (--conditions=browser, conditions in Bun.build). A [macros] remap in bunfig.toml does not help: it is not read for an import that has the attribute.
  9. An "imports" entry that names a builtin ("#path": "node:path") now resolves to the builtin. Before it was not found.
  10. macro:node:path and macro:path now resolve to the builtin in every mode. On the base they did in a bundle, and failed under bun run with Macro "macro:node:path" not found. The builtin lookup now takes the specifier without the macro: prefix, as it does for the attribute spelling. So a package in node_modules that has the name of a builtin no longer runs for a macro: import. Do not look a bare builtin name up as a package: after a ?query cut, for a preload, for a macro: import #44957 has the same line.
  11. A macro file named bun.js loads as a file. MacroEntryPoint::generate took every file with the base name bun for the bun builtin (Macro 'who' not found in 'bun'). A package whose bun entry has that name is reachable from every target now. macros: compare the whole entry path to bun, so a macro file named bun.ts is not the builtin #46827 owns this line and has its tests: the hunk here is the same and drops out when macros: compare the whole entry path to bun, so a macro file named bun.ts is not the builtin #46827 lands.

No fallback to the importing resolver. A review asked for one for item 8. A macro import that falls back runs the file of the output target whenever Bun's conditions find nothing, and it hides a "macro" or "bun" entry that names a missing file. A note: on the error was written and removed: it has to tell a file for the output target from a dead "macro" entry, and the first text was wrong for the second. It is a follow-up.

The require.extensions list and transpile jobs. A module that import loads is transpiled on a pool thread, with a bytewise copy of vm.transpiler (TranspilerJob::run). The copy aliases the require.extensions list of the VM, and the JS thread frees that list on each require.extensions[x] = .... The macro resolver compares that list on each macro call, so the first commit of this PR read freed memory when a script assigned an extension during such a job. ASAN reports heap-use-after-free in MacroContext::resolve. A stress script (400 imports of macro modules while the list changes) crashed 7 times in 20 runs on the release build of the first commit, 0 times on the base and 0 times now. The base has the same bug on a narrower path: a macro import that resolves through a registered extension fails there with Macro "./gate" not found. Now RuntimeTranspilerStore::transpile clones the list on the JS thread and the job reads its own copy. The clone of an empty list does not allocate. TranspilerJob gets one field of 16 bytes (424 bytes now, 64 slots per VM).

Not changed.

  • An import inside a macro module resolves with bun, node and without macro. That needs a macro VM with a module registry of its own (macros: run every macro on one dedicated VM thread #40059). In bun run the macro module shares the registry of the program.
  • A macro file that is created after bun run or the dev server listed its directory is not found. Bun.build finds it. Handed off as a separate fix.
  • A tsconfig paths entry maps to a file, so it skips the exports of a package, as for a plain import.

How the resolver is made. macro_resolver_options (src/bundler/transpiler.rs) is one exhaustive literal of the resolver's options. Eight fields are set for the macro environment: target, conditions, main fields, externals, packages, builtins as externals, node polyfills, framework. Every other field is a copy of the importing resolver's value. The extension order is in that group: a macro import is an import statement and reads only the esm lists, which no target changes. A new resolver option does not compile until it is sorted into one of the two groups. Three values can change after the first macro call of a context that lasts longer than one module. MacroContext::resolve copies them on each call: the log, the auto-installer (PackageJSON::parse records the npm: aliases of a project through it), and the require.extensions list.

A MacroContext owns its macro resolver. A bundler worker has one context per build. The VM has one for the entry point and for require(). A transpile job and a Bun.Transpiler call make a context of their own, so each of them that calls a macro builds a resolver. An earlier commit kept the resolver in a thread-local slot between such contexts. That saved the 12,500 instructions and added a cache key, a Drop, and a resolver that outlived its VM. It was removed.

The Worker test and LeakSanitizer. A worker that called a macro does not free vm.macros and vm.macro_entry_points when it ends: VirtualMachine::destroy does not name them, and the VM storage is freed raw. LeakSanitizer reports 8,936 bytes in 7 allocations for one macro import (debug build, no suppression file). Nothing of the macro resolver is in that report. The leak is on main and #46228 owns it. So the bun run: a Worker test turns leak checking off for its child. When #46228 lands, that line can go.

Alternatives.

Related PRs and the order.

Measurements. Release builds (LTO) of the base 8ccf558da7 and of this branch (25a04e8a54), taken back to back. perf, strace, valgrind and bloaty are not in the container. Used: a perf_event_open wrapper for instructions:u, a ptrace syscall counter, gdb breakpoint counters on the mimalloc entry points, size, nm, gdb sizeof on the debug build.

Measure Base This PR
Bun.resolveSync, instructions per call (exports / main fields / main fields with browser) 13,070 / 18,137 / 19,988 13,070 / 18,137 / 19,988
bun -e 0, instructions (min of 20, three interleaved rounds) 4,447,522 to 4,451,085 4,449,614 to 4,451,639
bun -e 0, allocations 4,855 4,855
2,000 import() of modules with no macro, instructions per module 132,863 132,455
bun run of a file with no macro (min of 20, three rounds) 4,520,272 to 4,525,602 4,523,913 to 4,529,563
bun run of a file with one macro call (min of 20, three rounds) 4,935,662 to 4,940,617 4,945,001 to 4,952,181
One macro call (20,000 calls in one file) 18,378 18,440
bun run of 2,000 imported modules with one macro call each, per module 164,916 177,450
Bun.Transpiler#transformSync of a file with one macro call, per call 82,784 94,989
bun build of 2,000 modules with one macro call each 218,370,716 218,904,556
bun build with one macro import: openat / read / getdents64 / close 30 / 24 / 21 / 29 30 / 24 / 21 / 29
Release text (size) 107,558,832 107,564,976 (+6,144)
MacroContext 152 bytes 160 bytes
TranspilerJob 408 bytes (424 less the new field) 424 bytes
  • One build of the macro resolver: 90 allocations and 2,805 bytes under bun run, 98 allocations in a bundle that is not for production, 88 with --production.
  • How often it is built: 1 time for 20,000 calls in one file. 12 times for 2,000 modules under bun build (12 threads). 2,000 times for 2,000 imported modules under bun run.
  • Test matrix: in the tests that compare cells, 117 of 331 cells change against the base.

Self-review. Two passes. The first was on the first cut: 46 concerns, 40 addressed. The second was on the follow-up commit: 31 concerns, 29 addressed. The second pass found the require.extensions use-after-free, the react-server split, three wrong docs statements and the wider overlap with #46548. It also asked to drop the thread-local slot and the note.

Not done, with the reason:

Suites run with the debug build. test/bundler/transpiler/macro-test.test.ts and the two new tests of test/bake/dev/bundle.test.ts (both also with BUN_DESTRUCT_VM_ON_EXIT=1, detect_leaks=1 and test/leaksan.supp, as the x64-asan lane runs them), the whole test/bake/dev/bundle.test.ts, transpiler.test.js, runtime-transpiler.test.ts, scope-mismatch-panic.test.ts, test/bundler/resolver/cache-runtime.test.ts, bundler_barrel, the macro tests of bundler_edgecase and bun-build-api, test/js/bun/resolve/ (build-error, resolve, import-custom-condition), test/js/bun/plugin/plugins.test.ts, test/cli/test/isolation.test.ts, test/cli/run/transpiler-cache.test.ts, test/js/node/module/require-extensions.test.ts, test/js/node/disallow-code-generation-from-strings.test.ts, test/regression/issue/{03830,22656,26360,39900}.test.ts, test/internal/source-lints/. With the release build: test/cli/run/require-cache.test.ts (one test times out at 30 s on the base too).

… export condition applies

A macro import was resolved by the resolver of the importing build. That
resolver has the conditions of the output target, so the documented
"macro" export condition never matched. `bun build` for the browser also
ran the "browser" build of a package as the macro, and `--external` or
`--packages=external` changed which copy of the package the macro ran.

The macro context now resolves with a second resolver, made from the
importing one on the first macro call. What a target decides is set as
Bun's runtime sets it: the conditions "macro", "bun" and "node" plus the
user's conditions, Bun's main fields and extensions, no "browser" field,
and no externals. Every other option stays as the importing resolver has
it. A thread keeps the resolver for the next module of the same build.

A builtin module behind the `macro:` prefix is found before the resolver
runs, and a macro file named `bun.js` is no longer taken for the `bun`
builtin.

The docs example lists "macro" first, because the first matching key
decides the file.
@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:29 PM PT - Oct 24th, 2026

❌ @robobun, your commit c1be659 has 2 failures in Build #132418 (All Failures):

  • test/js/bun/http/serve-body-leak.test.ts - code 1 on 🍎 27 aarch64
  • ❌ Baseline verification failed on Windows x64 — 1 check(s)
  • The baseline build crashed or failed tests under Nehalem (SSE4.2, no AVX/AVX2/AVX512) emulation (not an unsupported-instruction fault). See the step log for the crash output.

    • SIMD baseline tests

🧪   To try this PR locally:

bunx bun-pr 46802

That installs a local version of the PR into your bun-46802 executable, so you can run:

bun-46802 --bun

@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced with a package whose package.json has "exports": { "macro": "./index.macro.js", "default": "./index.js" }. An import of it with { type: "macro" } runs index.js in bun build and in bun run, on 0.6.7, 1.0.0, 1.1.0, 1.2.0, 1.3.0 and 1.4.3. On this branch both run index.macro.js.

PR: #46802

CI on c1be659 (build 132418): 180 jobs passed, with all x64-asan lanes. Two jobs failed, and neither runs code of this diff:

  • windows x64 - verify-baseline: the JPEG test of test/js/bun/jsc-stress/fixtures/simd-baseline.test.ts timed out at 5000 ms. The same job fails the same way on main builds 131922 and 131661.
  • darwin 27 aarch64 - test-bun: test/js/bun/http/serve-body-leak.test.ts timed out at 40 s after an ECONNRESET.

This PR is stacked on #46652. That PR must land first.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread src/bundler/transpiler.rs Outdated
Comment thread src/bundler/transpiler.rs Outdated
Comment thread src/js_parser_jsc/Macro.rs Outdated
Comment thread test/bundler/transpiler/macro-test.test.ts
@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

The MacroEntryPoint::generate line of this PR (note 10) is also in #46827, as a change of its own with tests for that bug. src/bundler/entry_points.rs is byte-identical in the two PRs, so they merge in either order with no conflict. If #46827 lands first, this PR can drop that hunk.

…s own require.extensions list

- The macro resolver has the "development" condition when the importing
  resolver has it. The user's `--conditions` and `--main-fields` apply as
  before. The extension order is a copy of the importing resolver's: a macro
  import reads only the `esm` lists, which no target changes. `from_api` is
  as on the base again.
- A transpile job on another thread read `require.extensions` through a
  bytewise copy of `vm.transpiler`. The JS thread frees that list when a
  script assigns an extension, and the macro resolver reads the list on each
  macro call. The job now has its own copy of the list.
- Tests: the page of an HTML import, `--conditions=browser`, `--main-fields`,
  a "macro" entry that gives no file, the dev server, `require.extensions`
  replaced during an import, and a Worker in a child of its own. That child
  runs with leak checking off: a worker that called a macro does not free its
  tables of macros when it ends. That leak is on main (#46228).
- Docs: the first version is v1.4.5, the subpath route has a full `exports`
  example, and the conditions that also apply are listed.
@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

25a04e8 answers the four review comments and two more findings.

From the comments:

  • development stays for a macro import. react-server does not.
  • The thread-local slot for the macro resolver is removed. Each macro context owns its resolver.
  • A package that only has a browser file is still not found: no fallback. The thread has the reasons and what a user can do.
  • The two sequential tests say why they are sequential.

Found after the first push:

  • A use-after-free in the first commit. A module that import loads is transpiled on another thread, with a bytewise copy of vm.transpiler. The macro resolver read the require.extensions list through that copy on each macro call, and the JS thread frees the list when a script assigns an extension. The transpile job has its own copy of the list now. The new test require.extensions changes while another thread resolves a macro import also fails on the base (Macro "./gate" not found): the base has the same bug for a macro import through a registered extension.
  • The x64-asan failure in build 132209. A worker that called a macro does not free vm.macros and vm.macro_entry_points when it ends. That leak is on main and Worker teardown: run the VM's field destructors before its storage is freed #46228 owns it, so the Worker test runs its child with leak checking off.

The PR body has the new measurements and the merge order against #46652, #46750 and #46548.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

The second commit addresses the development-condition and thread-local idle-slot points from my earlier review. Beyond the inline nit, I also checked the ptr::read/ptr::write handoff of extra_cjs_extensions in TranspilerJob::run (src/jsc/RuntimeTranspilerStore.rs:724-731): the bytewise transpiler copy is ManuallyDrop and never dropped, the macro resolver only clone_froms that list, and the only assignments to it (src/jsc/NodeModuleModule.rs:166 and :192) go through vm.transpiler on the JS thread, so the job remains the single owner and the list is freed once with the job on every path, including when run() is never reached.

Extended reasoning...

The change adds a lazily built second resolver for macro imports (Target::BunMacro conditions, Bun main fields, no externals) on MacroContext, a derived BundleOptions literal in transpiler.rs, a Resolver::for_target wrapper, a per-job snapshot of require.extensions in RuntimeTranspilerStore, and a tighter "bun" builtin check in entry_points.rs, with docs and ~680 lines of tests. It touches unsafe raw-pointer and lifetime-erased code in the transpiler/resolver path but no auth, crypto, or injection surface. The new commit removed the thread-local resolver slot and now carries the importing resolver's development condition, resolving two of three earlier findings; the remaining one (browser-only packages no longer resolve as macros) is a documented behavior change the author chose deliberately, so a human maintainer should weigh that trade-off.

Comment thread test/bundler/transpiler/macro-test.test.ts
… the main thread

The first macro of the fixture waits for the main thread. When the import is
transpiled on the main thread (`NODE_COMPILE_CACHE`, or the async transpiler
is off), that wait cannot end. The macro now returns a message in that case,
the main script stops waiting when the import settles, and the child runs
without the two settings.
@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

c1be659 takes the nit about the require.extensions race fixture. It is a test-only commit.

  • When the import is transpiled on the main thread, the first macro returns a message and does not wait. The main script of the fixture stops its wait when the import settles. So the child ends with a readable result, not with a timeout.
  • The child runs without BUN_FEATURE_FLAG_DISABLE_ASYNC_TRANSPILER and NODE_COMPILE_CACHE, because the test needs the import on another thread.

On the open point of the first review (a package that only has a browser file is not found as a macro): that is the behaviour change a maintainer has to accept or reject. The PR body has it under Downsides and in the scope note at the top of Notes.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread src/jsc/RuntimeTranspilerStore.rs
@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

The third review has one note, marked pre-existing and not blocking: the bytewise copy of vm.transpiler in a transpile job is a data race under the Rust memory model. The thread has the answer. The race is on main and covers more words than the require.extensions list. This PR makes the job stop using the one word whose old value could be freed. There is no new commit for this note.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants