Repository navigation
Conversation
…cjs and iife output The linker printed the directive of a wrapped file inside its `__commonJS` or `__esm` wrapper only when the chunk was not an entry point chunk. Code splitting needs esm output, so every cjs and iife chunk is an entry point chunk and the directive was never printed. A CommonJS file that starts with "use strict" ran as sloppy code after bundling. The gate now tests the file: every wrapped file except the entry point of the chunk gets the directive, and that entry point keeps it at the top of the chunk. `InsideWrapperPrefix::append_directive` keeps the directive ahead of the `init_*()` calls the linker inserts at the start of the wrapper body. An ES module that has no wrapper symbol prints no closure, so it gets no directive.
… --compile --bytecode Six of the bundles in bundler_bytecode_portable.test.ts hold wrapped CommonJS files that start with "use strict". Their text and bytecode move. The other entries are unchanged.
…ach file kind An ES module with "use strict" and no wrapper gets no directive. An __esm closure can hold the directive alone. A read of fn.caller throws again. One bundle with each kind of file records which ones run strict in cjs and iife output.
|
Updated 5:18 AM PT - Oct 23rd, 2026
✅ @robobun, your commit eb3afaa15d45d86ffd56b731d986377ec7c7857d passed in 🧪 To try this PR locally: bunx bun-pr 46578That installs a local version of the PR into your bun-46578 --bun |
|
Status: ready for review. Reproduced on bun 1.4.3 and on main 5c6d4e2. A CommonJS package that starts with PR: #46578 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe linker now inserts explicit ChangesWrapped directive insertion
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked whether the debug_assert! in append_directive (src/bundler/LinkerContext.rs:5220) leaves release builds exposed to a directive landing after init_*() calls: its only call site (generateCodeForFileInChunkJS.rs:268) runs directly after stmts.reset() at line 240, so the prefix is empty and sync_dependencies_end is 0 regardless of build mode. The !ast.wrapper_ref.is_empty() clause matches the existing checks at lines 859 and 923 that decide whether an __esm closure is printed at all.
Extended reasoning...
The change is a 2-file linker edit (a gate rewrite from chunk-level to file-level is_entry_point_file, a wrapper-symbol check, and a new append_directive that advances the sync-dependency cursor) plus ~380 lines of bundler tests and bytecode-portability hash updates; it touches no security-sensitive surface. One confirmed finding was posted inline (a pre-existing sloppy-mode gap for hoisted function expressions in required ES modules), so a human should weigh whether it belongs in this PR, and the bytecode hash updates certify new output a reviewer should sanity-check.
|
#41601 changed at The |
Problem
bun build --format=cjsand--format=iife(also--bytecode) drop the"use strict"of a bundled CommonJS file, so it runs as sloppy code. The repro in Notes printsSLOPPY | SLOPPY: no throw. node and esbuild printstrict | strict: TypeError.generateCodeForFileInChunkJS.rs:263printed the directive only in a chunk that is not an entry point chunk. Without code splitting no such chunk exists.Fix
Chunk::is_entry_point_file), as the gate of esbuild does. A wrapped file prints its directive first in its wrapper. The entry point of a chunk keeps it at the chunk top.append_directivekeeps the directive ahead of theinit_*()calls that the linker inserts. An ES module with no wrapper symbol prints no closure, so the gate skips it.test/bundler/bundler_cjs.test.ts(13 fail on 1.4.3) and one inbundler_compile.test.ts. Notes list the other suites.Background
__commonJS(function(exports, module) { ... }), or in__esm(() => { ... })for arequire()d ES module. A directive works only as the first statement of a function body.Downsides
"use strict"file: +1,153 bytes (0.19%) on undici. esm output is byte-identical."use strict"file withwithordelete namenow makes the whole chunk a SyntaxError at load, and the build reports nothing (parser: report strict-mode early errors and accept legacy sloppy syntax #40827). 0 of 67 bundled packages changed behavior.bun run: a stack trace throughreturn f()calls loses frames.Notes
Repro
bun build ./e.mjs --format=cjs --target=node --outdir o && node o/e.js--format=cjsSLOPPY | SLOPPY: no throwstrict | strict: TypeError--format=iifeSLOPPY | SLOPPY: no throwstrict | strict: TypeError--format=cjs --minifySLOPPY | SLOPPY: no throwstrict | strict: TypeErrorrequire("ext"), cjsSLOPPY | SLOPPY: no throwstrict | strict: TypeError--format=cjs --target=bunSLOPPY | SLOPPY: no throwstrict | strict: TypeError--format=esmstrict | strict: TypeErrorAfter the change the wrapper reads
var require_ext = __commonJS(function(exports2) { "use strict"; ... }). The same holds forfn.callerandfn.argumentsof a function in such a file: node throws a TypeError for the read, the bundle from main returns the caller, and the bundle from this branch throws again.When bun runs the output, this change shows only in CommonJS output for bun:
--target=bun --format=cjs,--bytecode, and--compilewith either. bun decides the mode of any other bundle as a whole when it loads it. As an ES module each file is strict. This is the case forbun o/e.jsabove, on main too, because the file has no top-levelmodule,exportsorrequire. As CommonJS each file is sloppy (the same bytes ase.cjs, or under"type": "commonjs"), because bun's parser drops a"use strict"inside a function body (#29533, #31807). A bundle that bun bundles again loses the directive for the same reason. The new tests run the node-target bundles with node for this reason.Reference
esbuild decides at the same place in its linker:
if repr.Meta.Wrap != graph.WrapNone && !file.IsEntryPoint()(internal/linker/linker.go:4654-4662in v0.21.5,:4680-4688on main). The comment above bun's gate is a copy of the comment above that line. bun's copy, from #14232, tested the chunk. "As esbuild does" in this description means that test, not the whole output: the table below has the differences.bun needs the test per chunk and not per file. Without code splitting each entry point has its own chunk (#41263), and a file that is an entry point can also be a wrapped dependency in the chunk of another entry point.
Chunk::is_entry_point_filealready makes the two neighbouring decisions (convertStmtsForChunk.rs:72and a few lines below the gate).The clauses of the gate, and the case that fails without each
bundler_cjs.test.tsandcompile/BytecodeKeepsUseStrictOfWrappedCommonJSfail on main.append_directive: the linker inserts theinit_*()call of a wrapped import at a cursor that starts at index 0. A plain push of the directive leaves it after those calls, where it is a string statement. Without the cursor step, the twocjs/UseStrictBeforeInitCalls*cases fail.!ast.wrapper_ref.is_empty(): the parser makes no wrapper symbol for an ES module whose statements all hoist out of the__esmclosure (needs_wrapper_refinsrc/js_parser/p.rs). With a directive pushed for such a file the linker prints a closure that has no name. A debug build failsdebug_assert!(!ast.wrapper_ref.is_empty())in the__esmarm of the same function.cjs/UseStrictEsmFileWithoutWrapperSymbolaborts there when the check is removed.flags.wrap != WrapKind::None: without it, a strict ES module that has no wrapper gets a stray directive in the middle of the chunk. The three cases that count the directives fail (target_bun,target_bun_bytecode,minify).!output_format.is_always_strict_mode()is unchanged.cjs/UseStrictInCommonJSWrapper_format_esmpins it.What runs strict in cjs and iife output
One bundle with each kind of file, run by node.
cjs/UseStrictModeTable_format_cjsand_iifepin the first ten rows. The other four rows have no directive of the file's own, or are decided by the parser, so this gate does not read them. The self-review asked for a pin of all fourteen. A pin of those four would record behavior that #40926 and #40838 change.importof a CommonJS file with"use strict"importof a CommonJS file with no directiveimportof an ES module with"use strict": bodyimportof a"use strict"script with no module syntaxrequire()of a CommonJS file with"use strict"require()of an ES module with"use strict": bodyrequire()of a"use strict"script with no module syntax: bodyimportof an ES module with no directive: bodyrequire()of an ES module with no directive: body"use strict"in a CommonJS fileCases that stay sloppy
"use strict"(UMD wrappers, theisES5test of bluebird)export const f = function () {}or= () => {}in arequire()d"use strict"ES module: bun hoists the initializer out of the__esmclosurevarstores back into the wrapper and leavesconstvalues outsideexport function f() {}in the same modulerequire()d"use strict"script with no module syntax: the bundler wraps that file in__esmand hoists the function__esmalwaysStrictorstrict: bun has no such input// @bunpragma that bun loads as CommonJSSites that this change leaves as they are:
internal_bake_dev): its branch returns before the gate."use client"(bun builddoesn't hoist"use strict","use client","use server", etc. directives #6854, js_parser: keep "use strict" and the directive prologue in the output #40838).ParserOptions::keep_directivesis on main and onlybun pm diffsets it. Turning it on for the bundler changes what the parser prints for each file. That is the function-level case above, not this gate.__esmclosure can now hold the directive alone:var init_lib = __esm(() => { "use strict"; })where main printsvar init_lib = () => {}(+24 bytes, esbuild prints such a closure too).cjs/UseStrictAloneInEsmWrapperpins it.Syntax that strict mode forbids
legacy.cjsis"use strict"; with ({ a: 1 }) { module.exports = a; }. The entry requires it only insidetry { if (process.env.LOAD_LEGACY) ... }.LOAD_LEGACY=1app ran nullapp ran caught SyntaxErrorapp ran nullapp ran 1bun buildexits 0 and prints nothing in the two builds (#40827, #40861). With--bytecodethe build from this branch writesentry.jsand noentry.js.jsc, and exits 0 (#36708).delete nameand a strict reserved word as a label behave likewith. A legacy octal literal does not: the printer writes010as8.Stack traces under bun
JavaScriptCore makes tail calls only in strict code.
dep.cjsstarts with"use strict"and hasruncalla,acallb,bcallc, each asreturn f().cthrows.c < b < a < run < ...bun entry.cjson the sourcec < <anonymous>--target=bun --format=cjsor--bytecodefrom main, run by bunc < b < a < run < <anonymous>c < <anonymous>--target=bun --format=esm, main and this branchc < <anonymous> < ...Measurements
Release builds of the merge base 5c6d4e2 and of this branch, same tree, same flags. Packages from
test/node_modules.Output bytes,
bun build e.mjs --target=node, wheree.mjsimports the package:--minifyIn the output that is not minified, each added line is the directive or a blank line. No line is removed.
Instructions.
valgrind,perf,bloatyandstraceare not installed in the build container, and it cannot reach a package mirror. The counts are user-space instructions of the wholebun buildprocess, read withperf_event_open:GOMAXPROCS=2, one CPU, the median of 15 interleaved runs, a generated graph of 400 wrapped CommonJS files, about 41M instructions for each build. A/A is a second series of the base binary.A file that gets no directive shows no delta above the A/A floor. Each printed directive costs 706 instructions.
Allocations: not measured, because
dhatis not available. By the code, a file that gets the directive takes one AST store slot for theS::Directive. TheVecthat holds it is reused for each file. A file that gets no directive takes nothing.Binary size (
size): text 107,635,854 to 107,636,110 (+256 bytes). data and bss are equal.Pinned bytecode snapshot (
bundler_bytecode_portable.test.ts). 6 entries move. Each is a bundle with wrapped CommonJS files that start with"use strict":all.js--minify all.jslibraries.js(64 packages)happy-dom/lib/index.jsreact-dom/cjs/react-dom.development.jsundici/index.jsThe other entries are byte-identical. The cache-load and
--compile --bytecodecases of that file pass with the new output. The 64 packages oflibraries.jsload with 2,586 wrappers that are now strict.Bundles that stop loading. Each of 67 packages (the 64 of
libraries.js, undici, svelte/compiler and lodash) was bundled alone to cjs for node with the two builds.node --checkfails for 0 of 67. The type and the key list of therequire()result differ between the two builds for 0 of 67. 3,593 of the 4,038__commonJSwrappers in these bundles now start with the directive. The total size goes from 25,967,806 to 26,025,305 bytes (+0.22%).__esmclosures. A cjs bundle ofimport("axios")has 43__esmwrappers. 32 start with the directive. One holds only the directive (+24 bytes, esbuild prints the same closure). The output holds no__INVALID__REF__.Earlier pull requests
append_directiveand keeps the one from here, with the wrapper symbol check. js_parser: keep "use strict" and the directive prologue in the output #40838 keeps its loop overast.directives. bundler: keep an ES module strict in cjs and iife output #40926 keepswrapper_needs_use_strictandcovered_by_chunk_top.sync_dependencies_endcursor. If this change lands first, bundler: keep source order in a wrapper prefix that has no async dependency #41601 turnsappend_directiveinto a plain push. If bundler: keep source order in a wrapper prefix that has no async dependency #41601 lands first, this change drops itsLinkerContext.rshunk and callsappend_non_dependency. The one that lands second regenerates the six snapshot entries.Suites
All runs are
bun bd teston the debug build.On this branch, rebased onto 5c6d4e2:
bundler_cjs(77 pass), the new case ofbundler_compile,bundler_bytecode_portable(22 pass,--timeout 1500000).bundler_cjs2esm(113),bundler_npm(2),bundler_bun(22),esbuild/extra(222),bun-build-api(90) andcli(43), the last two with--timeout 300000.esbuild/default(154 of 155) andbundler_edgecase(187 of 188).default/TopLevelAwaitForbiddenRequireSourceMapCLIandedgecase/DeepImportDiamondDAGtimed out while another job used the machine. Each passes when it runs alone.css/andresolver/(200 cases, all pass).bun-build-compile-sourcemap(9) andbundler_compile_splitting(30) with--timeout 600000.bun-build-compile(79 of 80) with--timeout 900000:bytecode from a compiled executable is not copied into private memoryexceeds its own 60 s limit on the debug build. It passes on the release builds of main and of this branch (3.8 s and 3.5 s).Before the rebase, on 38afee6: 96 of the 108 test files under
test/bundler(all butcss/andresolver/). The failures were 13 cases in 5 files (bun-build-api,bun-build-compile,bun-build-compile-sourcemap,bundler_compile_splitting,cli), each a--compileor bytecode case that ran longer than the default 5 s limit, or than its own 60 s limit, on the debug build.Mutation checks on the debug build:
!ast.wrapper_ref.is_empty(),cjs/UseStrictEsmFileWithoutWrapperSymbolaborts.append_directive, the twoUseStrictBeforeInitCallscases fail.flags.wrap != WrapKind::None,target_bun,target_bun_bytecodeandminifyfail.no test proof · iteration 0 · 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/bundler/bundler_bytecode_portable.test.ts