parser: .mjs/"type":"module" is authoritative over module/exports identifier refs

[robobun]

Problem

A bare identifier reference to module or exports (for example the common dual-mode guard typeof module !== "undefined") makes Bun's parser classify a file as CommonJS even when the .mjs/.mts extension or package.json "type":"module" already says it is ESM.

Two observable faces:

• If the file also has an import statement (valid ESM, Node runs it), Bun refuses to load it with Cannot use import statement with CommonJS-only features / note: This file is CommonJS because 'module' was used.
• If the file has no import/export, it silently runs with full CJS semantics: typeof module === "object", top-level this is the exports object.

// guard.mjs
import path from "node:path";
console.log(typeof module !== "undefined" ? "cjs" : "esm", path.sep);

Node prints esm /; Bun errors.

Cause

In src/js_parser/parse/parse_entry.rs, the exports_kind decision runs the CJS content sniff (uses_exports_ref || uses_module_ref || ...) before ever consulting options.module_type. Only an export keyword or top-level await out-ranks the sniff; the extension and "type" field do not.

Fix

• parse_entry.rs: treat options.module_type == Esm the same as export/top-level await when choosing exports_kind, on the runtime path only (!p.options.bundle). The bundler's resolver can return Esm for files that are actually CJS (a nameless nested {"type":"commonjs"} scope that enclosing_package_json skips, or an exports "import" condition pointing at a .js file), so the content sniff is still needed there; the runtime path is fed the correct value by the change below.
• p.rs value_for_this: substitute the ESM value for top-level this when module_type == Esm (not only when ESM syntax is present), so a bare .mjs with top-level this does not rewrite to exports.
• RuntimeTranspilerStore: thread the caller's extension-aware module_type through transpile() onto TranspilerJob and use it in the worker, instead of re-deriving from the package.json tag. The resolved_source.tag (used for ignoreESModuleAnnotation) is left as-is.
• jsc_hooks.rs: derive the runtime module_type for .js/.ts from the nearest package.json's "type" by walking DirInfo parents, including nameless ones that enclosing_package_json skips. Without this a .js file under parse5/dist/cjs/ (which ships {"type":"commonjs"} with no name) was seeing the outer parse5 "type":"module". Carried as a separate LoaderResult field so pkg_name semantics are unchanged.
• resolver.rs resolve_with_framework: stop hard-coding ModuleType::Esm for embedded BuiltInModule::Code; the react-refresh fallback it carries is CJS. Use Unknown so the parser decides from content; the ESM .tsx built-ins still classify as ESM via their import syntax.
• Cache: hash module_type into the runtime transpiler features hash (byte-identical source now transpiles differently under different module types) and bump EXPECTED_VERSION to 23 so stale entries with the old classification are invalidated.

The es-module-lexer test fixture declared "type":"module" while its index.ts is CJS and relied on the sniff override; the stray "type" is dropped.

Tests

test/cli/run/run-cjs.test.ts gains a describe block covering .mjs, .mts, "type":"module" .js, with and without import statements, loaded directly and via dynamic import(), plus negative controls (.cjs, ambiguous .js, .cjs inside "type":"module", .mjs inside "type":"commonjs", and a nested nameless {"type":"commonjs"} under a "type":"module" root). 7 of the 11 new tests fail on the unfixed build and all pass with this change.

test/bundler/bundler_edgecase.test.ts adds NamelessNestedTypeCommonjsUnderTypeModule and ExportsMapImportConditionPointsAtCjs to lock in the bundler keeping its content-sniff fallback for those layouts.

Also ran: test/js/bun/resolve/, test/bundler/bundler_cjs*.test.ts, test/bundler/bundler_edgecase.test.ts, test/bundler/esbuild/default.test.ts, test/bundler/transpiler/, test/cli/run/transpiler-cache.test.ts, test/integration/jsdom/, test/bake/dev/react-spa.test.ts, test/bake/dev/esm.test.ts with no new failures.

Related: #32173 independently makes the same value_for_this change (plus changing null to undefined there) and the same cache-key addition; whichever lands second will have a small conflict in those two spots.

---

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

[robobun]

^{Updated 6:38 PM PT - Jul 11th, 2026}

❌ @robobun, your commit 23c6fcf has 2 failures in Build #71988 (All Failures):

• test/integration/next-pages/test/dev-server-ssr-100.test.ts - crash reported on 🍎 14 aarch64
• test/napi/napi.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 33807

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

bun-33807 --bun

[github-actions]

Found 1 issue this PR may fix:

1. pyodide.mjs throws Cannot determine runtime environment when run under Bun, even though it works in Node #28864 - pyodide.mjs contains typeof module / typeof module.exports dual-mode guard patterns; Bun's content-sniffing misclassifies the .mjs file as CJS, causing the environment detection to take the wrong branch and throw "Cannot determine runtime environment"

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

Fixes #28864

🤖 Generated with Claude Code

[robobun]

Checked #28864: the pyodide failure there is caused by process.browser being defined (as false) in Bun, which makes pyodide's typeof process.browser > "u" check fail and fall through to the browser branch. The file has export statements so it was already classified as ESM; typeof module is "undefined" there both before and after this change. Not adding the Fixes line.

[github-actions]

This PR may be a duplicate of:

1. Substitute top-level this with undefined in ES modules #32173 - Also fixes value_for_this and exports_kind logic in the parser to respect module_type from .mjs/"type":"module", touching the same code paths in p.rs and parse_entry.rs
2. Apply implicit strict mode to files forced to ESM by .mjs or package.json type module #32177 - Fixes the same authoritative ESM signals (.mjs/"type":"module") not being respected by the parser, adding strict mode enforcement based on the same module_type signals
3. fix: handle import statements in CJS files (#20718) #30367 - Changes exports_kind classification in parse_entry.rs when CJS and ESM signals coexist, addressing the same "Cannot use import statement with CommonJS-only features" error
4. bundler: let DCE drop unreachable top-level await before rejecting CJS #29246 - Modifies the same exports_kind CJS-vs-ESM decision logic in parse_entry.rs, specifically around top-level await interaction with CJS classification

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

This PR makes explicit module type authoritative across loader resolution, runtime transpilation, parser ESM handling, cache hashing, and tests covering extension and package.json type interactions.

Changes

module_type authoritative propagation

Layer / File(s)	Summary
Loader derives nearest module type src/resolver/resolver.rs, src/runtime/jsc_hooks.rs	Embedded built-in code is resolved with module_type = Unknown, and loader resolution computes the nearest package-json-derived module type for transpilation.
Transpiler jobs and cache use module_type src/jsc/RuntimeTranspilerStore.rs, src/jsc/RuntimeTranspilerCache.rs, src/js_parser/parse/parse_entry.rs	Transpiler jobs persist module_type, parse options read it directly, and cache hashing and versioning account for module type.
Parser treats module_type as ESM signal src/js_parser/p.rs, src/js_parser/parse/parse_entry.rs	this substitution and initial ESM export-kind selection recognize explicit ESM module type when not bundling.
Module type behavior tests test/cli/run/run-cjs.test.ts, test/bundler/bundler_edgecase.test.ts, test/js/third_party/es-module-lexer/package.json	Tests cover .mjs, .mts, .js, .cjs, dynamic imports, package type settings, and exports-map resolution; the fixture no longer declares "type": "module".

🚥 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 is concise and accurately summarizes the main change about ESM taking precedence over module and exports refs.
Description check	✅ Passed	It explains the problem, fix, and verification steps, covering the template's required information even with different headings.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@test/cli/run/run-cjs.test.ts`:
- Around line 129-152: The last CJS sniff tests use inconsistent failure
handling, so align them with the earlier tests by capturing stderr and guarding
JSON parsing. Update the "ambiguous .js with typeof module still runs as CJS"
and ".cjs with typeof module still runs as CJS" cases in run-cjs.test.ts to
destructure stderr from run() and parse stdout with a safe fallback like the
other tests, so failures report exitCode/stderr instead of an opaque
SyntaxError. Use the existing tempDir, run, and JSON.parse(stdout || "null")
pattern as the reference for consistency.

🪄 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: cd246ccb-c320-4bdb-9021-be93f8d1e8ad

📥 Commits

Reviewing files that changed from the base of the PR and between b05b4fa and ecf8749.

📒 Files selected for processing (6)

• src/js_parser/p.rs
• src/js_parser/parse/parse_entry.rs
• src/jsc/RuntimeTranspilerCache.rs
• src/jsc/RuntimeTranspilerStore.rs
• src/runtime/jsc_hooks.rs
• test/cli/run/run-cjs.test.ts

[robobun]

On the dedup suggestions:

• Substitute top-level this with undefined in ES modules #32173 overlaps on the value_for_this hunk and the cache-version bump (already noted in the PR body). It does not touch the exports_kind branch in parse_entry.rs or the RuntimeTranspilerStore path, which is where the reported load failure lives. Not a duplicate; whichever lands second resolves a small conflict in those two spots.
• Apply implicit strict mode to files forced to ESM by .mjs or package.json type module #32177 adds strict-mode marking for module_type == Esm and explicitly preserves the CJS-interop fallback for forced-ESM files that reference module/exports/with. This PR removes that fallback (the explicit type now wins, matching Node). The two disagree on that design point; they can coexist code-wise since Apply implicit strict mode to files forced to ESM by .mjs or package.json type module #32177 gates its deferred strict errors on the final exports_kind, which this PR changes to Esm for these files, so the errors would fire. The has_top_level_return fix in Apply implicit strict mode to files forced to ESM by .mjs or package.json type module #32177 is orthogonal.
• fix: handle import statements in CJS files (#20718) #30367 rewrites import statements to require() when CJS features are present. That targets ambiguous .js files mixing both; this PR targets files whose type is already explicit. Different scope, same error message.
• bundler: let DCE drop unreachable top-level await before rejecting CJS #29246 is about DCE of unreachable top-level await in bun build --format=cjs. Unrelated.

[coderabbitai]

Actionable comments posted: 1

🤖 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/p.rs`:
- Around line 5613-5615: In `value_for_this`, explicit ESM should take
precedence over `commonjs_named_exports`, since that set can still be populated
in `.mjs`/`.mts` or `"type":"module"` files when `unwrap_commonjs_to_esm` is
enabled. Update the conditional so `self.options.module_type ==
options::ModuleType::Esm` bypasses the `commonjs_named_exports.count() == 0`
gate, and only use that count check for inferred ESM cases based on
`self.has_es_module_syntax`. Keep the top-level `this` rewrite to `exports` only
for non-explicit ESM scenarios.

🪄 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: c35b7c15-2b53-48f5-9ce8-53ab8dbb19d3

📥 Commits

Reviewing files that changed from the base of the PR and between ecf8749 and 636faf7.

📒 Files selected for processing (2)

• src/js_parser/p.rs
• test/cli/run/run-cjs.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@test/cli/run/run-cjs.test.ts`:
- Around line 38-46: The parsing in the run-cjs test helper can throw on
malformed non-empty stdout, preventing crash diagnostics from being surfaced.
Update the stdout handling around the `out` निर्माण in the helper used by
`run-cjs.test.ts` so that JSON parsing failures are caught and fall back to the
same `{ crashed: stderr }` shape used for empty stdout. Keep the fix localized
to the helper that builds `out`/`exitCode`, ensuring partial or invalid lines do
not leak a raw `SyntaxError`.

🪄 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: cef22b14-f0ca-4f93-a26a-1ba3c06970cd

📥 Commits

Reviewing files that changed from the base of the PR and between 636faf7 and d92882b.

📒 Files selected for processing (6)

• src/js_parser/parse/parse_entry.rs
• src/resolver/resolver.rs
• src/runtime/jsc_hooks.rs
• test/bundler/bundler_edgecase.test.ts
• test/cli/run/run-cjs.test.ts
• test/js/third_party/es-module-lexer/package.json

💤 Files with no reviewable changes (1)

• test/js/third_party/es-module-lexer/package.json

[claude]

Thanks for addressing all three rounds of feedback — the !bundle gates and the bundler regression tests look right. I'm not auto-approving because this changes core CJS/ESM classification semantics across parser/resolver/runtime and has a stated design disagreement with #32177, so a maintainer should sign off on the direction.

Extended reasoning...

Overview

This PR makes .mjs/.mts/"type":"module" authoritative over the parser's CJS content sniff (bare module/exports identifier references). It touches 6 source files across four subsystems: the parser's exports_kind decision and value_for_this rewrite (parse_entry.rs, p.rs), the runtime transpiler store's module-type threading (RuntimeTranspilerStore.rs), the runtime loader's nearest-package.json walk (jsc_hooks.rs), the resolver's built-in-module classification (resolver.rs), and the transpiler cache's version bump + hash key (RuntimeTranspilerCache.rs, parse_entry.rs). Test coverage is thorough: 11 runtime cases in run-cjs.test.ts and 2 bundler regression cases in bundler_edgecase.test.ts.

Security risks

None identified. This is module-classification logic with no auth, crypto, network, or untrusted-input parsing surface. The nearest-package.json walk reuses the existing resolver's DirInfo cache and does not introduce new filesystem traversal.

Level of scrutiny

High. Module classification (CJS vs ESM) is one of the most semantically load-bearing decisions in the runtime — getting it wrong silently changes this, module, exports, strict mode, and import/require interop for user code. The PR already went through three rounds of review-driven fixes here (a real bundler regression was caught and gated on !bundle), which is evidence the interaction surface is subtle. The change is well-reasoned and well-tested, but it is not mechanical.

Other factors

• The PR description and the author's own dedup analysis note that this overlaps with #32173 (same value_for_this + cache-key change) and disagrees on a design point with #32177 (whether explicit ESM should override the CJS-interop fallback for files that reference module/exports). That's a direction call a maintainer should make, not a bot.
• The resolver.rs change (built-in module_type: Esm → Unknown) and the es-module-lexer fixture change are small but each shift existing behavior; they're justified in the description but warrant a human glance.
• All prior inline feedback (mine and CodeRabbit's) is resolved; the bug hunter found nothing on 7408ef4.

[robobun]

CI on 7408ef4 is red on tests that are failing across multiple unrelated PR builds right now (napi.test.ts on builds 70852-70862, update_interactive_install.test.ts on 70853/70857/70858/70862, bun-install.test.ts on 70856, complex-workspace.test.ts on 70853/70858, postgres-invalid-message-length on 70852-70858, express/res.send on 70853-70858). The one bundler_compile.test.ts failure is a bytecode disk-cache hit/miss ordering diff on a single lane, unrelated to module classification.

The diff itself is green: the new runtime tests in run-cjs.test.ts (7 fail before, 11 pass after) and the new bundler_edgecase tests (NamelessNestedTypeCommonjsUnderTypeModule, ExportsMapImportConditionPointsAtCjs) pass locally, as do jsdom, react-spa, es-module-lexer, bundler_cjs*, esbuild/default, and transpiler/ with no new failures. Ready for review.

[robobun]

Ran into the mirror of this from the .cjs side: a .cjs with no module/exports/require markers (e.g. comment-only, or a conditional that never assigns) is content-sniffed as an export-less ES module when imported from .mjs, so import D from './b.cjs' throws Missing 'default' export. The RuntimeTranspilerStore change here already fixes that by threading module_type through to the worker, so no extra src/ change is needed for it.

One case this PR doesn't yet cover: an empty (or whitespace-only) file declared as CommonJS. parse_maybe_return_file_only short-circuits before the parser runs and returns ParseResult::empty_with(...) with exports_kind = None, so the new module_type never reaches the parser. The sync path has a special case in jsc_hooks.rs that returns a synthetic (function(){}) for .cjs/.cts only; the async path has nothing, and neither covers .js under "type":"commonjs".

Pushed claude/farm/4a8127f4/fix-cjs-extension-async-transpiler with:

• the same TranspilerJob.module_type threading as here
• the empty-file check in RuntimeTranspilerStore::run_impl and a widened one in jsc_hooks.rs (keyed on module_type == Cjs rather than a literal extension match)
• tests for marker-less .cjs/.cts imported from .mjs/.mts across comment-only / empty / side-effect-only content, .js under "type":"commonjs", and "use strict" in .mjs

Feel free to cherry-pick the empty-file bits and tests if useful; happy to rebase on top of this once it lands otherwise.

[robobun]

CI on a46baf7 (build 71980, post-rebase): the three failing tests (30205.test.ts napi-finalizer SIGABRT, fetch-backpressure, bake/dev-and-prod) are failing on other PR builds 71972/71976/71977/71978/71979 as well; none touch module classification. Diff remains green on all new runtime and bundler tests locally.

[claude]

No new issues after 23c6fcf — the four concerns I raised earlier are all addressed. Deferring to a human reviewer because this changes core module-classification semantics, has stated design overlap with #32173/#32177/#30367, and the two !bundle gates are an intentional workaround pending a resolver-side finalize_result fix; those are design calls worth a maintainer's sign-off.

Checked this pass:

• nearest_module_type walk now breaks at is_node_modules() before consulting package_json(), matching Node's LOOKUP_PACKAGE_SCOPE; new runtime test covers it.
• Both !bundle gates (parse_entry.rs exports_kind, p.rs value_for_this) are consistent; the two bundler-resolver misclassification layouts are locked in by itBundled tests.
• module_type is threaded through TranspilerJob, hashed into the features hash, and EXPECTED_VERSION bumped to 23.
• resolve_with_framework → ModuleType::Unknown for embedded code — the ESM .tsx built-ins still classify via their import syntax.

Extended reasoning...

Overview

The PR makes .mjs/.mts/"type":"module" authoritative over the parser's CJS content sniff (bare module/exports references). It touches: parse_entry.rs (the exports_kind decision + hashing module_type), p.rs (value_for_this top-level this substitution), RuntimeTranspilerStore.rs (thread caller-derived module_type onto TranspilerJob and use it in the worker), jsc_hooks.rs (new nearest_module_type walk over DirInfo parents including nameless package.json, stopping at node_modules), resolver.rs (resolve_with_framework embedded code → Unknown), RuntimeTranspilerCache.rs (version 23), plus 12 new runtime tests, 2 new bundler tests, and dropping a stray "type":"module" from the es-module-lexer fixture.

Security risks

None identified. This is module-format classification; no auth, crypto, network, or untrusted-input parsing is introduced. The nearest_module_type walk reads already-cached DirInfo entries via read_dir_info.

Level of scrutiny

High. Module classification governs whether a file is wrapped as CJS or run as ESM — misclassification breaks real packages (the PR itself found and fixed three such regressions during review: nameless nested {"type":"commonjs"}, exports-map "import" condition, and the node_modules boundary). The runtime path now trusts module_type == Esm unconditionally, which is only correct because nearest_module_type derives it per Node's rule; any remaining divergence there is a runtime regression. The bundler path is deliberately kept on the old content-sniff via !bundle gates as a workaround.

Other factors

• Design overlap: the author's own dedup analysis notes #32177 explicitly preserves the CJS-interop fallback for forced-ESM files that reference module/exports, while this PR removes it. #32173 makes overlapping changes to value_for_this and the cache key. Whichever direction is chosen, a maintainer should reconcile these.
• Workaround shape: the !bundle gates are documented as temporary until finalize_result is fixed to re-derive module_type from extension + nearest package.json. That's a reasonable scoping choice, but it leaves the parser's behavior forked between runtime and bundler on the same input — worth explicit maintainer acknowledgment.
• Companion branch: the author mentioned a separate branch handling the .cjs mirror (empty/marker-less CJS files) that isn't folded in here.
• Test coverage: the new tests are thorough (extension × package.json × direct/dynamic-import × nested scopes × node_modules boundary, plus negative controls). All four issues I raised during earlier passes are addressed with tests.

Given the critical code path, the intentional runtime/bundler behavioral fork, and the open design question vs. #32177, this warrants human review rather than bot approval.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-12, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.