refactor(codegen): consolidate cjs-module-lexer helpers into one module#22552
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
Merging this PR will not alter performance
Comparing Footnotes
|
03af938 to
f2aa30a
Compare
Merge activity
|
…le (#22552) ## Summary - Move all four `try_print_cjs_*` helpers — `require("…")`, `Object.defineProperty(_, "name", …)`, `exports[STR] = …`, and `key === "default"` / `key === "__esModule"` — into a new `cjs_module_lexer` module. - Add a module-level doc explaining the shared root cause (`calculate_quote_maybe_backtick`'s backtick tie-breaker that the lexer's heuristic parser doesn't accept) and listing all four positions in one table, so the next workaround has an obvious home. - Names drop the redundant `cjs_` prefix; call sites use `cjs_module_lexer::try_print_…`. No behavior change. Stacked on top of #22551. AI disclosure: drafted with Claude Code, reviewed manually.
1fcdf7f to
a069b1c
Compare
f2aa30a to
93f2213
Compare
…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.

Summary
try_print_cjs_*helpers —require("…"),Object.defineProperty(_, "name", …),exports[STR] = …, andkey === "default"/key === "__esModule"— into a newcjs_module_lexermodule.calculate_quote_maybe_backtick's backtick tie-breaker that the lexer's heuristic parser doesn't accept) and listing all four positions in one table, so the next workaround has an obvious home.cjs_prefix; call sites usecjs_module_lexer::try_print_…. No behavior change.Stacked on top of #22551.
AI disclosure: drafted with Claude Code, reviewed manually.