resolver: read every default main-field name when parsing package.json

[robobun]

Fixes #14253.

Repro

// node_modules/dual-pkg/package.json
{ "name": "dual-pkg", "main": "./node.js", "browser": "./browser.js" }

// build.ts
import v from "dual-pkg";              // runtime import (target=bun)
const r = await Bun.build({ entrypoints: ["./entry.ts"], target: "browser" });
// r.outputs[0] bundles ./node.js instead of ./browser.js

Same symptom for two sequential Bun.build calls with different targets in the same process: a target: "bun" build followed by a target: "browser" build bundles the main entry for both.

Cause

PackageJSON::parse populated main_fields by iterating only the invoking resolver's opts.main_fields:

for main in r.opts.main_fields.iter() {
    if let Some(main_json) = json.as_property(main) { ... main_fields.put(main, ...) }
}

The parsed PackageJSON is then interned into the process-global DirInfo cache (keyed only by directory path). The runtime's resolver (target bun, main_fields = ["module", "main", "jsnext:main"]) parses first and never stores "browser". A later Bun.build({ target: "browser" }) reuses that cached PackageJSON, iterates its own opts.main_fields = ["browser", "module", ...], fails to find "browser" in the map, and falls through to "main".

The object-form "browser" map right below this block is already parsed unconditionally for exactly this reason (see the existing comment in package_json.rs); the string-form main field was not.

Fix

Always read the fixed union of every per-target default (browser, module, main, jsnext:main) into main_fields, in addition to r.opts.main_fields for custom user-configured names. This matches esbuild, which reads a hardcoded ["main", "module", "browser"] at parse time independent of options.MainFields.

The consuming loop in load_as_file_or_directory iterates the current resolver's opts.main_fields and looks each key up in the stored map, so storing extra entries is harmless: a target that does not list "browser" never reads it.

There is one literal table for the default names. bun_ast::Target::MAIN_FIELD_NAMES is the canonical list (it already backs Target::default_main_fields). bun_resolver::options::ALL_DEFAULT_MAIN_FIELD_NAMES is its byte view, which PackageJSON::parse reads. bun_bundler re-exports that byte view as TARGET_MAIN_FIELD_NAMES instead of keeping its own copy.

Verification

$ USE_SYSTEM_BUN=1 bun test test/bundler/bun-build-api.test.ts -t "target main-field"
(fail) runtime import then Bun.build({ target: 'browser' })   Expected: "browser"  Received: "node"
(fail) Bun.build for each target in the same process          browser: "node", browser2: "node"

$ bun bd test test/bundler/bun-build-api.test.ts -t "target main-field"
(pass) runtime import then Bun.build({ target: 'browser' })
(pass) Bun.build for each target in the same process

Also green after the rebase: full bun-build-api.test.ts (55 pass, 0 fail) with the debug build.

Rebase notes

Main deleted the resolver crate's per-target DEFAULT_MAIN_FIELDS_* block as dead code and moved the canonical per-target lists into bun_ast::Target (same union-index structure this PR had introduced). The branch was rebuilt on top of that: the union constant now derives from Target::MAIN_FIELD_NAMES (made pub, str to bytes via as_bytes()), and the bundler's byte table became a re-export. The parse-side fix and the tests are unchanged.

---

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

[coderabbitai]

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

• Run review for free

_{On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file.}

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 25 minutes), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 763b55ed-da57-46de-ab91-0d3925eae099

📥 Commits

Reviewing files that changed from the base of the PR and between 1b88ad3 and ce3377d.

📒 Files selected for processing (5)

• src/ast/target.rs
• src/bundler/options.rs
• src/resolver/options.rs
• src/resolver/package_json.rs
• test/bundler/bun-build-api.test.ts

---

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

[claude]

LGTM — targeted fix that reads all default main-field names into the cached PackageJSON so a later resolver with a different target can find its field.

What was reviewed:

• Confirmed the consuming loop at resolver.rs:5447 iterates self.opts.main_fields and looks each key up in the map, so storing extra entries is harmless — a target that doesn't list browser never reads it.
• Verified StringArrayHashMap::put overwrites on collision, so the overlap between ALL_DEFAULT_MAIN_FIELD_NAMES and r.opts.main_fields is a no-op.
• Checked the main_fields.count() > 0 gate: newly entering the block with a non-matching map falls through cleanly to index resolution, same as skipping it.
• Tests spawn subprocesses with tempDir/bunEnv, drain pipes concurrently, assert exact output, and cover both runtime→build and sequential-build orderings.

Extended reasoning...

Overview

The PR fixes a resolver cache poisoning bug (#14253): PackageJSON::parse populated main_fields using only the invoking resolver's opts.main_fields, but the parsed struct is interned into a process-global DirInfo cache keyed by directory path. A runtime import (target=bun) would parse first and never store "browser"; a subsequent Bun.build({ target: 'browser' }) would reuse that cached entry and fall through to "main".

The fix introduces ALL_DEFAULT_MAIN_FIELD_NAMES (the union of every per-target default: browser/module/main/jsnext:main) in options.rs and reads all of them at parse time, plus any user-configured names from r.opts.main_fields. This mirrors esbuild's approach of hardcoding ["main", "module", "browser"] at parse time independent of options.MainFields.

Security risks

None. This changes which package.json string fields are stored in an in-memory cache; no untrusted input parsing, path handling, or privilege boundaries are touched.

Level of scrutiny

Moderate — resolver correctness affects every build. I traced the change through to the consumer at resolver.rs:5424-5583: the loop iterates self.opts.main_fields (the current resolver's ordered list) and calls main_field_values.get(key) on the cached map, so extra stored keys are dead entries for targets that don't request them. put overwrites on collision (array_hash_map.rs:1370), so the overlap between the two loops just re-stores the same value. The count() > 0 gate can now be true where it was previously false (e.g. bun target parsing a browser-only package.json), but the loop then finds no keys and falls through to the same index-file resolution at line 5575.

Other factors

The tests are well-constructed: two subprocess-spawned test.concurrent cases in the existing bun-build-api.test.ts, one covering runtime-import→browser-build (the exact issue repro) and one covering all four target permutations sequentially with a re-run of browser at the end. Assertions are exact (toBe("browser"), toEqual({...})), pipes are drained concurrently, exit code asserted last. PR description shows USE_SYSTEM_BUN=1 fails and debug build passes, plus the related suites (bundler_browser, esbuild/packagejson) stay green.

The one known limitation — a custom --main-fields on a second build after a different first parse still won't find its custom field in the cache — is not a regression (it was already broken) and is orthogonal to this fix.

[robobun]

^{Updated 8:02 PM PT - Aug 20th, 2026}

✅ @robobun, your commit ce3377daaf6906b94c339cc6cbe5794eb2893fc5 passed in Build #102087! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 35595

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

bun-35595 --bun

[robobun]

Reproduced with a package that has "main" + string-form "browser" (no exports): a runtime import followed by Bun.build({ target: 'browser' }) bundles the main entry. PR is up; second commit tightens the per-target default lists to index into ALL_DEFAULT_MAIN_FIELD_NAMES so the union invariant is structural. Waiting on CI.

[claude]

Beyond the inline nit, I verified: StringArrayHashMap::put overwrites on hit (array_hash_map.rs:1370), so the ALL_DEFAULT + r.opts.main_fields overlap is a no-op; the consumer at resolver.rs:5447 iterates self.opts.main_fields and does keyed lookups, so extra stored entries are never read by a target that doesn't list them; and the main_fields.count() > 0 gate now being entered for a package with only "browser" under target=bun still falls through to load_as_index_with_browser_remapping identically to the count() == 0 path.

Extended reasoning...

Checked the two loops don't double-insert or reorder (put preserves the original key on hit), that the consuming site reads only the current resolver's opts.main_fields so over-populating the cached map can't leak a browser entry into a bun/node resolution, and that the newly-always-nonzero count() doesn't short-circuit anything the empty-map path would have reached. Resolver is a critical path so leaving the merge decision to a human; the comment-cop flag looks addressed by the second commit (invariant is now structural via ALL_DEFAULT_MAIN_FIELD_NAMES indexing, and the remaining 3-line comment states the cache invariant rather than justifying a workaround).

[claude]

LGTM — the third commit addressed my earlier note by making ALL_DEFAULT_MAIN_FIELD_NAMES the single source of truth and re-exporting it as TARGET_MAIN_FIELD_NAMES from the bundler crate, so both crates' per-target lists now derive from one table.

Verified the indexed per-target statics in both src/resolver/options.rs and src/bundler/options.rs resolve to the same byte-slices as before (NODE=[main,module], BROWSER=[browser,module,jsnext:main,main], BUN=[module,main,jsnext:main]). The double iteration in PackageJSON::parse is harmless — MainFieldMap::put on a duplicate key overwrites with the same value. The two open comment-cop flags are on 2–3-line doc comments that match the file's existing re-export style (see TARGET_MAP directly above) and the moved jsnext:main URL note, not workaround justifications.

Extended reasoning...

Overview

Fixes #14253: PackageJSON::parse now unconditionally reads the union of all per-target default main-field names (browser, module, main, jsnext:main) into the cached main_fields map, plus whatever r.opts.main_fields lists (for user-configured names). Since the parsed PackageJSON is cached process-globally by directory, this ensures a later Bun.build with a different target finds the field it needs regardless of which resolver parsed first. The consuming loop in load_as_file_or_directory still iterates only the current resolver's opts.main_fields, so extra stored entries are inert.

The third commit (719a1ff) responded to my earlier inline comment: ALL_DEFAULT_MAIN_FIELD_NAMES is now pub in the resolver crate and bun_bundler::options re-exports it as TARGET_MAIN_FIELD_NAMES (dropping its own copy), and both crates' DEFAULT_MAIN_FIELDS_* per-target constants index into that one array. bun_ast::Target::MAIN_FIELD_NAMES remains a separate &str-typed private copy — noted as out-of-scope follow-up in my prior comment.

Security risks

None. This changes which package.json string properties are read into an in-memory map at parse time; no untrusted-input parsing, path handling, or privilege boundaries are affected.

Level of scrutiny

Moderate — resolver is a hot, correctness-critical path, but the change is narrow: it only adds keys to a map whose consumer already does keyed lookup against its own ordered list. I hand-verified the array-index rewrites in both crates produce byte-identical per-target orderings, and checked that StringArrayHashMap::put on a duplicate key (from the second r.opts.main_fields loop) is a benign overwrite. This matches esbuild's approach (hardcoded [main, module, browser] at parse time).

Other factors

Two subprocess tests cover both failure modes from the issue (runtime-import-then-build, and sequential builds across all three targets including a repeat), with USE_SYSTEM_BUN=1 failure demonstrated in the PR description. The bug-hunting system found nothing. The two unresolved comment-cop bot flags target ordinary 2–3-line doc comments (one is the pre-existing jsnext:main jsforum URL moved verbatim from the bundler crate; the other matches the TARGET_MAP re-export comment style two lines above it) — they are the multi-line-comment heuristic firing, not substantive review feedback.

[robobun]

Addressed the review feedback: ALL_DEFAULT_MAIN_FIELD_NAMES in bun_resolver::options is now the single source of truth, with bun_bundler re-exporting it as TARGET_MAIN_FIELD_NAMES instead of keeping a byte-identical copy (719a1ff), and the flagged multi-line comments are condensed to one line each (12e9631). No behavior change; the per-target orderings are unchanged.

[claude]

LGTM — the earlier feedback was addressed (719a1ff makes bun_bundler re-export ALL_DEFAULT_MAIN_FIELD_NAMES instead of duplicating it; 12e9631 condenses the flagged comments).

Checked that the index-based rewrite of DEFAULT_MAIN_FIELDS_{NODE,BROWSER,BUN} in both crates is byte-for-byte identical to the previous literal orderings.
Verified the consumer at resolver.rs:5447 iterates self.opts.main_fields and does keyed .get()s, so extra stored entries can't affect targets that don't list them; the count() > 0 guard change is also harmless there.
Confirmed StringArrayHashMap::put upserts, so the overlap between ALL_DEFAULT_MAIN_FIELD_NAMES and r.opts.main_fields doesn't create duplicate map entries.

Extended reasoning...

Overview

Fixes #14253: PackageJSON::parse populated main_fields from only the invoking resolver's opts.main_fields, but the parsed struct is interned into a process-global DirInfo cache keyed only by directory. A runtime import (target bun, no browser in its main-fields list) followed by Bun.build({ target: 'browser' }) in the same process reused the cached entry and never found the string-form "browser" field. The fix reads the fixed union of all per-target defaults (browser, module, main, jsnext:main) unconditionally, plus the invoking resolver's list for user-configured names. Two options files were reworked so ALL_DEFAULT_MAIN_FIELD_NAMES is defined once in bun_resolver::options and re-exported by bun_bundler, and the per-target default lists index into it. Two subprocess-isolated regression tests cover runtime-then-build and sequential-builds-per-target.

Security risks

None. No untrusted input parsing, no path traversal, no allocation sizing. The change only widens which package.json top-level string properties get copied into an internal map at parse time.

Level of scrutiny

Medium: this is resolver behavior, which is load-bearing. But the change is strictly additive (stores more keys, never fewer), and I verified the sole consumer (load_as_file_or_directory at resolver.rs:5424+) drives lookups off self.opts.main_fields and does keyed .get() — it never iterates the stored map — so extra entries provably cannot change resolution for a target whose list doesn't include them. This matches esbuild's approach (hardcoded [main, module, browser] at parse time). The refactor of the per-target default lists to index into ALL_DEFAULT_MAIN_FIELD_NAMES is mechanically verifiable and the orderings are unchanged: NODE=[main,module], BROWSER=[browser,module,jsnext:main,main], BUN=[module,main,jsnext:main]. The bundler's TARGET_MAIN_FIELD_NAMES re-export preserves the same array contents and order.

Other factors

• My prior review flagged the constant-table duplication as a follow-up nit; the author addressed it in this PR by having bun_bundler re-export the resolver's constant instead of keeping a copy. The remaining bun_ast::Target::MAIN_FIELD_NAMES copy is &str-typed and private — a separate cleanup, as noted before.
• All comment-cop and inline review threads are resolved.
• The redundant .put() calls (defaults ∩ r.opts.main_fields) are harmless: StringArrayHashMap::put upserts (array_hash_map.rs:1370), and it's ~3 extra property lookups on a one-time-per-package parse.
• Tests follow harness conventions: tempDir, bunEnv/bunExe, subprocess isolation, test.concurrent, drain stdout/stderr/exited via Promise.all, exact-value assertions with exitCode last.

[claude]

LGTM — the single-source-of-truth follow-up (719a1ff) and comment condensing (12e9631) both look right; bun_bundler now re-exports the resolver's ALL_DEFAULT_MAIN_FIELD_NAMES and both crates' per-target lists index into it, so the orderings are byte-identical to before.

Checked: MainFieldMap::put overwrites on duplicate key, so the second r.opts.main_fields loop re-reading defaults is harmless. Per-target orderings verified against the old literals (NODE=[2,1], BROWSER=[0,1,3,2], BUN=[1,2,3]). Closure borrow of &mut package_json ends via NLL before the browser_map block below. The remaining bun_ast::Target::MAIN_FIELD_NAMES copy is &str-typed/private and out of scope here.

Extended reasoning...

Overview

Fixes #14253: PackageJSON::parse populated main_fields only from the invoking resolver's opts.main_fields, but the parsed struct is interned in a process-global cache keyed by directory path. A runtime import (target=bun, no browser in its main-fields list) would poison the cache so a later Bun.build({ target: 'browser' }) couldn't find the string-form "browser" field and fell through to "main". The fix reads the fixed union of all per-target defaults (browser, module, main, jsnext:main) unconditionally, plus r.opts.main_fields for user-configured custom names — matching esbuild's parse-time behavior. The consuming loop in load_as_file_or_directory iterates the current resolver's list and does map lookups, so storing extra keys is inert for targets that don't list them.

Follow-up commits addressed my earlier nit: ALL_DEFAULT_MAIN_FIELD_NAMES is now pub in bun_resolver::options and bun_bundler::options re-exports it as TARGET_MAIN_FIELD_NAMES instead of keeping a byte-identical copy. Both crates' per-target DEFAULT_MAIN_FIELDS_* statics index into it, so the union invariant is now structural across resolver and bundler. The comment-cop bot's flags on multi-line comments were resolved by condensing to one line each.

Security risks

None. No untrusted input handling changes; the only new reads are of fixed literal property names from an already-parsed JSON tree.

Level of scrutiny

Moderate — PackageJSON::parse is on the resolver hot path, but the change is additive (a few extra as_property lookups per parse) and the constant-table refactor is mechanically verifiable. I checked each per-target index list against the original byte literals and they match exactly (NODE: main,module; BROWSER: browser,module,jsnext:main,main; BUN: module,main,jsnext:main). StringArrayHashMap::put (array_hash_map.rs:1370) overwrites the value on key hit, so the overlap between the two loops (defaults ⊇ per-target list in every case) just re-boxes the same string.

Other factors

Two subprocess-spawned regression tests cover both reported symptoms (runtime-import-then-build, and sequential builds across bun/browser/node/browser). Tests follow harness conventions (tempDir + using, test.concurrent, concurrent pipe drain, exact-value assertions, exit code last). PR description shows the tests fail under USE_SYSTEM_BUN=1 and pass with the fix. All prior inline comments (mine and comment-cop) are resolved. The remaining bun_ast::Target::MAIN_FIELD_NAMES duplicate is private, &str-typed, and pre-dates this PR — reasonable to leave for a separate cleanup.

[robobun]

Ready for review. The diff is done and review threads are resolved. The last two CI runs did not get a verdict on the code: build 80992 passed 166 jobs with only retry-passing flaky tests plus a Windows build step cancelled waiting on artifacts, and the retriggered build 81170 stalled farm-wide (four build-bun steps timed out across linux aarch64, android, freebsd and windows x64 with 118 jobs still queued after 2h). The Rust change compiled on every lane that ran. A maintainer re-running CI or merging is the next step; I'm not pushing further retriggers.

[robobun]

Re-checked against main at 165dc9f now that #14253 is closed. Both tests from this PR still fail on main: after a runtime import of a package whose package.json has a string-form "browser": "./browser.js" field, Bun.build({ target: "browser" }) still bundles the main entry, and a target: "bun" build followed by a target: "browser" build in one process does the same.

The check that closed #14253 used exports conditions and the object-form browser map, both of which are read unconditionally when package.json is parsed. The string-form browser main field is still read through r.opts.main_fields in package_json.rs, so it is still missing from the cached entry when a different target parsed the file first. Keeping this open. It needs a rebase: src/bundler/options.rs conflicts with main.

[claude]

LGTM — the follow-up I flagged earlier is addressed: ALL_DEFAULT_MAIN_FIELD_NAMES is now pub in bun_resolver::options and bun_bundler::options re-exports it as TARGET_MAIN_FIELD_NAMES, so both crates' per-target lists derive from one table.

Checked that the per-target orderings are byte-identical to before (just re-expressed as indices), that main_fields.put() on the same key twice is a harmless overwrite (default names re-read via the r.opts.main_fields loop), and that the consumer in load_as_file_or_directory only reads keys from the current resolver's list so storing extra entries is inert for non-browser targets. Tests follow harness conventions (subprocess, test.concurrent, concurrent pipe drain, using tempDir).

Extended reasoning...

Overview

The PR fixes cross-target cache poisoning of the string-form "browser" main field in PackageJSON::parse (src/resolver/package_json.rs). Parsed package.json is interned process-globally keyed only by directory, so the first resolver to touch a package determined which main-field names were stored. The fix reads the fixed union of all four default main-field names (browser, module, main, jsnext:main) unconditionally, plus r.opts.main_fields for user-configured names — matching esbuild. It also consolidates the main-field-name table: bun_resolver::options::ALL_DEFAULT_MAIN_FIELD_NAMES is now the single source, re-exported by bun_bundler::options as TARGET_MAIN_FIELD_NAMES, with both crates' per-target DEFAULT_MAIN_FIELDS_* constants indexing into it. Two subprocess tests in test/bundler/bun-build-api.test.ts cover runtime-import-then-build and sequential-builds-with-different-targets.

Security risks

None. This is module-resolution metadata parsing from package.json; no untrusted-input validation, auth, crypto, or filesystem-write paths are touched.

Level of scrutiny

Medium. The resolver is a hot, correctness-critical path, but the change is narrow: a ~10-line closure extraction in PackageJSON::parse that adds one extra iteration over a 4-element const array, plus mechanical re-expression of existing per-target constants as indices into a shared table. I verified index-by-index that DEFAULT_MAIN_FIELDS_NODE/BROWSER/BUN in both crates are byte-identical to their previous literal forms, and that the consuming loop iterates opts.main_fields (not the stored map), so extra stored entries cannot change resolution for a target that doesn't list them.

Other factors

My prior inline nit (2026-07-25) asked for exactly the bun_bundler re-export that landed in commit 292dcb7, and the comment-cop flags were addressed by condensing comments. All inline threads are resolved. The bun_ast::Target::MAIN_FIELD_NAMES third copy remains, but I noted that as a separate cleanup and it doesn't affect correctness (it's &str-typed and private). The author's 2026-08-12 note mentions a rebase conflict on src/bundler/options.rs; that's a merge-mechanics concern, not a code-review one.

[claude]

LGTM — both prior concerns are addressed: the two test.concurrent cases from #39799 are back in bun-build-api.test.ts (the diff is now purely additive there), and the main-field name table is down to one literal source (Target::MAIN_FIELD_NAMES) with bun_resolver deriving the byte view and bun_bundler re-exporting it.

Extended reasoning...

Overview

The PR fixes cross-target cache poisoning in the resolver's process-global DirInfo cache: PackageJSON::parse now stores every default main-field name (browser, module, main, jsnext:main) plus any user-configured ones, instead of only the invoking resolver's target-specific list. The read side (load_as_file_or_directory) already iterates the current resolver's opts.main_fields and looks each up, so extra stored entries are inert for targets that don't list them. Five files touched: one visibility change (Target::MAIN_FIELD_NAMES → pub), one const derived from it in bun_resolver::options, one re-export replacing a byte-identical copy in bun_bundler::options, the ~15-line parse-side fix, and two subprocess-spawning tests.

Security risks

None. This only affects which package.json top-level string fields are copied into an in-memory map; the set of names is a fixed compile-time union plus whatever the user already passed via main_fields. No untrusted-input parsing, no path handling changes.

Level of scrutiny

Medium — the resolver is hot-path and correctness-critical, but the change is narrowly scoped to widening a cache-write with a read side that already filters. The mechanism matches esbuild's approach (hardcoded union at parse time). The two overlapping loops re-put() common names, which is a harmless upsert on StringArrayHashMap. I verified the TARGET_MAIN_FIELD_NAMES re-export preserves the same indices the DEFAULT_MAIN_FIELDS_* slices in bundler/options.rs index into, so per-target orderings are unchanged.

Other factors

Both of my earlier review comments are resolved on the current head: the accidentally-dropped #39799 tests are restored (test-file diff is now +85/-0), and the third copy of the name table is gone in favor of derivation + re-export. The comment-cop flags are also resolved (comments condensed to one line each). Tests follow harness conventions (tempDir, bunEnv, concurrent pipe drain, exit code asserted last, test.concurrent). The bug-hunting pass found nothing on this revision. CI on the latest push is building (#102087).