Skip to content

js_parser: run an ambiguous file with a label-body or if-body function declaration as CommonJS - #44803

Draft
robobun wants to merge 4 commits into
robobun/d4353ade/annex-b-sloppy-block-fn-runtimefrom
robobun/e7f32f41/label-if-fn-decl-commonjs
Draft

robobun wants to merge 4 commits into
robobun/d4353ade/annex-b-sloppy-block-fn-runtimefrom
robobun/e7f32f41/label-if-fn-decl-commonjs

Conversation

@robobun

@robobun robobun commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #25737

Stacked on #44741.

Problem

Fix

  • The visit pass sets P::has_sloppy_only_syntax for either form in non-strict code, and the Unknown arm reads it. Runtime only, cache version 45.
  • Correct because strict code rejects both forms. The visit pass knows strictness exactly: a class body, a "use strict" scope and an import keep the flag off.
  • Verified: test/cli/run/commonjs-no-export.test.ts (20 new tests, 12 fail on js_parser: keep a sloppy block-level function as a declaration when no renamer runs #44741 alone). Self-reviewed: 12 concerns raised, 12 addressed.

Background

Downsides

Notes

The lines that change the behaviour

  • src/js_parser/p.rs: the field has_sloppy_only_syntax and the setter mark_sloppy_only_syntax. The setter writes only when options.features.commonjs_at_runtime is true and is_strict_mode() is false.
  • src/js_parser/visit/mod.rs (visit_single_stmt, the if has_if_scope block): the body of if and of else.
  • src/js_parser/visit/visit_stmt.rs (s_label, a new StmtData::SFunction arm): the body of a label.
  • src/js_parser/parse/parse_entry.rs: || p.has_sloppy_only_syntax, the last operand of the hint condition in the ModuleType::Unknown arm. The has_with_scope arm does not change, so import plus the form still loads.
  • src/jsc/RuntimeTranspilerCache.rs: EXPECTED_VERSION 44 to 45. An older entry stores the ESM kind. main has 43 and js_parser: keep a sloppy block-level function as a declaration when no renamer runs #44741 takes 44.

Against node v26.3.0

Files with no module syntax. Debug builds, transpiler cache off.

File node main #44741 alone this branch
index.js of #25737 (entry, require(), import) bar ReferenceError ReferenceError bar
if (true) function f(){} then f() function f ReferenceError ReferenceError function f
else function g(){} then g() function g ReferenceError ReferenceError function g
{ l: function h(){} } then h() h ReferenceError ReferenceError h
this in a plain call, file with the label form sloppy strict strict sloppy
if (true) { function k(){} } then typeof k function undefined undefined undefined
typeof bar before foo: function bar(){} function undefined undefined undefined
shapes 1 and 2 of the Notes of #44741, no CommonJS marker plain, V plain, V plain, V labelled, labelled

The route matrix (entry, .ts, require(), import, default import, -e, stdin, a bun test file, and the strict contexts): 13 rows change against #44741 alone and all 13 now equal node. 11 go from strict to sloppy and 2 go from ReferenceError to bar and function f. 18 rows do not change. 2 runs per binary.

What stays an ES module

Node rejects each of these files. bun runs them as ES modules, as on main.

  • The form next to import, export or top-level await.
  • The form under a "use strict" directive or in a class body.
  • The form in a file whose module type the loader knows: a .js or .ts file in a package with "type": "module", and a .mjs entry point.

The loader reports no module type for some files that have one in Node. There the form makes the file CommonJS, as __dirname and the other hints do on main. Measured: a .jsx or .tsx file and a file with no extension in a "type": "module" package (src/runtime/jsc_hooks.rs lets the file contents decide for them), and an imported .mjs with no package type. 6 of 20 rows of that matrix change. #40940 is open for the loader. One test pins that the form and __dirname give the same kind for these files.

Sources that stop loading (7 probes: 7 run on main and on #44741 alone, 0 run on this branch)

What a flipped file gets from the CommonJS path

Measurements (release builds of main at 5749c31 and of this change on it, taken before the restack. The code lines are the same. The comments are shorter and the cache version is 45.)

  • release binary .text: +0 B by size -A (65,618,773 B in both). The sum of text symbol sizes is +89 B by nm -S: _parse x2 +47 / +20 B, visit_single_stmt x2 +31 / +29 B, s_label x2 +36 / +40 B, P::init x5 -15 B each, parser_snapshot -39 B. bloaty is not installed in the container.
  • per parse that reaches the ESM default: +5 instructions in the JavaScript copy of _parse (2 for the new operand, 3 register reloads that the compiler duplicated), +2 in the TypeScript copy. Per label whose body is not a loop: +2 instructions. Every other path: +0 (objdump).
  • sizeof(P): 4288 -> 4288 bytes (5 instantiations). ParserSnapshot: 568 -> 568 bytes. Allocations added: 0 (DWARF, debug builds).
  • 6,000-file npm corpus (278 packages, 52.6 MB, each file required once, none evaluated): the new operand ran 9 times and the flag was written 0 times (gdb hit counts, debug build).
  • transpiler cache: 1,481 of 1,481 entries re-written on the first run after the upgrade, 1 on the second (the base build also writes 1 per run). main has the version of bun-v1.4.4 (43). js_parser: keep a sloppy block-level function as a declaration when no renamer runs #44741 adds 44 and this change adds 45.

Not in this change

Tests

  • 20 new tests. On js_parser: keep a sloppy block-level function as a declaration when no renamer runs #44741 alone 12 fail and 8 pass: the 5 that pin what stays an ES module or unbundled, and the 3 test.failing rows.
  • Mutants, each a real build. No commonjs_at_runtime gate: the bun build test fails. No strictness test: the strict-context test fails. Dead branches skipped: the dead-branch row fails. The setter called for every label body and every single-statement body: the test for files without the two forms fails.
  • The tests are serial. With 20 debug processes at once, single tests passed the 5 second limit after a fresh link.
  • On this branch (debug build): commonjs-no-export.test.ts 21 pass, runtime-transpiler.test.ts 15 pass and 1 skip, preserve-use-strict-cjs.test.ts 18 pass, transpiler.test.js 272 pass, repl-transform.test.ts 38 pass.
  • Before the restack, with a release build of the change on main: test/bundler/transpiler/, test/cli/run/, test/js/bun/resolve/, test/js/node/module/, test/js/bun/plugin/. The only failures also fail on the base build: the 2 GiB printer test, 2 FUSE tests and 2 require-cache timeouts.

[human-review] gate passed · iteration 1 · 6 files touched

fails on main (without fix)
ASAN without fix: 12 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/cli/run/commonjs-no-export.test.ts
bun test v1.4.4 (367d939d9)

test/cli/run/commonjs-no-export.test.ts:
(pass) commonjs-no-export > CommonJS entry point with no exports [480.92ms]
60 |       "dead-branch.js": `if (false) function g() {}\n` + named("dead branch", "typeof g"),
61 |       "typescript.ts": label + named(".ts", "g()"),
62 |       "no-extension": label + named("no extension", "g()"),
63 |     };
64 |     using dir = tempDir("sloppy-fn-decl-forms", { ...forms, "main.cjs": requireAll(forms) });
65 |     expect(await spawn(String(dir), ["main.cjs"])).toEqual({
                                                        ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
-   "stderr": "",
-   "stdout": 
- "label: sloppy g
- if: sloppy g
- else: sloppy g
- nested label: sloppy g
- label in a block: sloppy g
- in a function: sloppy g
- in an arrow function: sloppy g
- dead branch: sloppy undefined
- .ts: sloppy g
- no extension: sloppy g
+   "exitCode": 1,
+   "stderr": 
+ "1 | l: function g() { return "g"; }
+ 2 
... (truncated)

release without fix: 12 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/cli/run/commonjs-no-export.test.ts:
(pass) commonjs-no-export > CommonJS entry point with no exports [24.13ms]
60 |       "dead-branch.js": `if (false) function g() {}\n` + named("dead branch", "typeof g"),
61 |       "typescript.ts": label + named(".ts", "g()"),
62 |       "no-extension": label + named("no extension", "g()"),
63 |     };
64 |     using dir = tempDir("sloppy-fn-decl-forms", { ...forms, "main.cjs": requireAll(forms) });
65 |     expect(await spawn(String(dir), ["main.cjs"])).toEqual({
                                                        ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
-   "stderr": "",
-   "stdout": 
- "label: sloppy g
- if: sloppy g
- else: sloppy g
- nested label: sloppy g
- label in a block: sloppy g
- in a function: sloppy g
- in an arrow function: sloppy g
- dead branch: sloppy undefined
- .ts: sloppy g
- no extension: sloppy g
+   "exitCode": 1,
+   "stderr": 
+ "1 | l: function g() { return "g"; }
+ 2 | console.log("label:", (function () { return this === undefined; })() ? "strict" : "sloppy", g());
+                                                               
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/cli/run/commonjs-no-export.test.ts
bun test v1.4.4 (367d939d9)

test/cli/run/commonjs-no-export.test.ts:
(pass) commonjs-no-export > CommonJS entry point with no exports [409.55ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS in each form [427.43ms]
(pass) a function declaration as the body of a label, if or else > leaves the file an ES module without the two forms [370.75ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS: the file of the issue as the entry point [229.09ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS: the entry point [249.28ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS: an import from an ES module [292.96ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS: -e [295.45ms]
(pass) a function declaration as the body of a label, if or else > makes the file CommonJS: -p [260.73ms]
(pass) a functio
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 3130ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/29] gen generated_host_exports.rs
generated_host_exports.rs: 103 exports (host=5, lazy=4, generic=94, rust=0); 247 extern-C blocks audited
[2/28] rustc bun_ini 
[3/28] rustc bun_js_parser 
[4/28] rustc bun_resolver 
[5/28] rustc bun_router 
[6/28] rustc bun_bundler 
[7/28] rustc bun_transpiler 
[8/28] rustc bun_standalone_graph 
[9/28] rustc bun_bunfig 
[10/28] rustc bun_install 
[11/28] rustc bun_jsc 
[12/28] rustc bun_ast_jsc 
[13/28] rustc bun_patch_jsc 
[14/28] rustc bun_semver_jsc 
[15/28] rustc bun_sys_jsc 
[16/28] rustc bun_bundler_jsc 
[17/28] rustc bun_sourcemap_jsc 
[18/28] rustc bun_css_jsc 
[19/28] rustc bun_js_parser_jsc 
[20/28] rustc bun_http_jsc 
[21/28] rustc bun_install_jsc 
[22/28] rustc bun_sql_jsc 
[23/28] rustc bun_runtime 
[24/28] link bun-profile
ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_lint_react-8f596b753a9d2596.rlib(bun_lint_react
... (truncated)
diff hotspot
src/js_parser/p.rs                      |  12 ++
 src/js_parser/parse/parse_entry.rs      |   2 +
 src/js_parser/visit/mod.rs              |   1 +
 src/js_parser/visit/visit_stmt.rs       |   1 +
 src/jsc/RuntimeTranspilerCache.rs       |   3 +-
 test/cli/run/commonjs-no-export.test.ts | 308 +++++++++++++++++++++++++++++++-
 6 files changed, 325 insertions(+), 2 deletions(-)

gate history · 3 passed · 1 rejected · iteration 1

evidence per changed file
file                                     reads  edits  tests
src/js_parser/p.rs                           3      2     15
src/js_parser/parse/parse_entry.rs           3      1     12
src/js_parser/visit/mod.rs                   1      1     12
src/js_parser/visit/visit_stmt.rs            1      1     11
src/jsc/RuntimeTranspilerCache.rs            3      3     14
test/cli/run/commonjs-no-export.test.ts      1      0     10

@github-actions github-actions Bot added the claude label Oct 8, 2026
@robobun

robobun commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: draft, stacked on #44741.

Reproduction (index.js of #25737, no package.json, node v26.3.0 and bun main 5749c31):

foo:
    function bar() { return "bar"; }

console.log(bar()); // node: bar, bun: ReferenceError: bar is not defined

@robobun
robobun force-pushed the robobun/e7f32f41/label-if-fn-decl-commonjs branch from 1c139c3 to 66d4ff7 Compare October 14, 2026 03:25
@robobun

robobun commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:04 AM PT - Oct 23rd, 2026

✅ @robobun, your commit 98b6f2f4166dc7a5d0b83a3d1abe5352743eddf7 passed in Build #131167! 🎉


🧪   To try this PR locally:

bunx bun-pr 44803

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

bun-44803 --bun

…block-fn-runtime' into robobun/e7f32f41/label-if-fn-decl-commonjs

# Conflicts:
#	src/jsc/RuntimeTranspilerCache.rs
…block-fn-runtime' into robobun/e7f32f41/label-if-fn-decl-commonjs

# Conflicts:
#	src/jsc/RuntimeTranspilerCache.rs

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant