Skip to content

fix(minifier): preserve 0 && (module.exports = { ... }) cjs-module-lexer hint#22729

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-preserve-cjs-module-lexer-hint
May 26, 2026
Merged

fix(minifier): preserve 0 && (module.exports = { ... }) cjs-module-lexer hint#22729
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-preserve-cjs-module-lexer-hint

Conversation

@Dunqing

@Dunqing Dunqing commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

esbuild emits unreachable 0 && (module.exports = { ... }) statements on Node platform as parse-time hints for cjs-module-lexer. The actual exports happen through helper calls (__export(...) or similar) that the lexer can't trace, so it scans for the literal module.exports = { ... } shape inside this dead-code expression to discover named exports.

Exact emission site (esbuild v0.28.0): internal/linker/linker.go#L5127-L5138. Always LogicalAnd(0, Assign(module.exports, ObjectExpression)) — no other shape.

oxc's DCE judges the expression by JS-runtime semantics — false && X is unreachable, drop it — and is correct on those grounds. But this expression carries extra-semantic information the lexer reads. Dropping it silently breaks ESM consumers:

import { clearRequireCache } from "@tailwindcss/node/require-cache";
//       ^^^^^^^^^^^^^^^^^
// SyntaxError: The requested module does not provide an export named 'clearRequireCache'

Observed in monitor-oxc CI: https://github.com/oxc-project/monitor-oxc/actions/runs/26427515847/job/77794148798

This is the same class of bug as #22475 (string quotes for require()) and #22552 (consolidated codegen helpers) — output properties that matter to downstream tooling, lost by minifier passes that only see JS semantics.

Fix

Commit 1: bailout in try_fold_and_or

Restores the guard added in #4878 and dropped during the #8618 refactor. In the constant-fold shortcut at fold_constants.rs, when lval == false && op == And and the right operand is a module.exports = { ... } assignment, return None instead of folding 0 && X → 0.

Commit 2: close remaining elimination paths

Code review surfaced two bypasses around the first guard:

  1. remove_unused_logical_expr (separate elimination path) — when treeshake.property_write_side_effects: false (a documented rolldown/vite option), the assignment reports no side effects, the whole 0 && (module.exports = {...}) reports no side effects, and try_fold_expression_stmtremove_unused_expressionremove_unused_logical_expr drops the entire ExpressionStatement before the fold-constants guard ever runs. Added a matching guard at remove_unused_expression.rs.

  2. Compound assignments + paren-wrapping — the helper matched module.exports ||= X (never emitted) and didn't strip ParenthesizedExpression uniformly. Tightened to require AssignmentOperator::Assign and apply get_inner_expression() at the top.

Helper scope

is_cjs_module_exports_hint matches exactly esbuild's emission shape: an = assignment with LHS module.exports. Verified against the source — esbuild never emits exports.X = v, module.exports.X = v, || form, or compound operators as hints.

Not handled (intentional):

  • 0 && (exports.X = v) — esbuild doesn't emit it.
  • 0 && (module.exports.X.Y = v) — esbuild doesn't emit it.
  • Locally-shadowed module/exports — the lexical check over-preserves harmlessly. Threading ctx.is_global_reference through would expand the helper's surface for no observed bug.

Tests

test_preserve_cjs_module_lexer_hint in tests/peephole/fold_constants.rs covers the positive shape + negative cases (compound assignments fold; non-export RHS folds).

Smoke-tested against @tailwindcss/node/dist/require-cache.js: hint line survives DCE.

Stacks on #22722.

@github-actions github-actions Bot added the A-minifier Area - Minifier label May 26, 2026
@codspeed-hq

codspeed-hq Bot commented May 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 52 untouched benchmarks
⏩ 63 skipped benchmarks1


Comparing fix/minifier-preserve-cjs-module-lexer-hint (5d99750) with fix/minifier-mark-changed-on-dead-stmt-drop (d46a46a)

Open in CodSpeed

Footnotes

  1. 63 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Dunqing Dunqing added the run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it label May 26, 2026
@oxc-guard

oxc-guard Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

@Dunqing Dunqing force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from 0a13eb6 to ec974a2 Compare May 26, 2026 02:24
@Dunqing Dunqing changed the base branch from main to fix/minifier-mark-changed-on-dead-stmt-drop May 26, 2026 02:24

Dunqing commented May 26, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Dunqing Dunqing added run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it and removed run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it labels May 26, 2026
@Dunqing Dunqing force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from 7c7597b to 8626ad0 Compare May 26, 2026 04:13
@Dunqing Dunqing force-pushed the fix/minifier-mark-changed-on-dead-stmt-drop branch from f773b40 to f27adf6 Compare May 26, 2026 04:51
@Dunqing Dunqing force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from 836bd21 to bc5d190 Compare May 26, 2026 04:51
@Dunqing Dunqing force-pushed the fix/minifier-mark-changed-on-dead-stmt-drop branch from f27adf6 to d46a46a Compare May 26, 2026 04:53
@Dunqing Dunqing force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from bc5d190 to fe67f83 Compare May 26, 2026 04:53
@Dunqing Dunqing added 0-merge Merge with Graphite Merge Queue run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it and removed run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it labels May 26, 2026
@Dunqing Dunqing force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from 5d99750 to aee7f70 Compare May 26, 2026 05:19
@Dunqing Dunqing marked this pull request as ready for review May 26, 2026 05:29

Dunqing commented May 26, 2026

Copy link
Copy Markdown
Member Author

Merge activity

…lexer hint (#22729)

## Summary

esbuild emits unreachable `0 && (module.exports = { ... })` statements on Node platform as parse-time hints for [`cjs-module-lexer`](https://github.com/nodejs/cjs-module-lexer). The actual exports happen through helper calls (`__export(...)` or similar) that the lexer can't trace, so it scans for the literal `module.exports = { ... }` shape inside this dead-code expression to discover named exports.

Exact emission site (esbuild v0.28.0): [`internal/linker/linker.go#L5127-L5138`](https://github.com/evanw/esbuild/blob/v0.28.0/internal/linker/linker.go#L5127-L5138). Always `LogicalAnd(0, Assign(module.exports, ObjectExpression))` — no other shape.

oxc's DCE judges the expression by JS-runtime semantics — `false && X` is unreachable, drop it — and is correct on those grounds. But this expression carries **extra-semantic information** the lexer reads. Dropping it silently breaks ESM consumers:

```js
import { clearRequireCache } from "@tailwindcss/node/require-cache";
//       ^^^^^^^^^^^^^^^^^
// SyntaxError: The requested module does not provide an export named 'clearRequireCache'
```

Observed in monitor-oxc CI: https://github.com/oxc-project/monitor-oxc/actions/runs/26427515847/job/77794148798

This is the same *class* of bug as #22475 (string quotes for `require()`) and #22552 (consolidated codegen helpers) — output properties that matter to downstream tooling, lost by minifier passes that only see JS semantics.

## Fix

### Commit 1: bailout in `try_fold_and_or`

Restores the guard added in #4878 and dropped during the #8618 refactor. In the constant-fold shortcut at `fold_constants.rs`, when `lval == false && op == And` and the right operand is a `module.exports = { ... }` assignment, return `None` instead of folding `0 && X → 0`.

### Commit 2: close remaining elimination paths

Code review surfaced two bypasses around the first guard:

1. **`remove_unused_logical_expr` (separate elimination path)** — when `treeshake.property_write_side_effects: false` (a documented rolldown/vite option), the assignment reports no side effects, the whole `0 && (module.exports = {...})` reports no side effects, and `try_fold_expression_stmt` → `remove_unused_expression` → `remove_unused_logical_expr` drops the entire ExpressionStatement before the fold-constants guard ever runs. Added a matching guard at `remove_unused_expression.rs`.

2. **Compound assignments + paren-wrapping** — the helper matched `module.exports ||= X` (never emitted) and didn't strip `ParenthesizedExpression` uniformly. Tightened to require `AssignmentOperator::Assign` and apply `get_inner_expression()` at the top.

## Helper scope

`is_cjs_module_exports_hint` matches **exactly esbuild's emission shape**: an `=` assignment with LHS `module.exports`. Verified against the source — esbuild never emits `exports.X = v`, `module.exports.X = v`, `||` form, or compound operators as hints.

Not handled (intentional):

- `0 && (exports.X = v)` — esbuild doesn't emit it.
- `0 && (module.exports.X.Y = v)` — esbuild doesn't emit it.
- Locally-shadowed `module`/`exports` — the lexical check over-preserves harmlessly. Threading `ctx.is_global_reference` through would expand the helper's surface for no observed bug.

## Tests

`test_preserve_cjs_module_lexer_hint` in `tests/peephole/fold_constants.rs` covers the positive shape + negative cases (compound assignments fold; non-export RHS folds).

Smoke-tested against `@tailwindcss/node/dist/require-cache.js`: hint line survives DCE.

Stacks on #22722.
@graphite-app graphite-app Bot force-pushed the fix/minifier-mark-changed-on-dead-stmt-drop branch from d46a46a to 40ff611 Compare May 26, 2026 05:31
graphite-app Bot pushed a commit that referenced this pull request May 26, 2026
…row statement (#22722)

## Summary

In `minimize_statements`, the dead-after-jump branch silently drops statements without flagging the peephole fixed-point loop as having changed. That leaves stale reference counts in `Scoping` until the next iteration kicks in for unrelated reasons — and in some cases, no further iteration ever happens, so the downstream cleanup never runs.

### Repro

```js
export function f() {
  var x = {};
  throw new Error('boom');
  module.exports = x;
}
```

Expected: `export function f() { throw new Error('boom'); }`.
Actual (before fix): `export function f() { var x = {}; throw new Error('boom'); }` — the dead `module.exports = x` is removed, but the now-orphaned `var x = {}` survives because the unused-declarator pass reads stale scope counts.

### Fix

In the dead-stmt-drop branch, set `ctx.state.changed = true` so the next iteration's `LiveUsageCollector` refreshes scope counts and unused-declarator cleanup runs.

To avoid an oscillation where dropping a bare `var x;` and immediately re-hoisting an identical `var x;` (via `KeepVar`) looks like progress, gate the flag on `dead_drop_mutates_ast`: drops of `var <id>[, <id>]*;` (no initializers) are byte-identical no-ops and don't flag.

Two related cases were also fixed in this commit after code review:

1. **Coalescing** — `throw; var x; var y;` → `throw; var x, y;`. Each individual identity-drop looked byte-identical, but the combined re-emit is a real change. Count identity drops; flag changed when more than one was harvested.
2. **Rehoist-then-remove** — when the rehoisted `var x;` is itself unused and `remove_unused_variable_declaration` drops it, the net effect is removing a declaration. Flag changed in that `None` branch.

### Tests

- New `dead_after_throw_drop_triggers_unused_declarator_removal` in `dead_code_elimination.rs`.
- All 505 existing minifier tests pass.

Stacked under #22729.
@graphite-app graphite-app Bot force-pushed the fix/minifier-preserve-cjs-module-lexer-hint branch from 007a708 to 4ac0fca Compare May 26, 2026 05:32
Base automatically changed from fix/minifier-mark-changed-on-dead-stmt-drop to main May 26, 2026 05:35
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 26, 2026
@graphite-app graphite-app Bot merged commit 4ac0fca into main May 26, 2026
31 checks passed
@graphite-app graphite-app Bot deleted the fix/minifier-preserve-cjs-module-lexer-hint branch May 26, 2026 05:36
Dunqing added a commit that referenced this pull request May 26, 2026
### 🚀 Features

- e857b0c napi/minify: Expose legalComments option and result (#20370)
(Boshen)
- 661132d parser: More friendly error messages for rest assignment
target and rest binding element (#22719) (sapphi-red)
- ee659b6 transformer/legacy-decorator: Add `strictNullChecks` option
for nullable-union design:type (#22266) (Kyle Cannon)

### 🐛 Bug Fixes

- e1d064e transformer/class-properties: Reparent lifted private method
helpers (#22716) (Cameron)
- 4ac0fca minifier: Preserve `0 && (module.exports = { ... })`
cjs-module-lexer hint (#22729) (Dunqing)
- 40ff611 minifier: Mark peephole loop changed when dropping
dead-after-throw statement (#22722) (Dunqing)
- 2f7b210 codegen: Emit pife-arrow/function leading comments inside the
wrap (#22720) (Dunqing)
- e184f74 parser: Improve invalid `import` property access diagnostic
(#22693) (camc314)
- 7baed9c transformer/private-method: Clear inherited strict flags
(#22508) (camc314)
- a9ad27e parser: Keep annotation comments leading without preceding
newline (#22711) (Dunqing)
- 9ea4d64 minifier: Re-evaluate pure/no-side-effects flags after
peephole inlining (#22595) (Dunqing)
- 07afbb6 minifier: Drop empty-body IIFE wrapper when called with
arguments (#22589) (Dunqing)
- fa7c463 semantic: Correct TS enum member symbol spans (#22689)
(camc314)
- 26b9396 semantic: Resolve parameter decorators outside parameter scope
(#22623) (camc314)
- b284045 parser: Switch to module goal eagerly on `export` (#22684)
(Boshen)
- dfa931d semantic: Propagate unresolved auto-increment enum value
instead of defaulting to 0 (#22646) (Dunqing)
- 69a6ba6 transformer/legacy-decorator: Emit Array for ReadonlyArray<T>
in decorator metadata (#22265) (Kyle Cannon)
- e421ef0 transformer/legacy-decorator: Return runtime binding for
design:type (#22640) (Dunqing)
- d61e1d7 codegen: Preserve verbatim text of pure/no-side-effects
comments (#22525) (Dunqing)
- 702b14e minifier: Preserve IIFE structure in DCE-only mode (#22547)
(Dunqing)
- 917da24 parser: Apply PURE comment through member-access chains
(#22566) (Dunqing)
- a069b1c codegen: Preserve quotes for cjs-module-lexer equality strings
(#22551) (Dunqing)

### ⚡ Performance

- 2f623b0 semantic: Skip unresolved checks for re-exports (#22660)
(camc314)
- 0d9553d semantic: Early-exit `check_object_expression` for objects
with <2 properties (#22668) (Dunqing)
- d721ad9 semantic: Use direct grandparent lookup for TS type parameters
(#22658) (camc314)
- 0aff288 semantic: Reorder numeric literal strict mode checks (#22657)
(camc314)
- 4d5ddb1 semantic: Reorder binding identifier checks (#22656) (camc314)
- e32acd8 semantic: Reorder identifier ambient binding check (#22653)
(camc314)
- 09fe178 semantic: Reorder ident reference strict mode check (#22652)
(camc314)
- 4b6add2 semantic: Avoid duplicate ident clone for bindings (#22663)
(camc314)
- 82f9662 parser: Check identifier kind before context flag (#22662)
(camc314)
- d7cd951 parser: Fast path identifier parsing and inline operator
helpers (#22650) (Boshen)
- 7b84314 semantic: Use direct byte access for numeric leading-zero
check (#22642) (camc314)
- 0345a31 semantic: Pre-size class elements hash map (#22618) (camc314)
- 04d3065 minifier: Drop per-call buffers in try_fold_concat (#22596)
(Dunqing)
- 4f289f1 semantic: Resolve_references_for_current_scope without a temp
Vec (#22599) (Dunqing)
- e862c15 semantic: Avoid heap alloc for var hoist scope ids (#22603)
(Dunqing)
- 8ff8674 semantic: Early return if `excess` is `0` in
`Stats::increase_by` (#22616) (camc314)
- 7a4120e semantic: Pre-reserve unresolved_references using
Stats::references (#22580) (Dunqing)

Co-authored-by: Dunqing <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant