Skip to content

fix(transpiler): parse minifySyntax and minifyIdentifiers as top-level Bun.Transpiler options#28714

Open
robobun wants to merge 4 commits intomainfrom
farm/e6bc96b1/fix-transpiler-minify-syntax
Open

fix(transpiler): parse minifySyntax and minifyIdentifiers as top-level Bun.Transpiler options#28714
robobun wants to merge 4 commits intomainfrom
farm/e6bc96b1/fix-transpiler-minify-syntax

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented Mar 31, 2026

Problem

Two issues preventing string comparison DCE:

1. Bun.Transpiler ignored minifySyntax top-level option

Bun.Transpiler accepted minifyWhitespace as a top-level option but silently ignored minifySyntax and minifyIdentifiers. They only worked via minify: { syntax: true } or minify: true.

This meant new Bun.Transpiler({ minifySyntax: true }) did not enable syntax minification — if ("a" === "b") { ... } folded the comparison to false but left the empty if(false){} skeleton.

2. Cross-type strict equality not folded in bundler

The eql() function was missing cases for strict equality between different primitive types:

  • boolean === string (e.g. true === "x")
  • number === string (e.g. 42 === "x")
  • string === boolean (e.g. "x" === true)

These returned Equality.unknown instead of Equality.false, preventing DCE when --define substitutes a value of a different type than the comparison target.

Fix

  1. Added minifySyntax and minifyIdentifiers as top-level config keys in JSTranspiler.zig
  2. Added missing cross-type strict equality cases in Expr.zig eql() function

Verification

  • USE_SYSTEM_BUN=1 bun test → 11/12 fail
  • bun bd test → 12/12 pass

…l Bun.Transpiler options

minifyWhitespace was already accepted as a top-level option on Bun.Transpiler,
but minifySyntax and minifyIdentifiers were silently ignored when passed as
top-level keys. They only worked via minify: { syntax: true } or minify: true.

This caused if ("a" === "b") { ... } to not be DCE'd when using
new Bun.Transpiler({ minifySyntax: true }) — the comparison would fold to
false but the empty if(false){} skeleton remained because minify_syntax
was never actually set on the parser options.
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Mar 31, 2026

Updated 6:59 AM PT - Mar 31st, 2026

@autofix-ci[bot], your commit 5e49eb6 has 5 failures in Build #43024 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 28714

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

bun-28714 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a7b67d7d-8c77-4ba4-965b-b72199257408

📥 Commits

Reviewing files that changed from the base of the PR and between b050dbd and 5e49eb6.

📒 Files selected for processing (2)
  • src/ast/Expr.zig
  • test/bundler/transpiler/transpiler-minify-dce.test.ts

Walkthrough

Adds top-level minification flags to the transpiler config, expands strict-equality folding logic in expressions, and introduces tests covering minification combined with dead-code elimination and constant-folding behaviors.

Changes

Cohort / File(s) Summary
Transpiler config
src/bun.js/api/JSTranspiler.zig
Parses top-level boolean flags minifySyntax and minifyIdentifiers from globalThis in Config.fromJS, assigning to minify_syntax and minify_identifiers before existing deadCodeElimination parsing and independent of minify composite options.
Expression equality logic
src/ast/Expr.zig
Extends Expr.Data.eql() strict-equality (kind == .strict) handling to explicitly return Equality.false for cross-type comparisons between strings and booleans/numbers, preventing indeterminate results.
Tests — minify + DCE
test/bundler/transpiler/transpiler-minify-dce.test.ts
Adds tests exercising syntax and identifier minification combined with dead-code elimination and constant folding; verifies removed/retained branches, boolean/arithmetic folding, identifier minification, and cross-type strict-equality folding outcomes.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately describes the main change: adding support for minifySyntax and minifyIdentifiers as top-level Bun.Transpiler options.
Description check ✅ Passed The PR description exceeds the template with clear problem statements, detailed fixes, and verification steps, though it doesn't use the exact template structure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

robobun and others added 2 commits March 31, 2026 12:17
With strict equality (===), values of different types are never equal.
The eql() function was missing cases for:
- boolean === string (e.g. true === 'x')
- number === string (e.g. 42 === 'x')
- string === boolean (e.g. 'x' === true)

These returned Equality.unknown instead of Equality.false, preventing
DCE when --define substitutes a value of a different type than the
comparison target.
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