Skip to content

macros: compare the whole entry path to bun, so a macro file named bun.ts is not the builtin - #46827

Open
robobun wants to merge 1 commit into
mainfrom
robobun/492de4e9/macro-file-named-bun
Open

robobun wants to merge 1 commit into
mainfrom
robobun/492de4e9/macro-file-named-bun

Conversation

@robobun

@robobun robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • A macro imported from a file named bun (bun.ts, an exports target ./bun.js) is never loaded. An export with the name of a Bun function runs that function instead. A user write(path, text) macro calls Bun.write at build time, exit 0.
  • Other export names fail: error: Macro 'who' not found in 'bun', then error: "MacroLoadError" error in macro.
  • MacroEntryPoint::generate (src/bundler/entry_points.rs:202) tests import_path.base == b"bun". base has no directory or extension, so /abs/dir/bun.ts matches.

Fix

  • The test is now import_path.dir.is_empty() && import_path.filename == b"bun". It is true only for the exact path bun, which is how the builtin arrives.
  • Verified: test/bundler/transpiler/macro-test.test.ts (2 new tests, both fail on main). Also the test/regression/issue/ macro tests.
  • Self-reviewed: 33 concerns raised, 28 addressed, 5 rejected (Notes).

Background

  • A macro import runs through a generated entry module, one per export. It imports the macro's path and calls Bun.registerMacro(id, fn). For the builtin it reads globalThis.Bun[x].
  • PathName::init splits a path into dir, base, ext and filename. With an empty dir, filename is the whole path.
  • Considered a &[u8] path parameter written one time: it changes how pkg//macro loads and overlaps Real lifetimes for Source, import records and resolver results #44493. Considered no builtin arm: inherited Bun names stop resolving.

Downsides

  • With --external '*', a relative macro import resolves from the working directory (resolver: do not apply user externals to macro imports #40631). A bun-named file failed there and now loads like any other name, possibly the wrong copy.
  • bun/sub with --external bun still gets the builtin.
  • generate grows 18 bytes. First load of a macro export: 5567 to 5568 instructions. Per macro call: 0.
Notes

Repro (no network, any folder). On main the first command prints &lt;a&gt; and the second prints the error. With this PR they print mine: <a> and from ./bun.ts.

printf 'export function escapeHTML(s) {\n  return "mine: " + s;\n}\nexport function who() {\n  return "from ./bun.ts";\n}\n' > bun.ts
printf 'import { escapeHTML } from "./bun.ts" with { type: "macro" };\nconsole.log(escapeHTML("<a>"));\n' > silent.ts
printf 'import { who } from "./bun.ts" with { type: "macro" };\nconsole.log(who());\n' > loud.ts
bun run silent.ts   # main: &lt;a&gt;   (Bun.escapeHTML ran, exit 0)
bun run loud.ts     # main: error: Macro 'who' not found in 'bun'

Origin. Found by a read of the code, in the review of #42706 (#42706 (comment)). No user reported it. The base test came with #7113. Until this lands, a rename of the file or a one-line export * from "./bun.ts" file avoids the error case. The silent case gives no sign that a change is necessary.

What main does, by export name. Bun has no such property: Macro '<name>' not found in 'bun'. A property that is not a function (version): error: Macro must be a function. A function (escapeHTML, write): that function runs with the macro's arguments. With write("./notes.txt", "x"), bun build replaced an existing 39-byte notes.txt with 1 byte and printed console.log(1);.

Why the line is exact. PathName::init (src/paths/lib.rs) sets filename to the text after dir, or to the whole input when dir is empty. So the test is true only when the entry path is the 3 bytes bun. That string comes from the alias lookup in MacroContext::call (import ... from "bun") and from the resolver for an imports entry that maps to bun.

Before and after, release builds of 179664e and this PR, bun run and bun build, 61 specifier and mode pairs.

  • 0 macro ids changed. 41 pairs are equal.
  • The 20 pairs that changed all moved from the globalThis.Bun module to the await import() module: ./bun.ts, ./lib/bun.js, ./noext/bun (no extension), a package with exports target ./bun.js, and with --packages=external also @scope/bun and bun/.
  • Equal before and after: bun, #bun through an imports map, node:fs, ./other.ts, pkg//macro, a data: URL.

The tests pin the exact compare. A build with dir.is_empty() && ext.is_empty() && base == b"bun" passes the bun run test and fails the --packages=external test, where the path bun/ takes the builtin module.

Measurements, release bun-profile, 179664e against this PR.

  • First macro load, from the entry of load_macro_entry_point to the JSLock::lock call (gdb stepi, 3 equal runs each): file macro 5567 to 5568 instructions, bun builtin 2227 to 2229.
  • MacroEntryPoint::generate 1171 to 1189 bytes (nm -S). .text 82164617 to 82164873 bytes (size -A). The stripped bun file has the same size.
  • JS cells for one first macro load (heapStats(), BUN_JSC_useGC=0): bun builtin 385 to 385, file macro 433 to 433. ./bun.ts is 433 with this PR.
  • 51 calls of one export generate the entry module 1 time, before and after (gdb breakpoint counts).

Order against the open PRs on this function.

Not changed.

  • Externals reach the macro as written, and a macro that fails to load under bun build aborts a debug build (ASSERTION FAILED: container.vm().currentThreadIsHoldingAPILock()): resolver: do not apply user externals to macro imports #40631.
  • MacroEntryPoint::generate_id hashes the path and the export name with nothing between them. util.js with export xValue and util.jsx with export Value are one macro: ["util.js:xValue","util.js:xValue"]. Same on main. No open PR.
  • VirtualMachine::_resolve (src/jsc/VirtualMachine.rs:5260) resolves a specifier whose last part is bun:wrap to the runtime module. Same class, different subsystem. No open PR.
  • A data: URL macro whose last / comes directly after a / loses one slash, because the entry module joins dir and filename.

Self-review, the 5 rejected concerns.

  • Remove the globalThis.Bun module (2 concerns): it changes the builtin path. import { toString } from "bun" resolves today and a module namespace has no such export.
  • Merge this with Escape the macro path and export name in the generated macro entry module #42706: the two fixes are independent, and this one is 1 line.
  • Add a separator to generate_id here: a different defect, and it changes every macro id.
  • Add a test for the missing-export message: this PR changes no message text, and the new rows prove which module loads.

The first design for this fix changed generate to take the path as &[u8] and write it one time. The self-review cut it to this line: same result for every path above, no change for pkg//macro, no overlap with #44493, and the same bytes as #46802.

Suites run with the debug build. test/bundler/transpiler/macro-test.test.ts (36 pass, on main 34 pass and 2 fail), test/regression/issue/03830.test.ts, 22656, 26360, 39900, test/bundler/transpiler/scope-mismatch-panic.test.ts (44 pass), test/js/node/disallow-code-generation-from-strings.test.ts (34 pass, 4 skip, with --timeout 600000).

…bun.ts is not the builtin

MacroEntryPoint::generate chose the `globalThis.Bun` entry module when
the file name of the macro's path, without directory and extension, was
`bun`. A macro file named bun.ts, bun.js or bun was never imported. An
export with the name of a Bun function ran that function in its place.

The builtin is now chosen only when the entry path is exactly `bun`.
@robobun

robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:22 PM PT - Oct 24th, 2026

✅ @robobun, your commit 21f5dcb27d4eeb3446065270107a8cfcf2cf964e passed in Build #132317! 🎉


🧪   To try this PR locally:

bunx bun-pr 46827

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

bun-46827 --bun

@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Status: open, waiting for CI.

How it was reproduced. On main (179664e) and on 1.4.3-canary, a macro imported from ./bun.ts is never loaded. With the three files from the Notes in the description:

  • bun run silent.ts prints &lt;a&gt;. Bun.escapeHTML runs in place of the escapeHTML that the file exports, and the exit code is 0.
  • bun run loud.ts prints error: Macro 'who' not found in 'bun'.

With this PR the two commands print mine: <a> and from ./bun.ts.

PR: #46827

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and 21f5dcb.

Files selected for processing: 2
  • src/bundler/entry_points.rs
  • test/bundler/transpiler/macro-test.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The macro entry point now selects the built-in path only for the exact specifier bun. A regression test checks direct imports, aliases, and similar specifiers with bun run and external-package builds.

Changes

Macro specifier matching

Layer / File(s) Summary
Built-in macro selection and regression coverage
src/bundler/entry_points.rs, test/bundler/transpiler/macro-test.test.ts
The built-in path requires an empty directory and filename bun. Tests check direct imports, an imports-map alias, and similar specifiers under bun run and external-package build output.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to 21f5d

The change is ready to merge after normal checks; no actionable risk remains identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: exact builtin bun detection prevents files such as bun.ts from being treated as the builtin.
Description check Passed The description explains the problem, fix, scope, tradeoffs, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@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.

LGTM — a one-line tightening of the builtin check in MacroEntryPoint::generate, with a fixture test covering the variants.

What was reviewed:

  • Traced PathName::init in src/paths/lib.rs: filename is the whole input when dir is empty, so the new condition matches only the 3-byte specifier bun; bun/, /bun, and Windows C:\bun all fall through to the module path as intended.
  • Confirmed the builtin arrives as exactly bun via the BUN_EXTRA_ALIAS_KVS alias (src/resolve_builtins/HardcodedModule.rs) through MacroContext::call and load_macro_entry_point; grepped for sibling base == b"bun" sites — this is the only one.
  • Checked the stderr: "" assertion against debug builds: the [macro] call line is emitted via prettyln! to stdout, which the lastLine extraction already accounts for.
Extended reasoning...

The change replaces import_path.base == b"bun" with import_path.dir.is_empty() && import_path.filename == b"bun" in src/bundler/entry_points.rs and adds a describe.concurrent block in test/bundler/transpiler/macro-test.test.ts exercising bun run and bun build --packages=external against a tempDir fixture with bun.ts, scripts/bun, an exports target ./bun.js, @ scope/bun, bun/, the real builtin, and an imports-map alias. It touches no security-sensitive surface; the only behavioral shift is that macro specifiers whose basename is bun now load as modules instead of silently aliasing globalThis.Bun, which is the bug being fixed. The fix is a single line whose semantics I verified directly from PathName::init and the hardcoded alias table, no CODEOWNER covers the changed files, and the test follows the harness conventions (tempDir, bunExe, bunEnv, concurrent pipe draining, combined object assertion), which decided approve.

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.

1 participant