bundler: resolve require('bindings')(name) to the package's .node addon at build time

[robobun]

Problem

• Packages such as sqlite3, better-sqlite3, serialport or zeromq load their native addon with require("bindings")("name"). The bindings package locates the .node file at runtime by walking up from the caller's __filename to the package root and probing build/Release/name.node and a few sibling directories. Once the caller is bundled that walk starts at the bundle, and in a bun build --compile executable it fails immediately (bun --compile error. error: Could not find module root given file. #10964, Bun build does not work with the sqlite3 library #14301, better-sqlite3 .node exe is not bundled #8895):

  error: Could not find module root given file: "/$bunfs/root/app". Do you have a `package.json` file?
        at getRoot (/$bunfs/root/app:154:15)
        at bindings (/$bunfs/root/app:84:41)
  

• Even when the walk could succeed, the .node file is never part of the build, because the bundler only ever sees require("bindings"), not the addon.

Fix

• Parser (src/js_parser/p.rs, maybe_rewrite_bindings_require): when Features::rewrite_bindings_require is set and a call has the shape require("bindings")("name") or require("bindings")({ bindings: "name" }), the import record that require("bindings") produced is retargeted at name.node (the extension is added like bindings does) and tagged NativeBindings, and the call is replaced by a require of that record. Any other argument (further options, a computed name) changes what bindings would look for and is left alone.
• The existing record is retargeted rather than a new one added next to it. A record merely marked unused is not dead: when the module containing it is reached through require() or import(), the barrel optimization un-defers every unused record of that module and resolves it, so the bindings package ended up bundled after all (the earlier revision of this PR, and bundler: resolve require('bindings')(name) to the built .node file #35642 and bundler: resolve require("bindings")(name) to a static .node import #35688, had this gap; their tests used a module.exports = require(...) module that the bundler collapses before linking, which hid it).
• ParseTask enables the feature only for server-side targets, outside the dev server, and when bindings is not external (Resolver::is_package_external, which covers --external bindings and --packages external). Browser builds, dev-server builds and builds that externalize bindings are byte-for-byte what they were; the last one keeps the runtime lookup the user asked for.
• Resolution (src/bundler/bundle_v2.rs, resolve_import_records): a NativeBindings record is looked up with Resolver::resolve_bindings_addon before anything else. The module root is found the way bindings' getRoot() finds it (nearest directory with a package.json, named or not, or a node_modules directory) and the build directories are probed in bindings 1.5.0's order, minus the two entries whose names encode the build host's Node version. On success the record now holds the addon's absolute path and continues through normal resolution, so plugins, --outdir and --compile treat it exactly like a hand-written require("/abs/addon.node") and the existing napi loader copies or embeds the file. On failure the record goes through the same path as any unresolvable import: a build error naming the addon; inside a try block, a require that throws at runtime, so packages with a JS fallback behave as they would with bindings; under ignoreModuleResolutionErrors, the same stub every other unresolvable require gets.
• Why this is correct: the rewrite only replaces the call when the argument pins the addon name statically, which is exactly the input bindings would use at runtime, and it reproduces bindings' own search; everything bindings would do differently (other options, a dynamic name) is left to run at runtime. The missing-addon behaviour mirrors what bindings does at runtime (throw), moved to build time where it can be.
• Verified with test/bundler/bundler_loader.test.ts (describe("require('bindings')"), 16 cases: both targets, both argument forms, other options and computed names left alone, a build/Debug addon with bindings not installed at all, both module-root rules, an addon named zlib, the missing-addon error, the try/catch fallback at runtime, external: ["bindings"], packages: "external", the browser target and the dev-server format; 10 fail on the released binary, the 6 left-alone cases pass on both and pin the gating) and test/napi/napi.test.ts, which compiles an app whose dependency loads the real napitests.node through require('bindings') and runs the executable from an unrelated directory (esm and cjs). bundler_loader, bundler_edgecase, bundler_cjs, bundler_barrel, bundler_npm and esbuild/packagejson pass.

Consolidation

#35642 and #35688 were alternative fixes for the same issue. Folded in from #35642: appending .node in the parser, not touching browser builds, the build/Debug case without bindings installed, and the docs note; from #35688: the { bindings: "name" } form and the negative test for other options. Not adopted: #35642 left a missing addon as a runtime require("name.node") and #35688 turned it into an empty stub; both hid the problem until runtime, and #35688's stub also defeated try/catch fallbacks. The bindings package itself is not vendored as a fixture (a throwing stub proves the same thing).

Background

• Import record: the bundler's per-module list of things a module imports or requires; the parser creates one per require("x") and the bundler resolves them after parsing. Expressions refer to records by index.
• HANDLES_IMPORT_ERRORS / WAS_UNRESOLVED: record flags meaning "this require is inside a try" and "resolution failed"; the printer turns a record with both into a throwing expression and one with only the latter into an empty stub, which is how unresolvable requires behave everywhere else in the bundler.
• Barrel optimization: re-export-only modules have their unneeded import records marked unused and resolved later on demand; a require() or import() of a module requests all of it, which un-defers every unused record in it.
• Napi loader: the bundler's handling of .node files, which copies the file next to the bundle or embeds it into a compiled executable and rewrites the require to point at it.

Fixes #10964
Fixes #14301
Fixes #8895

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_compile.test.ts test/napi/napi.test.ts

[coderabbitai]

Walkthrough

The compiler now recognizes literal require("bindings")("<name>") calls, resolves matching native addons, embeds .node assets, and reports or suppresses resolution failures according to context. Tests cover bundling, externalization, fallback behavior, non-literal calls, and N-API execution.

Changes

Native bindings support

Layer / File(s)	Summary
Parse bindings calls src/ast/import_record.rs, src/js_parser/visit/visit_expr.rs	Adds the NativeBindings import tag and transforms literal require("bindings") calls into tagged import records.
Resolve native addons src/bundler/bundle_v2.rs, src/resolver/resolver.rs	Searches standard package and build paths for .node files, bypasses generic builtin and plugin resolution, and handles missing or external native addons.
Validate bindings support test/bundler/bundler_compile.test.ts, test/napi/napi.test.ts	Tests embedded addons, missing addons, builtin-name handling, externalization, try/catch fallback, non-literal calls, and execution from another directory.

Suggested reviewers: jarred-sumner, dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: build-time resolution of require('bindings')(name) to a native addon.
Description check	✅ Passed	The description explains the problem, fix, behavior, scope, verification, and related issues, although it does not use the template headings exactly.

---

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

[robobun]

^{Updated 1:05 AM PT - Aug 13th, 2026}

❌ @robobun, your commit 5fee0de has 1 failures in Build #94182 (All Failures):

• test/js/web/timers/setInterval.test.js - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 36713

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

bun-36713 --bun

[github-actions]

Found 3 issues this PR may fix:

1. bun --compile error. error: Could not find module root given file. #10964 - The bindings package fails at runtime inside a --compile binary with "Could not find module root given file"; build-time resolution eliminates this lookup entirely
2. better-sqlite3 .node exe is not bundled #8895 - better-sqlite3 uses require('bindings') to find its .node addon, which the bundler currently cannot resolve
3. Auto-install doesn't work for sqlite3 library #9292 - sqlite3 uses require('bindings')('node_sqlite3') which fails to locate the .node file; build-time resolution fixes this

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #10964
Fixes #8895
Fixes #9292

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. bundler: resolve require('bindings')(name) to the built .node file #35642 - Also resolves require('bindings')(name) to the built .node file at bundle time using the same directory-probing approach
2. bundler: resolve require("bindings")(name) to a static .node import #35688 - Also rewrites require("bindings")(name) to a static .node import at build time via import record retagging

🤖 Generated with Claude Code

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/bundler/bundle_v2.rs:6062-6091 — The error's suggested remedy "mark the import external" is unactionable: neither --external bindings nor --external <addon> can suppress this error, because the parser already marked the bindings record IS_UNUSED (skipped at bundle_v2.rs:5884) and the NativeBindings not-found branch does continue 'outer before ever reaching resolve_with_framework (the only place external patterns are consulted). This is also a behavior regression — a project that today passes --external bindings to defer addon loading to runtime will now fail to build when the .node file isn't present on the build machine. Consider checking the resolver's external patterns for bindings / the addon name before emitting the fatal error, and reword the hint to name the working escape (--external <package-that-calls-bindings>, e.g. --external sqlite3).

  Extended reasoning...

  What the bug is

  The new error message reads:

  Could not locate native addon "…" for the "bindings" package. Run "bun install" to build it, or mark the import external.

  But there is no --external value the user can pass that suppresses this error for the import in question. The remedy is unactionable as written, and the rewrite additionally regresses the previously-working --external bindings escape hatch.

  Code path that triggers it

  Step 1 — parser rewrite is unconditional on --external. In src/js_parser/visit/visit_expr.rs:2015-2064, the rewrite fires whenever p.options.bundle is set and the target is ERequireString with path.text == b"bindings". It then:

  • marks the original bindings import record IS_UNUSED (line ~2032-2034), and
  • creates a new NativeBindings-tagged record whose path.text is the addon name.

  The parser has no access to the resolver's external-pattern list, so --external bindings cannot influence this step.

  Step 2 — --external bindings never reaches the resolver. In bundle_v2.rs:5882-5891, records with IS_UNUSED are skipped before any resolution runs. The bindings record therefore never reaches resolver.resolve_with_framework (line 6169), which is where is_external_pattern is consulted (resolver.rs:1209). So passing --external bindings is a no-op for the immediately-called form.

  Step 3 — --external <addon_name> never reaches the resolver either. The NativeBindings branch at bundle_v2.rs:6047 runs before resolve_with_framework. In the None arm it sets path.is_disabled = true, logs the error, and does continue 'outer at line 6091 — the resolver is never called for this record, so is_external_pattern(addon_name) is never consulted.

  There is no external-pattern check anywhere between line 5884 and line 6047; only builtin/bun:*/onResolve-plugin handling sits in between.

  Concrete walkthrough

  Given node_modules/mypkg/index.js containing module.exports = require('bindings')('myaddon') with no myaddon.node on disk, and the user runs:

  1. Parser visits the call, sees bundle=true, matches ERequireString("bindings"), marks record #N IS_UNUSED, creates record #M tagged NativeBindings with path.text = "myaddon".
  2. Resolve loop iteration for record #N: IS_UNUSED → continue at 5890. --external bindings never checked.
  3. Resolve loop iteration for record #M: enters NativeBindings block at 6047, resolve_native_bindings returns None, HANDLES_IMPORT_ERRORS is unset → error logged → continue 'outer at 6091. --external myaddon never checked.
  4. Build fails with "…or mark the import external", but the user already did.

  Trying --external myaddon or --external myaddon.node produces the identical failure for the same reason (step 3 short-circuits before the resolver).

  The only working escape is --external mypkg — externalizing the enclosing package so mypkg/index.js is never parsed at all — which the message does not say and which a user reading "mark the import external" would not naturally infer (the import in question is bindings, not mypkg).

  Why this matters

  • REVIEW.md is explicit: "Error messages are reviewed word-for-word as code … a concrete remedy." A remedy that cannot work as stated blocks merge.
  • Behavior regression: before this PR, bun build --outdir dist --external bindings ./entry.ts would emit require('bindings')('myaddon') verbatim, and the bundle would work at runtime provided node_modules is deployed alongside it. After this PR, the same command fails the build when the .node file isn't present on the build machine (common when bundling in CI/Docker without native toolchains and building the addon on the target). The flag the user already relies on is now silently ignored for this shape.

  How to fix

  Two independent pieces:

  1. Make the escape hatch work. In the None arm of the NativeBindings block (bundle_v2.rs:6062-6091), before emitting the error, consult transpiler.resolver.is_external_pattern(b"bindings") (and/or is_external_pattern(import_record.path.text)). If either matches, skip the error — either leave the record disabled (so the call becomes a no-op like a try/catch fallthrough) or, better, restore the original require('bindings')(name) shape as an external call so runtime resolution still works.
  2. Reword the remedy to name the escape that actually works today: e.g. "…or add --external <package> for the package that loads it." — pointing at the enclosing package (sqlite3, mypkg) rather than at bindings or the addon.

[claude]

No further findings from me — the earlier three rounds of feedback (builtin-alias/onResolve interception, --external bindings regression, vacuous test assertion) are all addressed and now covered by tests. Deferring to a human because this is a user-facing bundler feature with design choices (hard-fail on missing .node, which bindings search prefixes to probe, --external semantics) and two open sibling PRs (#35642, #35688) that a maintainer should reconcile.

What was reviewed: the Tag::NativeBindings insertion vs. the ordinal is_internal() check (safe — placed before Runtime, threshold is symbolic); that the rewrite only fires under p.options.bundle with a string-literal arg so transpile-only and non-literal callers are untouched; that the not-found path sets is_disabled and honours HANDLES_IMPORT_ERRORS / ignore_module_resolution_errors / --external bindings; and that the new napi --compile test runs the binary from an empty cwd so it can't accidentally find the source addon.

Extended reasoning...

Overview

The PR adds build-time resolution of require('bindings')('<name>') to the actual .node addon so bun build --compile can embed native modules. It touches four native source files: a new Tag::NativeBindings enum variant in src/ast/import_record.rs; a parser rewrite in src/js_parser/visit/visit_expr.rs that marks the bindings import record IS_UNUSED and emits a tagged require of the addon name; resolve_native_bindings + tag-specific handling in src/bundler/bundle_v2.rs that probes the enclosing package's build/Release-style directories and either rewrites to an absolute .node path or fails the build; and a pub(crate) → pub visibility widening on Resolver::is_external_pattern. Six new itBundled cases and one end-to-end napi --compile test cover the happy path, not-found error, try/catch fall-through, builtin-name collision, --external escape, and non-literal passthrough.

Security risks

None identified. The filesystem probing (bun_sys::exists over a fixed prefix list joined under the enclosing package.json's directory) runs at build time on the developer's machine, not on untrusted input; the addon name is a literal from source the user is already bundling.

Level of scrutiny

High. This is a new user-facing bundler behaviour with a build-failure mode that did not exist before: a project whose .node addon is not yet built (fresh clone, cross-compile host, CI without native deps) now fails bun build unless the call is in try/catch or bindings is externalised. The PR intentionally omits two of the real bindings package's search paths (the ABI-versioned ones) and does not handle the object-argument form require('bindings')({ bindings: 'name' }). Those are reasonable scoping decisions but a maintainer should sign off on them, and on which of the three competing PRs (#35642, #35688, this one) to land.

Other factors

I reviewed this PR three times previously; each finding (builtin-alias / onResolve plugin intercepting the addon name, --external bindings no longer suppressing the error and the error text naming an unreachable remedy, a vacuous .not.toContain assertion, and the misplaced #8697 issue comment) was addressed in commits ae43811, 83ebb0e, and 1757a9f, with new tests compile/NativeBindingsBuiltinNameDoesNotAlias and compile/NativeBindingsExternalSkipsError covering the fixes. This run's bug hunt found nothing new. I spot-checked that inserting NativeBindings mid-enum does not break Tag::is_internal() (it compares against Tag::Runtime as u8 symbolically, and the new variant sits below that threshold). Test coverage looks solid for the shapes handled; the CI failures in the robobun comment (36577.test.ts, bun-upgrade.test.ts) are unrelated to this change.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js_parser/visit/visit_expr.rs`:
- Around line 2010-2055: Update the bindings handling in visit_expr.rs around
the require-expression transformation so the original runtime
require("bindings") call remains available when the derived native addon record
cannot be resolved, rather than marking it unused unconditionally; preserve or
emit an external fallback for that case. In src/bundler/bundle_v2.rs around
lines 6045-6100, add a runtime regression test covering an external bindings
package with a missing addon and verify the generated code still performs the
runtime lookup.

In `@test/bundler/bundler_compile.test.ts`:
- Around line 393-408: Strengthen the itBundled case
“compile/NativeBindingsTryCatchFallsThrough” by inspecting the generated bundle
and asserting it excludes the native-binding failure text “Could not find module
root given file” and the getRoot symbol. Keep the existing fallback runtime
assertion, ensuring the test also detects when the bindings call is not
suppressed and rewritten.

In `@test/napi/napi.test.ts`:
- Around line 207-217: Update the N-API executable test around spawnSync to
create a dedicated fixture temporary directory, set BUN_TMPDIR to that directory
in the child environment, and assert readdirSync on it is empty after execution.
Keep the existing output and success assertions, and do not rely on cwd/runDir
to verify embedded native-addon usage.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e265731f-ee6f-4d77-b62a-5716109a6a52

📥 Commits

Reviewing files that changed from the base of the PR and between f91d5c9 and 600f61c.

📒 Files selected for processing (6)

• src/ast/import_record.rs
• src/bundler/bundle_v2.rs
• src/js_parser/visit/visit_expr.rs
• src/resolver/resolver.rs
• test/bundler/bundler_compile.test.ts
• test/napi/napi.test.ts

[claude]

No new findings this pass — the earlier three rounds of feedback (builtin-alias/onResolve interception, the --external escape hatch and error wording, and the vacuous ExternalSkipsError assertion) have all been addressed. Deferring to a maintainer: this is a user-facing bundler feature with design choices (probe-path set, hard-fail on missing addon, --external bindings semantics) and two open PRs (#35642, #35688) covering the same ground.

What was reviewed:

• Tag::NativeBindings insertion vs. Tag::is_internal() ordinal check — placed before Runtime, so the >= Runtime comparison is preserved and no other code keys on the discriminant value.
• Parser rewrite in visit_expr.rs: the bindings record is marked IS_UNUSED only after add_import_record_by_range reborrows p.import_records, and import_records_for_current_part is updated so the new record is linked.
• resolve_native_bindings: uses bun_sys::exists + pooled path buffer + enclosing_package_json walk; the resolved path is arena-interned before assignment.

Extended reasoning...

Overview

The PR adds build-time resolution of require('bindings')('<name>') so that bun build --compile can embed the referenced .node addon instead of shipping a binary that fails at runtime with "Could not find module root". It touches four source files: a new Tag::NativeBindings enum variant in src/ast/import_record.rs; a parser rewrite in src/js_parser/visit/visit_expr.rs that detects the call shape, marks the bindings import unused, and emits a tagged require record for the addon name; a filesystem probe + record rewrite in src/bundler/bundle_v2.rs::resolve_import_records (with a new resolve_native_bindings helper); and a pub(crate) → pub visibility widening on Resolver::is_external_pattern. Six new itBundled cases in test/bundler/bundler_compile.test.ts and one end-to-end --compile test in test/napi/napi.test.ts cover the feature.

Security risks

None identified. The probe reads only from directories under the enclosing package.json of the importing file at build time (same trust boundary as the resolver already operates in), and the addon name comes from a source-code string literal, not runtime input. No path components are user-controlled beyond what the bundler already resolves.

Level of scrutiny

High. This is a feature change to the bundler's import-record resolution loop and the parser's visit pass — both hot, correctness-critical paths that affect every bundle. It changes user-visible behaviour: builds that previously succeeded (and produced a broken-at-runtime bundle) now hard-fail when the .node file is absent and the call is not in a try/catch. The chosen probe-directory list, the decision to skip the ABI-versioned compiled/ and lib/binding/ paths, and the --external bindings escape-hatch semantics are all design calls a maintainer should sign off on. There are also two open PRs (#35642, #35688) that github-actions flagged as covering the same ground with the same approach.

Other factors

I've already run three review passes on this PR; each finding was addressed with a follow-up commit and a covering test (NativeBindingsBuiltinNameDoesNotAlias, NativeBindingsExternalSkipsError, and the tightened assertions in 1757a9f). This pass I additionally checked that inserting NativeBindings before Runtime in the #[repr(u8)] enum does not break Tag::is_internal() (it compares >= Tag::Runtime as u8, and grep shows no other ordinal use of this enum), and that the parser reborrows p.import_records after add_import_record_by_range may have grown it, so the earlier record borrow does not dangle. The two CI failures reported by robobun (36577.test.ts on Windows x64 and bun-upgrade.test.ts on Windows aarch64) are unrelated to the changed files. Given the scope and the duplicate-PR situation, this needs a human maintainer's decision rather than a bot approval.

[robobun]

CI status for the current revision (5fee0de, build #94182): every build lane and 155 of 156 completed test shards are green, including the bundler and napi suites this PR touches (bundler_loader.test.ts require('bindings') cases and the napi.test.ts --compile cases passed on every lane that ran them). The one red shard is test/js/web/timers/setInterval.test.js on x64-asan, which is failing on main independently of this change and has been reported separately. The remaining entries in the annotation (napi_get_value_string output ordering, child_process, spawn-maxbuf, and a few others) passed on retry or when run alone. Four Windows shards expired without getting an agent (the same build-queue issue seen on earlier builds); the Windows shards that did run passed.

Earlier revision (600f61c, before the rework): build #87502

All build lanes green; all Linux test lanes (incl. ASAN), macOS, and the Windows shards that got an agent were green. Remaining red was Azure VM-creation throttling, the pre-existing bun-upgrade.test.ts win-aarch64 failure, and flaky tests that passed alone or on retry. Builds #87266 and #87342 before it expired without running anything (build-queue backlog).

[robobun]

Note on the review threads and the Files tab: after main was merged into this branch (4b15a09), GitHub is still listing the whole merge as the PR's changes (1946 files). The diff against the actual merge base is 11 files:

git diff --stat origin/main...5fee0de865
 docs/bundler/executables.mdx        |   2 +
 src/ast/import_record.rs            |   4 +
 src/bundler/ParseTask.rs            |   8 +
 src/bundler/bundle_v2.rs            |  42 +
 src/js_parser/p.rs                  |  74 +
 src/js_parser/parse/parse_entry.rs  |   1 +
 src/js_parser/parser.rs             |   7 +
 src/js_parser/visit/visit_expr.rs   |  10 +
 src/resolver/resolver.rs            |  55 +
 test/bundler/bundler_loader.test.ts | 172 +-
 test/napi/napi.test.ts              |  59 +

The 120 comment-cop threads from the latest run were generated from that stale file list; all but one are on lines that belong to main (PipeReader, NodeCompileCache, bun_core strings, and so on), so I have resolved them without changes. The one on code this PR adds (import_record.rs, the NativeBindings doc comment) is answered inline.