Repository navigation
Conversation
|
Updated 1:46 AM PT - Aug 13th, 2026
✅ @robobun, your commit 9f6342f7b0dde97ac7b916c3863872f91e92fa64 passed in 🧪 To try this PR locally: bunx bun-pr 29312That installs a local version of the PR into your bun-29312 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDiverted float printing in the JS printer to a new internal Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/js_printer.zig`:
- Around line 1615-1620: The code in printFloatSmart() currently strips the
leading zero from dec_full into dec (dec_full[1..]) which breaks JSON output
used by printJSON(); change printFloatSmart to avoid removing the integer 0 when
used for JSON by adding a boolean parameter (e.g., keep_leading_zero or is_json)
or a context flag, and only perform the dec_full[1..] transformation when the
flag is false; update callers (including printJSON) to pass the appropriate flag
so JSON printing preserves "0.5" and "-0.5".
In `@test/regression/issue/29311.test.ts`:
- Line 38: Tests currently assert exitCode with expect(exitCode).toBe(0) before
verifying stdout/content; move each expect(exitCode).toBe(0) to the end of its
respective test assertion block so stdout/content checks (e.g.,
expect(stdout).toContain(...), expect(content).toMatch(...)) run first and the
exit code assertion runs last; update the three occurrences (the
expect(exitCode).toBe(0) at the current locations and the ones referenced in the
comment around lines 74-75 and 111-113) accordingly.
- Line 37: Remove the fragile assertion that stderr is empty in the regression
test by deleting or commenting out the expect(stderr).toBe(""); check in the
test (the assertion on stderr found in the test/regression/issue/*.test.ts
file); for stability, either remove the stderr assertion entirely or replace it
with a non-fatal check (e.g., only fail on non-zero exit code) so ASAN/JSC
warnings in spawned Bun subprocesses do not cause CI flakes—update the test that
contains expect(stderr).toBe(""); and any duplicate assertions (e.g., the other
occurrence noted) accordingly.
🪄 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: 51dbc057-c25b-4c55-a4c2-2cf6f34c8c3c
📒 Files selected for processing (3)
src/js_printer.zigtest/bundler/bundler_minify.test.tstest/regression/issue/29311.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
test/regression/issue/29311.test.ts (2)
34-34:⚠️ Potential issue | 🟡 MinorAssert exit code last in each spawned-process assertion block.
Move
expect(...exitCode).toBe(0)to the end, after stdout/content checks, for clearer failures and to match test conventions.Suggested patch
- expect(exitCode).toBe(0); - // The bug: 1e300 was expanded to 301 digits. Guard against any regression // that re-introduces long runs of zeros in the minified output. expect(stdout).not.toMatch(/0{20,}/); @@ // 1.5e20 and 15e19 are both 6 chars — tie goes to decimal/scientific // depending on Zig's {e} output; both are acceptable. expect(stdout).toMatch(/1\.5e20|15e19/); + expect(exitCode).toBe(0); @@ - expect(runExitCode).toBe(0); expect(runStdout.split("\n").filter(Boolean)).toEqual([ @@ "-1e+300", "150000000000000000000", ]); + expect(runExitCode).toBe(0); @@ - expect(exitCode).toBe(0); // Without --minify, 1.5e20 stays in its full decimal form as before. expect(stdout).toContain("150000000000000000000"); + expect(exitCode).toBe(0);As per coding guidelines: Expect stdout assertions before exit code assertions - expect(stdout).toBe(...) BEFORE expect(exitCode).toBe(0) for more useful error messages.
Also applies to: 70-71, 103-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/regression/issue/29311.test.ts` at line 34, Several spawned-process assertion blocks in test/regression/issue/29311.test.ts assert exitCode too early; move each expect(exitCode).toBe(0) to the end of its respective assertion block so stdout/content checks (e.g., expect(stdout).toContain(...) / expect(stderr).toBe(...)) come first. Update the three affected blocks (the one containing the current expect(exitCode).toBe(0) near the start, and the other blocks referenced around the comment ranges) so that exitCode assertions run after all stdout/stderr/content expectations for clearer failure messages.
33-33:⚠️ Potential issue | 🟠 MajorRemove empty-
stderrassertions in regression subprocess tests.
expect(stderr).toBe("")is brittle in ASAN/debug CI where benign warnings can appear on stderr even when behavior is correct.Suggested patch
- const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const [stdout, _stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).toBe(""); + void _stderr;Based on learnings: In regression tests under
test/regression/issue/*.test.ts, avoid asserting spawned Bun subprocessstderris empty because ASAN may emit warnings.Also applies to: 102-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/regression/issue/29311.test.ts` at line 33, Remove the brittle empty-stderr assertion by deleting the expect(stderr).toBe("") check found in this regression test (and any identical occurrences in other tests under the regression issue suite); simply stop asserting stderr is exactly empty (remove the expect(stderr).toBe("") line) so spawned Bun subprocess tests no longer fail due to benign ASAN/debug warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/regression/issue/29311.test.ts`:
- Line 34: Several spawned-process assertion blocks in
test/regression/issue/29311.test.ts assert exitCode too early; move each
expect(exitCode).toBe(0) to the end of its respective assertion block so
stdout/content checks (e.g., expect(stdout).toContain(...) /
expect(stderr).toBe(...)) come first. Update the three affected blocks (the one
containing the current expect(exitCode).toBe(0) near the start, and the other
blocks referenced around the comment ranges) so that exitCode assertions run
after all stdout/stderr/content expectations for clearer failure messages.
- Line 33: Remove the brittle empty-stderr assertion by deleting the
expect(stderr).toBe("") check found in this regression test (and any identical
occurrences in other tests under the regression issue suite); simply stop
asserting stderr is exactly empty (remove the expect(stderr).toBe("") line) so
spawned Bun subprocess tests no longer fail due to benign ASAN/debug warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4a75986-a3ca-4b8b-b67f-a61c0e94fcb4
📥 Commits
Reviewing files that changed from the base of the PR and between ddf3aed and f40bbfe809f2cc8e53b04ba57fa72b69cb181040.
📒 Files selected for processing (1)
test/regression/issue/29311.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
f40bbfe to
20e0911
Compare
There was a problem hiding this comment.
LGTM — all prior review concerns (JSON guard, integer fast-path bypass, test precision, assertion ordering) addressed in the follow-up commit.
Extended reasoning...
Overview
This PR introduces printFloatSmart in src/js_printer.zig that, when minifying JS, picks the shorter of the decimal and scientific forms for every f64 value. It also adds a regression test and updates one bundler snapshot expectation.
Security risks
None. This is purely a code-generation optimization in the JS printer with no security implications.
Level of scrutiny
Moderate — the change touches the printer's float-emission path, which is used extensively. However, the logic is self-contained, well-commented, and the two critical correctness risks (JSON invalidity from .5 and integer fast-path bypass) were both found and fixed in the follow-up commit 20e0911.
Other factors
My previous two rounds of reviews flagged a 🔴 JSON-printer correctness bug (0.5→.5 invalid in RFC 8259), a 🔴 integer fast-path bypass, and several 🟡 test-quality issues. Commit 20e0911 addresses all of them: the early-return guard before the switch ensures minified non-JSON paths always go through printFloatSmart, the comptime is_json guard keeps JSON output RFC-compliant, and the regression tests now use precise assertions. No outstanding issues remain.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/regression/issue/29311.test.ts`:
- Around line 5-7: In the test header comment in
test/regression/issue/29311.test.ts remove the three-line bug-history prose that
follows the issue URL (the sentences about Minifier expanding
scientific-notation floats and the example) and leave only the issue URL and any
comments that explain the test design rationale; ensure you do not remove or
alter comments that describe why the test exists or how it validates
behavior—only delete the historical/contextual prose.
🪄 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: d6c4420f-e8df-4648-bd03-6aa0c48f4601
📒 Files selected for processing (1)
test/regression/issue/29311.test.ts
f026816 to
46a3033
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/regression/issue/29311.test.ts`:
- Around line 27-28: Add two focused test cases in
test/regression/issue/29311.test.ts that run Bun.build with each standalone
flag: spawn processes using Bun.spawn invoking bunExe() with ["build",
"--minify-syntax", `${dir}/index.js`] and ["build", "--minify-whitespace",
`${dir}/index.js`] respectively (similar to the existing combined "--minify"
invocation). For each process (created like the existing proc), await
completion, check exit code and stdout/stderr to mirror the expectations used
for the combined minify test so the suite verifies each flag is wired
independently (use the same assertions/assert helpers currently used for the
combined case). Ensure tests are named/described to indicate they are standalone
checks for --minify-syntax and --minify-whitespace.
🪄 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: 4deb24c5-4101-4af4-b4ad-9b57784f2f26
📒 Files selected for processing (1)
test/regression/issue/29311.test.ts
ff3ac3f to
dbdb8bf
Compare
There was a problem hiding this comment.
LGTM — all previously reported issues resolved.
Extended reasoning...
Overview
The PR adds printFloatSmart to src/js_printer.zig and hooks it into printNonNegativeFloat. When --minify-syntax or --minify-whitespace is active (and the printer is not in JSON mode), it formats each float with both Zig's {d} and {e} formatters, applies two normalizations (0.5→.5 and dropping trailing e0), and emits whichever form is shorter. Four files changed: the Zig printer, two bundler test expectation files, and a new regression test.
Security risks
None. The change is purely a formatting/output optimization with no auth, crypto, permissions, or external data-handling.
Level of scrutiny
Moderate — the fix touches a hot path in the JS printer that affects all minified builds, and a JSON correctness concern (invalid .5) was initially present. Both the JSON printer guard (comptime is_json early return in printFloatSmart, plus \!is_json on the early-dispatch in printNonNegativeFloat) and the integer fast-path bypass have been correctly addressed in subsequent commits.
Other factors
All red and yellow inline bug reports from prior review rounds have been marked resolved and confirmed fixed by robobun. The regression test covers: the primary 1e300 bug, small-exponent floats, the 0.5→.5 rewrite, integer fast-path ranges (20000, 2000000000), standalone --minify-syntax and --minify-whitespace flags, non-minified decimal preservation, and a runtime round-trip correctness check.
dbdb8bf to
941c208
Compare
There was a problem hiding this comment.
All prior review feedback has been addressed and the bug hunter found nothing new, but since this changes float emission in the core JS printer (affecting every minified bundle), I'd like a human to give it a final look before merge.
Extended reasoning...
Overview
This PR adds printFloatSmart to src/js_printer/js_printer.zig (~70 lines) and reroutes printNonNegativeFloat through it whenever minify_syntax or minify_whitespace is set for non-JSON output. It formats each float with both {d} and {e}, applies two rewrites (0.5→.5, strip trailing e0), and emits the shorter form. Three test files are touched: a new regression test, and updated capture expectations in bundler_minify.test.ts and bundler_edgecase.test.ts.
Security risks
None. This is pure output formatting of numeric literals; no untrusted input handling, auth, or I/O changes.
Level of scrutiny
Medium-high. The JS printer is on the hot path for every bun build invocation, and float emission correctness directly affects whether minified bundles parse and round-trip. Earlier review rounds caught a real correctness bug (.5 being emitted in JSON mode, breaking bun publish), an incomplete fix (integer fast-path ranges bypassing the smart path), and a stale test snapshot (bundler_edgecase NaN-boxed capture). All of those are now fixed and verified in the current diff — the is_json comptime guard is in place at both call sites, the early-return covers all values, and the edgecase capture is updated to -1.7976931348623157e308.
Other factors
The regression test is thorough (shape assertions + runtime round-trip + per-flag coverage), CI is green except for two unrelated flakes (bun-add, webview-chrome), and the implementation matches esbuild's approach. I'm deferring rather than approving only because this is a behavioural change to a critical printer path that took several iterations to get right, and a human sign-off on the final Zig diff seems prudent.
There was a problem hiding this comment.
All prior review feedback is addressed and the bug hunter found nothing new — deferring to a human since this changes numeric-literal output in the core JS printer for every minified bundle, and Build #51318 hasn't confirmed green yet.
Extended reasoning...
Overview
This PR adds printFloatSmart to src/js_printer/js_printer.zig and routes all non-negative floats through it when --minify-syntax or --minify-whitespace is enabled (non-JSON only). It compares Zig's {d} and {e} renderings, applies two rewrites (0.5→.5, strip trailing e0), and emits the shorter form. Three test files are updated/added: a new regression suite, plus capture updates in bundler_minify.test.ts and bundler_edgecase.test.ts.
Security risks
None. This is pure output formatting of numeric literals; no user input parsing, auth, filesystem, or network surface is touched.
Level of scrutiny
Moderate-to-high. The change is small and well-scoped, but printNonNegativeFloat runs for every numeric literal in every minified bundle Bun produces. A subtle round-trip mismatch between Zig's float formatter and JS number parsing would silently corrupt user code at runtime. The regression test does include a runtime round-trip check (bun -e <minified output>) which is good mitigation, and the JSON path is correctly guarded by comptime is_json at two layers — but this is still core-printer territory where a human sign-off is warranted.
Other factors
- This PR has been through several review rounds; every issue I (and CodeRabbit) raised is now resolved in the current diff, including the JSON
.5guard, the integer fast-path bypass, thebundler_edgecaseNaN-boxed capture, the anchoredconsole.log(1e300)assertion, and standalone--minify-syntax/--minify-whitespacecoverage. - The robobun status comment still shows ❌ for Build #51318; the latest commit (
eba26f5) is a CI retrigger because the previous jobs expired before running, so CI hasn't actually confirmed the current revision passes across all platforms. - Given the production-critical path and the not-yet-green CI, I'm deferring rather than approving.
|
FYI: builds 51318 and 51364 both suffered mass BuildKite job expirations (nearly every failure is status |
eba26f5 to
94f68bd
Compare
There was a problem hiding this comment.
All prior feedback is addressed and I found no new issues. Deferring to a human since this reroutes every float literal in minified output through new {e}-vs-{d} selection logic — a hot, correctness-critical printer path — and CI hasn't yet produced a clean green run (recent BK builds expired before executing).
Extended reasoning...
Overview
This PR adds printFloatSmart to src/js_printer/js_printer.zig and an early-return in printNonNegativeFloat so that, when --minify-syntax or --minify-whitespace is enabled (and not JSON), every non-negative float is formatted with both Zig's {d} and {e} specifiers and the strictly shorter form is emitted (with 0. → . and trailing e0 stripped). It updates two existing bundler test expectations (bundler_minify.test.ts +'0.1' → .1; bundler_edgecase.test.ts NaN-boxed -1.797…e308 capture) and adds a thorough regression test at test/regression/issue/29311.test.ts covering shape assertions, runtime round-tripping, the integer fast-path bypass, and standalone-flag wiring.
Security risks
None. This is pure output-formatting of numeric literals; no untrusted input parsing, auth, crypto, filesystem, or network surface is touched.
Level of scrutiny
High. printNonNegativeFloat runs for every numeric literal in every minified bundle bun produces. Correctness here depends on Zig's {e} formatter emitting a shortest-round-trip representation that re-parses in JS engines to the exact same f64 (Ryu-based, so this should hold), and on the two string rewrites preserving JS lexical validity. The JSON path is correctly guarded by comptime is_json at both the early-return and inside printFloatSmart, so bun publish etc. won't emit .5. The logic looks right and the regression test includes a runtime round-trip check, but a subtle mismatch would silently corrupt numeric constants across the ecosystem — that warrants a human sign-off rather than bot approval.
Other factors
- Over four prior review rounds, every issue I and CodeRabbit raised (JSON
.5, integer fast-path bypass, vacuous test assertions, exitCode ordering, ASAN-stderr flakiness, missingbundler_edgecasesnapshot update, standalone-flag coverage) has been addressed; all inline threads are resolved. - No CODEOWNERS entry covers
src/js_printer/. - Per the author's latest note, BuildKite builds 51318/51364 mass-expired before running, so there is not yet a fully green CI run on the final revision — another reason to wait for a human + green CI before merging.
da2fcf7 to
b307264
Compare
|
Rebased onto current main and resolved the conflict. The conflict was non-trivial: since this PR was opened, the runtime was ported from Zig to Rust (
Test-side changes carried over unchanged (they're runtime-agnostic Verified locally with |
There was a problem hiding this comment.
All prior feedback is addressed and I didn't find new issues, but this changes float printing for every minified bundle and was just re-implemented from Zig to Rust during the rebase — worth a human look at the print_float_smart port specifically (Rust's {}/{:e} semantics differ from Zig's {d}/{e}).
Extended reasoning...
Overview
This PR adds print_float_smart to src/js_printer/lib.rs and routes all non-JSON float printing through it when minify_whitespace or minify_syntax is enabled. It formats the value with both Rust's Display ({}) and LowerExp ({:e}), applies two rewrites (0.5 → .5, strip trailing e0), and emits whichever is strictly shorter. It also updates two existing bundler snapshot expectations (bundler_minify.test.ts, bundler_edgecase.test.ts) and adds a comprehensive regression test with runtime round-trip verification.
Security risks
None. This is pure output-formatting logic for numeric literals; there's no user input handling, auth, crypto, or filesystem interaction. The worst-case failure mode is emitting a numeric literal that doesn't round-trip to the same f64, which would be a correctness bug, not a security issue.
Level of scrutiny
Medium-high. print_non_negative_float runs for every numeric literal in every bundled/minified file, so a subtle round-trip bug here would silently corrupt user bundles. More importantly, this PR was originally written and reviewed against src/js_printer.zig, but the runtime has since been ported to Rust and the author re-implemented the fix during rebase. Rust's Display for f64 (Ryu-based shortest round-trip) and LowerExp have different edge-case behavior than Zig's {d}/{e}, so the prior review rounds don't fully transfer. The !IS_JSON guard at the call site correctly preserves RFC 8259 compliance for JSON output (the earlier in-function backstop was dropped, which is fine since there's only one call site).
Other factors
- All eight prior inline comments (mine and CodeRabbit's) are resolved and verified addressed in the current diff: JSON guard, integer fast-path bypass, exitCode ordering, ASAN stderr, vacuous
.5/1e300substring assertions,bundler_edgecasesnapshot, per-flag coverage. - The regression test is thorough (shape assertions + runtime round-trip + per-flag coverage), and the author reports 4/4 + 39/39 + 2/2 passing locally on the rebased build.
- CI currently shows musl build failures and a Windows
napi.test.tsfailure — these look unrelated to this change but the build is not green. - Given the cross-language re-implementation of a hot path in the printer, I'd prefer a human confirm the Rust formatting semantics rather than auto-approve.
|
The diff itself is green — every lane that exercises the change passes. The remaining red is unrelated Windows flake:
The failing set changes run-to-run, which is the flake signature. None of these tests touch the JS printer or float formatting — the diff is scoped to I've used my one CI re-roll for this round, so I'm not going to keep pushing retrigger commits. This needs a maintainer to re-run the flaky Windows lanes (or confirm |
cb0ebe8 to
a804c69
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
a804c69 to
3aa6915
Compare
3aa6915 to
0f76f8e
Compare
print_non_negative_float always emitted the shortest round-trip decimal, so 1e300 expanded to 301 digits under --minify. When minifying JS (not JSON), format the value with both Display (decimal) and LowerExp (scientific), apply two length-preserving rewrites (0.5 -> .5, strip a trailing e0), and emit whichever is shorter. Matches esbuild. Routing every value through print_float_smart when minifying also covers the integer fast path (e.g. 2000000000 -> 2e9), which previously emitted full decimal for anything that was not a round 1e4..1e9 power of ten. JSON output keeps the decimal form (a bare leading '.' is invalid JSON), guarded by the IS_JSON const generic. Closes #29311
0f76f8e to
9f6342f
Compare
There was a problem hiding this comment.
LGTM — the earlier comment-cop / verbosity feedback is addressed; comments are now single-line.
What was reviewed
print_float_smart: traced every test value through{}vs{:e}, the0.→.and trailing-e0rewrites, and the tie-break — all produce the asserted output and round-trip.- Confirmed
print_numberfilters NaN/±Infinity beforeprint_non_negative_float, soprint_float_smartonly sees finite non-negative values (noinfleakage). IS_JSONguard keeps JSON output on the plain decimal path; non-minified path unchanged.bundler_edgecase/bundler_minifysnapshot updates match the new shorter forms; no other float-shape assertions found in the suite.
Extended reasoning...
Overview
Adds print_float_smart to src/js_printer/lib.rs (~35 lines) and an early-return in print_non_negative_float that routes every finite non-negative float through it when minify_syntax || minify_whitespace is set and the output is not JSON. The helper formats both {} (decimal) and {:e} (scientific) into stack buffers, applies two length-preserving JS-specific rewrites (0.5→.5, strip trailing e0), and prints whichever slice is strictly shorter (decimal on ties, matching esbuild). Updates two existing bundler snapshot expectations and adds a 4-case regression test that also round-trips the minified output through the runtime.
Security risks
None. Pure formatting of an already-parsed f64 into a fixed stack buffer; buf_print returns Err on overflow and both let-else arms fall back to the previous streaming path. No allocation, no user-controlled length, no FFI.
Level of scrutiny
Moderate. The printer is hot-path and its output is user-visible, but the change is gated to minified JS only (non-minified and JSON output are byte-identical to before), the transform is length-preserving-or-shortening by construction, and the regression test executes the minified output to prove semantic equivalence for every covered value. I hand-traced the twelve fixture values plus 0.0, 10, and 1.5 through both branches and the rewrites; each matches the asserted string.
Other factors
This PR has been through three prior review rounds (mine and CodeRabbit's) — every thread is resolved, including the comment-cop verbosity flags (current diff has single-line comments only). The caller print_number handles NaN/Infinity/sign before this function, so leading-dot output (.5) only appears where a number literal is syntactically valid in JS. The bundler_edgecase NaN-boxed capture and bundler_minify +'0.1' capture are the only pre-existing assertions on decimal float shape under minify, and both are updated here.
Closes #29311.
Repro
Before:
After (and what esbuild produces):
Cause
printNonNegativeFloatinsrc/js_printer.zighas explicit fast-paths for a handful of integer powers of ten (1e4..1e9), but every other large or small float fell through top.fmt("{d}", ...), which always expands to the shortest round-trip decimal — 301 digits for1e300.Fix
Added
printFloatSmartnext toprintNonNegativeFloat. When--minify-syntaxor--minify-whitespaceis on, it formats the value with both{d}and Zig's{e}, applies two length-preserving rewrites (0.5→.5,3.14159e0→3.14159), and emits whichever is shorter. Non-minified output keeps the existing{d}behaviour so source maps and snapshots that lean on decimal form don't shift.Verification
test/regression/issue/29311.test.ts— two tests:--minify: confirms1e300,1e-5,1e-7,1e5,1e12,.5, etc. are emitted, runs the minified JS, and checks each value round-trips through the JS runtime.1.5e20still prints as the 21-digit decimal form (no unintentional change for non-minified output).One existing bundler expectation in
test/bundler/bundler_minify.test.ts(minify/ConstantFoldingUnaryPlusString) updated:+'0.1'now minifies to.1instead of0.1(strictly shorter, same semantics).Fail-before verified with
USE_SYSTEM_BUN=1— the minify test fails against stock bun and passes against this build.