ast/resolver/bundler: collapse dependent bool pairs into enums

[robobun]

Three pairs of booleans whose members are never independently true, collapsed into tri-state enums so the invalid combinations are unrepresentable.

S::Local origin

was_ts_import_equals is set only when parsing TS import x = ... (parse_typescript.rs), and was_commonjs_export is set only when the visitor rewrites exports.x = ... (visit_stmt.rs). Both construct a fresh S::Local via struct literal + Default, so a single local is never both. Folded into:

pub enum LocalOrigin { Normal, TsImportEquals, CommonJsExport }

can_merge_with continues to compare only the commonjs-export bit (origin.is_commonjs_export()) to keep merging of TsImportEquals with Normal locals unchanged.

ResultFlags external kind

Every site that sets IS_EXTERNAL_AND_REWRITE_IMPORT_PATH does so with the same value it just wrote to IS_EXTERNAL (resolver.rs browser/alias remap paths), and the only reader checks it inside if is_external(). Replaced the pair of setters with a single set_external_kind(ExternalKind); storage stays packed in the existing ResultFlags: u8.

pub enum ExternalKind { NotExternal, External, ExternalRewritePath }

BakeExtra route kind

generateChunksInParallel only assigns fully_static inside the is_route = true branch, and is_route itself is never read anywhere. Folded into:

pub enum BakeRouteKind { NotRoute, Route, FullyStaticRoute }

Why

Each pair of bools encodes four states where only three are reachable. The enum makes the invariant structural instead of incidental, so future call sites cannot produce the nonsensical combination (rewrite && !external, fully_static && !is_route, both origin bits set). For BakeExtra this also drops the write-only is_route field.

Verification

No behavior change. Verified against existing coverage:

• test/bundler/bundler_cjs2esm.test.ts (commonjs-export rewrite path)
• test/bundler/esbuild/ts.test.ts (TS import-equals)
• test/bundler/esbuild/packagejson.test.ts, test/bundler/bundler_browser.test.ts (browser remap / external rewrite)
• test/bundler/bundler_edgecase.test.ts
• test/bake/dev-and-prod.test.ts, test/bake/framework-router.test.ts

This is a type-narrowing refactor; there is no input that behaves differently before vs. after, so no new regression test is included.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b241f1cb-86ec-43c2-a9f1-dc1eb623425b

📥 Commits

Reviewing files that changed from the base of the PR and between 8f50cb5 and f44ee83.

📒 Files selected for processing (3)

• src/ast/s.rs
• src/bundler/OutputFile.rs
• src/resolver/result.rs

Walkthrough

The change replaces boolean metadata with LocalOrigin, ExternalKind, and BakeRouteKind enums. Parser, resolver, bundler, and runtime code now assign, preserve, and query these typed states.

Changes

Typed state metadata

Layer / File(s)	Summary
Local origin metadata and propagation src/ast/s.rs, src/js_parser/..., src/bundler/linker_context/...	LocalOrigin replaces local origin flags. TypeScript import-equals and CommonJS-export paths now assign or query the enum.
External resolution state src/resolver/result.rs, src/resolver/resolver.rs, src/resolver/lib.rs, src/bundler/bundle_v2.rs	ExternalKind represents resolver external states. Browser remapping and bundle rewrite detection use the enum.
Bake route metadata and runtime use src/bundler/OutputFile.rs, src/bundler/linker_context/generateChunksInParallel.rs, src/runtime/bake/production.rs	BakeRouteKind replaces route flags. Chunk generation assigns the enum, and production baking checks is_fully_static().

Possibly related PRs

• oven-sh/bun#35971: This PR also changes CommonJS export handling in src/js_parser/p.rs and parse_entry.rs, but it adds runtime export-name tracking instead of replacing origin flags.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main refactor from dependent boolean pairs to enums across the AST, resolver, and bundler.
Description check	✅ Passed	The description explains the refactor, its rationale, affected areas, behavior preservation, and verification coverage.
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.

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 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/resolver/result.rs`:
- Line 155: Remove the redundant “Convenience accessors with field-style names.”
comment above the accessor definitions in the result implementation, leaving the
accessor code unchanged.
- Around line 162-170: Update ResultFlags::external_kind so the
REWRITE_IMPORT_PATH and IS_EXTERNAL invariant is enforced in release builds,
replacing the debug-only assertion with an unconditional check or otherwise
preventing rewrite-only flag values. Preserve the existing ExternalRewritePath,
External, and NotExternal classification for valid flag combinations.

🪄 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: b8b4cb08-4006-4128-9671-a80919de2359

📥 Commits

Reviewing files that changed from the base of the PR and between f0043ff and 7fe5289.

📒 Files selected for processing (16)

• src/ast/s.rs
• src/bundler/OutputFile.rs
• src/bundler/bundle_v2.rs
• src/bundler/linker_context/convertStmtsForChunk.rs
• src/bundler/linker_context/generateChunksInParallel.rs
• src/bundler/linker_context/generateCodeForFileInChunkJS.rs
• src/js_parser/p.rs
• src/js_parser/parse/parse_entry.rs
• src/js_parser/parse/parse_typescript.rs
• src/js_parser/scan/scan_imports.rs
• src/js_parser/visit/mod.rs
• src/js_parser/visit/visit_stmt.rs
• src/resolver/lib.rs
• src/resolver/resolver.rs
• src/resolver/result.rs
• src/runtime/bake/production.rs

[claude]

LGTM — clean type-narrowing refactor that makes invalid state combinations unrepresentable.

What was reviewed:

• Grepped for every removed identifier (was_ts_import_equals, was_commonjs_export, set_is_external, is_external_and_rewrite_import_path, fully_static, is_route) — no stale references; the is_route hits in incremental_graph.rs are on an unrelated dev-server type.
• Verified can_merge_with still compares only the commonjs-export bit, so TsImportEquals locals continue to merge with Normal as before.
• Checked set_external_kind writes both bits and that direct ResultFlags::IS_EXTERNAL struct literals elsewhere in resolver.rs remain valid states.
• Confirmed writeOutputFilesToDisk.rs only uses BakeExtra::default(), so the new BakeRouteKind::NotRoute default preserves the old is_route: false, fully_static: false.

Extended reasoning...

Overview

This PR collapses three pairs of dependent booleans into tri-state enums across 16 files in the AST (S::Local), resolver (ResultFlags), and bundler (BakeExtra). Each pair had the invariant that one bool implied the other (or they were mutually exclusive), so only three of four states were reachable. The refactor is a 1:1 mechanical translation of every read and write site.

Security risks

None. This is an internal type refactor with no user-facing surface, no parsing of untrusted input, and no change to control flow.

Level of scrutiny

Medium. The touched files are core (parser, resolver, bundler linker), but the change is purely representational — the compiler enforces that every construction and match site was updated. The real risk in a bool→enum collapse is a subtle semantic drift at a comparison site; the PR explicitly calls out the one such site (can_merge_with compares only is_commonjs_export(), not full origin equality) and I verified the truth table matches the old behavior for all origin pairs.

I grepped for every removed field/method name and confirmed zero remaining references. The two .is_route hits in dev_server/incremental_graph.rs are on a different struct entirely. The six ResultFlags::IS_EXTERNAL struct-literal initializers in resolver.rs (unchanged by this PR) produce ExternalKind::External under the new decoder, which is correct — they never set the rewrite bit before either.

Other factors

• The ..Default::default() removals at the two S::Local construction sites are safe: the struct now has exactly four fields, all listed explicitly, so a missing field would be a compile error.
• set_external_kind(NotExternal) clears both bits, matching the old set_is_external(false); set_is_external_and_rewrite_import_path(false) sequence even if the flags had been set earlier on the same result.
• The PR description names the existing test files that exercise each of the three paths; no new test is warranted since there is no input that behaves differently before vs. after.
• No prior reviews or outstanding comments on the PR.

[robobun]

^{Updated 2:05 AM PT - Aug 2nd, 2026}

@robobun, your commit f44ee83 is building: #87712

[robobun]

Status: ready for review.

Self-review traced every write and read site of the three bool pairs against main and found no semantic divergence; all 33 adversarial probes were dismissed as either behavior-equivalent or pre-existing. Review-bot feedback (comment trimming, external_kind() ordering) is addressed in f44ee83.

Existing coverage exercised and passing locally: bundler_cjs2esm, esbuild/ts, esbuild/packagejson, bundler_browser, bundler_edgecase, bake/dev-and-prod, bake/framework-router. Build 87681's only non-flaky red was test/napi/napi.test.ts (napi_wrap GC timing on windows-aarch64), unrelated to this diff.

No new regression test is included: this is a representation-only refactor, so by construction there is no input whose output differs between main and this branch. Any test added here would pass identically on both, which does not meet the fail-before/pass-after bar. Happy to adjust if a maintainer wants a different shape here.

[claude]

LGTM — mechanical type-narrowing refactor; my earlier note about the unstaged result.rs hunks in 8f50cb5 is resolved by f44ee83.

What was reviewed:

• Traced every read/write of the three replaced bool pairs; can_merge_with still compares only the commonjs-export bit, merge_adjacent_local_stmts preserves origin, and external_kind() now agrees with is_external() for every bit pattern.
• Confirmed no stale callers of was_ts_import_equals/was_commonjs_export/set_is_external/is_external_and_rewrite_import_path/fully_static remain; the .is_route hits in incremental_graph.rs are on an unrelated dev-server struct.
• Checked the two S::Local literals that dropped ..Default::default() — both now spell out all four fields, so no default is silently changed.
• Verified all six literal flags: ResultFlags::IS_EXTERNAL construction sites in resolver.rs set only IS_EXTERNAL (= ExternalKind::External), consistent with the new invariant.

Extended reasoning...

Overview

This PR collapses three pairs of dependent booleans into tri-state enums across 16 files in the parser (src/js_parser/), AST (src/ast/s.rs), resolver (src/resolver/), bundler (src/bundler/), and bake runtime (src/runtime/bake/production.rs). Each pair had the invariant that at most one member could be true; the enum makes that structural. The three replacements are S::Local::{was_ts_import_equals, was_commonjs_export} → LocalOrigin, ResultFlags::{IS_EXTERNAL, IS_EXTERNAL_AND_REWRITE_IMPORT_PATH} → ExternalKind (still bit-packed), and BakeExtra::{is_route, fully_static} → BakeRouteKind.

Security risks

None. This is a representation-only change to internal data structures with no user-facing surface, no parsing of untrusted input, and no auth/crypto/permissions involvement.

Level of scrutiny

Medium — the touched files are core (parser, resolver, bundler linker), but every hunk is a mechanical field-rename or accessor swap. Per REVIEW.md's "treat every refactor as guilty until proven behavior-preserving," I traced each read and write site:

• LocalOrigin: can_merge_with compares origin.is_commonjs_export() on both sides, matching the old behavior where was_ts_import_equals was not compared (so TsImportEquals still merges with Normal). The two construction sites in parse_typescript.rs and visit_stmt.rs dropped ..Default::default() but now explicitly list all four S::Local fields, so nothing is silently defaulted. merge_adjacent_local_stmts and parse_entry.rs copy origin where they previously copied both bools. All five read sites (convertStmtsForChunk, p.rs, parse_typescript, scan_imports, visit/mod) use the accessor matching the bool they previously read.
• ExternalKind: The three resolver.rs sites that previously wrote both flags with the same value now call set_external_kind(if is_external { ExternalRewritePath } else { NotExternal }), which sets the same bits. The sole reader in bundle_v2.rs is already inside if is_external(), and external_kind() keys on IS_EXTERNAL first (post-f44ee83), so it agrees with is_external() for every bit pattern. The six literal ResultFlags::IS_EXTERNAL constructions are unchanged and decode to External.
• BakeRouteKind: is_route was write-only (grepped — no readers anywhere), so dropping it is safe. fully_static was only written when is_route = true and only read via .bake_extra.fully_static in production.rs, now .route.is_fully_static().

Other factors

All bot feedback (comment-cop paragraph-length comments, CodeRabbit's accessor-comment and external-flag-invariant notes) is resolved and marked as such in the timeline. My own prior inline comment about 8f50cb5's commit message describing unstaged result.rs edits was answered — f44ee83 landed those hunks, and the current diff matches. The PR description lists the existing test files exercised locally; no new test is included, which is appropriate for a representation-only refactor with no input that could behave differently. Grepped the whole src/ tree for the removed identifiers and found none. The .is_route/.fully_static-looking hits in incremental_graph.rs are fields on an unrelated dev-server file struct, not BakeExtra.