Skip to content

minify: do not fold new Array(x, ...spread) into an array literal - #41828

Merged
Jarred-Sumner merged 3 commits into
mainfrom
robobun/847eef49/new-array-spread-fold
Sep 7, 2026
Merged

Jarred-Sumner merged 3 commits into
mainfrom
robobun/847eef49/new-array-spread-fold

Conversation

@robobun

@robobun robobun commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • With minify_syntax, new Array(5, ...rest) becomes [5, ...rest]. When rest is empty the original means new Array(5), five holes, and the fold is [5]. For const none = []; const a = new Array(5, ...none); console.log(a.length, 0 in a) Node prints 5 false. Bun 1.4.3 prints 1 true under bun run and in bun build --minify-syntax output.
  • The cause is the more-than-one-argument branch of KnownGlobal::minify_global_constructor (src/ast/known_global.rs:244). It folds the arguments into a literal without checking for a spread, so the argument count can differ at runtime.

Fix

  • If any argument is a spread, emit Array(5, ...rest) instead of a literal. This is the call_from_new form the single-argument branch already uses when the argument may be a number.
  • Correct because Array called as a function behaves like new Array (ECMA-262 23.1.1). Only the literal fold was unsound.
  • EXPECTED_VERSION in RuntimeTranspilerCache.rs moves to 29: the runtime transpiler enables minify_syntax, so its cached output changes.
  • Verified: test/bundler/bundler_minify.test.ts (one case captures the output, one runs it, both fail on 1.4.3). Also bundler_npm.test.ts and minify-new-array-with-if.test.ts.

Background

Notes

With more than one argument, minify_syntax folds new Array(a, b) into
[a, b]. When an argument is a spread, the argument count is not known
until runtime: new Array(5, ...rest) is new Array(5), a length, when
rest is empty, but the fold produced [5]. Keep the constructor call for
any spread argument, as the single-argument case already does.
@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 7a65fa38-4b30-4d99-a005-1fed99db1ce9

📥 Commits

Reviewing files that changed from the base of the PR and between ae3c3ad and fb5b797.

📒 Files selected for processing (3)
  • src/ast/known_global.rs
  • src/jsc/RuntimeTranspilerCache.rs
  • test/bundler/bundler_minify.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


Walkthrough

Changes

The minifier now preserves new Array(...) when arguments include spreads. The runtime transpiler cache version is incremented. Bundler tests cover unknown spreads and sparse arrays created with empty spreads.

Array constructor minification

Layer / File(s) Summary
Preserve spread-based Array constructors
src/ast/known_global.rs
The new Array(...) optimization detects spread arguments and retains the constructor call. Non-spread calls with multiple arguments remain foldable.
Update cache version and validation
src/jsc/RuntimeTranspilerCache.rs, test/bundler/bundler_minify.test.ts
The cache version changes from 28 to 29. Tests cover unknown spreads, minified output, and sparse-array length and membership semantics.

Suggested reviewers: jarred-sumner

Merge Risk: ⚪ Minimal · up to fb5b7

Minified spread-based Array construction now preserves sparse-array semantics, with cache invalidation and regression coverage included. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, fix, rationale, cache version change, and verification. It does not use the exact template headings, but it provides the required information and is mostl…
Title check ✅ Passed The title is concise, specific, and accurately summarizes the main change: preventing folding of new Array calls with spread arguments.
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.

@github-actions github-actions Bot added the claude label Sep 7, 2026
@robobun

robobun commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

Reproduced on Bun 1.4.3 with:

// entry.js
const none = [];
const a5 = new Array(5, ...none);
console.log(a5.length, 0 in a5);
  • bun entry.js prints 1 true. Node prints 5 false.
  • bun build --minify-syntax entry.js emits a5 = [5, ...none].

With this branch both print 5 false, and the bundle emits a5 = Array(5, ...none). test/bundler/bundler_minify.test.ts fails on 1.4.3 in minify/AdditionalGlobalConstructorOptimization and minify/GlobalConstructorSemanticsPreserved, and passes here (51/51 under the debug build).

Split out of #37388, which is closed in favour of #41580.

Comment thread src/ast/known_global.rs
Comment on lines +246 to +247
// But NOT new Array(3) which creates an array with 3 empty slots,
// and `new Array(5, ...rest)` is `new Array(5)` when `rest` is empty.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

@robobun

robobun commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 11:40 PM PT - Sep 6th, 2026

⏳ @autofix-ci[bot], your commit fb5b797 is still building in Build #112078, but has 2 failures so far (All Failures):

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@Jarred-Sumner
Jarred-Sumner merged commit 8940b0e into main Sep 7, 2026
9 of 10 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/847eef49/new-array-spread-fold branch September 7, 2026 06:44
putao520 added a commit to putao520/bao that referenced this pull request Sep 7, 2026
…al (absorb bun 8940b0eec7)

minify_global_constructor's >1-argument branch folded arguments into a literal
without checking for spread: new Array(5, ...rest) became [5, ...rest], wrong
when rest is empty (5 holes vs [5]). Any spread argument now keeps the
call_from_new form. RUNTIME_TRANSPILER_CACHE_VERSION 20->21 (minify_syntax
output shape change; mirrors upstream expected_version 28->29).
Upstream: oven-sh/bun#41828.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants