bundler: sort module namespace exports ascending to match spec - #35957
Conversation
bun build was emitting the __export() property literal in descending order, so Object.keys()/Reflect.ownKeys()/JSON.stringify() on an import * namespace produced a different order than bun run, node and esbuild bundles (which all follow the Module Namespace Exotic Object [[OwnPropertyKeys]] spec: sorted export names). Fixes sorted_and_filtered_export_aliases to use sort_asc. Updates the handful of existing bundler tests whose stdout/snapshot encoded the old descending order (all now match esbuild's output).
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
|
Updated 12:57 AM PT - Jul 28th, 2026
✅ @robobun, your commit e7d75eb212a4f447af7c9c7365bca0b983ef1522 passed in 🧪 To try this PR locally: bunx bun-pr 35957That installs a local version of the PR into your bun-35957 --bun |
There was a problem hiding this comment.
LGTM — one-word direction flip aligns bundled namespace key order with the spec and esbuild.
What was reviewed:
- Verified all consumers of
sorted_and_filtered_export_aliases(postProcessJSChunk, computeCrossChunkDependencies, MetafileBuilder, scanImportsAndExports) are order-agnostic or benefit from ascending. - Confirmed the
cjs_export_copies[i]positional correspondence in postProcessJSChunk is preserved — both arrays are built from the same post-sort list. - Checked no other
__export(...)snapshots or metafile-exports assertions in the suite encode multi-key descending order beyond the ones updated here.
Extended reasoning...
Overview
Single-line change in src/bundler/linker_context/doStep5.rs: strings::sort_desc → strings::sort_asc on the export-alias list that drives the __export(exports_foo, {...}) object literal. Five test files updated to reflect the new (spec-correct) ascending key order, plus one new itBundled test asserting Object.keys/Reflect.ownKeys/re-export-chain enumeration matches the ECMA-262 Module Namespace [[OwnPropertyKeys]] ordering.
Security risks
None. Pure output-ordering change in the bundler; no untrusted input parsing, no memory-layout changes, no FFI/syscall paths touched.
Level of scrutiny
Low-to-medium. The native change is trivially local (a comparator direction), but I traced every consumer of sorted_and_filtered_export_aliases to confirm none depends on descending order:
postProcessJSChunk.rs:1024iterates and indexescjs_export_copies[i]— that array is built atscanImportsAndExports.rs:623by zipping over the already-sorted alias list, so the positional pairing is preserved regardless of direction.computeCrossChunkDependencies.rsandMetafileBuilder.rsjust iterate; ascending is fine (metafile output improves — now matches esbuild).create_exports_for_filein doStep5 itself iterates to build the__exportliteral — this is exactly the emission the fix targets.
I also grepped the test tree for other __export(exports_...) snapshots and metafile "exports": [...] assertions with ≥2 keys; the only affected ones are already updated in this PR (the bun-build-api.test.ts.snap hits are single-key so unaffected).
Other factors
- The updated test expectations are the correct new values (ascending byte-order:
$< uppercase <_< lowercase), matching what Node/esbuild/unbundled Bun produce. - The new
NamespaceOwnKeysSortedtest covers a good input span ($,_, uppercase, lowercase,default, re-export chain) and would fail on main. - The debugId hash changes in the inline snapshots are the expected consequence of the byte-for-byte output change.
There was a problem hiding this comment.
LGTM — one-token direction fix (sort_desc → sort_asc) aligning bundled namespace key order with the spec and esbuild.
What was reviewed:
- Confirmed
strings::sort_ascis the ascending sibling of the removedsort_desc. - Checked all consumers of
sorted_and_filtered_export_aliases(postProcessJSChunk, computeCrossChunkDependencies, MetafileBuilder, create_exports_for_file) — none depend on descending order; they only affect emitted key order. - Grepped for other
__export(...)snapshots that might encode the old order — onlybun-build-api.test.ts.snapmatched and its three occurrences are single-key, so unaffected. - Spot-checked the large NodePolyfills stdout: per-module keys reversed to ascending; the
stream:block correctly unchanged (runtime-assigned properties, not__export).
Extended reasoning...
Overview
Single functional line changed in src/bundler/linker_context/doStep5.rs: strings::sort_desc → strings::sort_asc on the export-alias slice that becomes sorted_and_filtered_export_aliases. The remaining six files are test-expectation updates flipping key order from descending to ascending, plus one new itBundled test in importstar.test.ts that directly asserts Object.keys(ns) / Reflect.ownKeys(ns) and an export * chain enumerate in ascending order. Three inline-snapshot debugId hashes changed as a byproduct of the byte-level output diff.
Security risks
None. Pure output-ordering change in the bundler's namespace-export emission; no parsing of untrusted input, no allocation/bounds logic touched.
Level of scrutiny
Low-medium. The native change is a one-token flip between two sibling helpers whose implementations I confirmed (order(a,b) vs order(b,a)). I audited every reader of sorted_and_filtered_export_aliases — postProcessJSChunk (emits export {...} / __export({...})), computeCrossChunkDependencies (walks aliases to mark refs), MetafileBuilder (writes exports JSON array), and create_exports_for_file in the same file. All iterate the slice without any order-dependent logic; the order only surfaces in emitted artifacts, where ascending is the spec-mandated / esbuild-matching direction.
Other factors
- Verified no other
sort_desccall sites remain insrc/bundler/. - Grepped test snapshots for multi-key
__exportliterals that might have been missed — none found beyond what the PR already updated. - The new
NamespaceOwnKeysSortedtest exercises$,_, uppercase, lowercase,default, and anexport *merge — good coverage of the bytewise sort the spec requires. - The huge
bundler_browser.test.tsstdout string is the mechanical reversal of each namespace's key list; thestream:block is correctly left untouched since those properties are assigned at runtime rather than via__export.
|
All test lanes pass on build #82554. The only red job is |
There was a problem hiding this comment.
LGTM — one-token direction flip (sort_desc → sort_asc) that aligns bundled namespace key order with the spec, esbuild, Node, and unbundled Bun.
What was reviewed:
- Verified
strings::sort_ascis the byte-wise ascending sibling of the previoussort_desc(immutable.rs:1881). - Audited every consumer of
sorted_and_filtered_export_aliases(postProcessJSChunk, computeCrossChunkDependencies, MetafileBuilder, scanImportsAndExports) — none depend on descending order;cjs_export_copiesstays index-aligned because it's built by iterating the same already-sorted array. - Spot-checked the reordered test expectations (NodePolyfills, cjs, edgecase, importstar, snapshots) — same key sets, ascending byte order; the
export *chain test correctly omitsdefault.
Extended reasoning...
Overview
Single source-line change in src/bundler/linker_context/doStep5.rs:166: strings::sort_desc → strings::sort_asc on the export-alias list that feeds the __export(exports, {...}) object literal. The rest of the diff is mechanical test-expectation updates (reversing key order in stdout/snapshot strings, plus three regenerated debugId hashes) and three new itBundled cases in importstar.test.ts covering Object.keys/Reflect.ownKeys, export * re-export chains, export * as ns, and CJS-format entry-point exports.
Security risks
None. Pure output-ordering change in the bundler; no user-input parsing, allocation sizing, or trust boundary is touched.
Level of scrutiny
Low-to-medium. The runtime change is trivially inspectable — sort_asc and sort_desc are 3-line siblings differing only in comparator argument order (order(a,b) vs order(b,a), where order is a.cmp(b)). The interesting question is whether any downstream consumer of sorted_and_filtered_export_aliases depended on descending order. I checked all five consumers: create_exports_for_file (same file) iterates in order to emit properties; postProcessJSChunk.rs:1024 pairs each alias with cjs_export_copies[i], but that array is populated in scanImportsAndExports.rs:623 by zipping over the same sorted array after step 5 runs, so index alignment holds regardless of direction; computeCrossChunkDependencies.rs and MetafileBuilder.rs just enumerate. Nothing assumes descending.
The new order matches esbuild's sort.Strings(aliases) (byte-wise ascending) and the ECMA-262 Module Namespace [[OwnPropertyKeys]] requirement. The byte-wise-vs-UTF-16-code-unit edge case for non-BMP export aliases is pre-existing esbuild-parity behavior, not introduced here.
Other factors
- CI: robobun reports all test lanes green on build #82554; the only red job is the known-stale
binary-sizebaseline, unrelated to this diff. - New tests are well-designed:
NamespaceOwnKeysSortedexercises$, uppercase,_, lowercase,default, and anexport *chain (which correctly dropsdefault);CjsEntryPointExportsSortedcovers theformat: cjspath that goes throughpostProcessJSChunk. - The large
bundler_browser.test.tsstdout blob is a pure key reorder; I verified theassertandbuffersections and confirmed thestreamsection (a Function with insertion-order own properties, not a__exportnamespace) is correctly left untouched.
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
Closes #4695
Repro
bun entry.mjs,node entry.mjs, and the esbuild bundle all print$dollar,Alpha,_under,beta,default,zed(Module Namespace Exotic Object [[OwnPropertyKeys]] returns sorted export names).bun build entry.mjs --target=nodeprintedzed,default,beta,_under,Alpha,$dollarinstead.Cause
sorted_and_filtered_export_aliasesindoStep5.rswas built withstrings::sort_descinstead ofstrings::sort_asc. The__export(exports_leaf, {...})object literal was emitted in reverse-sorted order, soObject.keys()/for..in/JSON.stringify()on the bundled namespace disagreed with the unbundled run.Fix
One-character direction flip:
sort_desc→sort_asc. The__exportliteral now matches esbuild's output exactly.Updated the handful of existing bundler tests whose
stdout/snapshot encoded the old descending order; the new expected values match what esbuild produces.Verification
New test
importstar/NamespaceOwnKeysSortedassertsObject.keys(ns),Reflect.ownKeys(ns), and anexport *re-export chain all enumerate in ascending sorted order. Fails on main (zed,default,beta,_under,Alpha,$dollar), passes with this change.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