Skip to content

refactor(codegen): consolidate cjs-module-lexer helpers into one module#22552

Merged
graphite-app[bot] merged 1 commit into
mainfrom
refactor/codegen-cjs-module-lexer-module
May 19, 2026
Merged

refactor(codegen): consolidate cjs-module-lexer helpers into one module#22552
graphite-app[bot] merged 1 commit into
mainfrom
refactor/codegen-cjs-module-lexer-module

Conversation

@Dunqing

@Dunqing Dunqing commented May 19, 2026

Copy link
Copy Markdown
Member

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.

@github-actions github-actions Bot added the A-codegen Area - Code Generation label May 19, 2026

Dunqing commented May 19, 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.

@codspeed-hq

codspeed-hq Bot commented May 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing refactor/codegen-cjs-module-lexer-module (f2aa30a) with fix/codegen-cjs-lexer-equality-strings (1fcdf7f)

Open in CodSpeed

Footnotes

  1. 3 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 force-pushed the refactor/codegen-cjs-module-lexer-module branch from 03af938 to f2aa30a Compare May 19, 2026 04:15
@Dunqing Dunqing marked this pull request as ready for review May 19, 2026 04:17
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label May 19, 2026

Dunqing commented May 19, 2026

Copy link
Copy Markdown
Member Author

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.
@graphite-app graphite-app Bot force-pushed the fix/codegen-cjs-lexer-equality-strings branch from 1fcdf7f to a069b1c Compare May 19, 2026 08:43
@graphite-app graphite-app Bot force-pushed the refactor/codegen-cjs-module-lexer-module branch from f2aa30a to 93f2213 Compare May 19, 2026 08:43
Base automatically changed from fix/codegen-cjs-lexer-equality-strings to main May 19, 2026 08:47
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 19, 2026
@graphite-app graphite-app Bot merged commit 93f2213 into main May 19, 2026
31 checks passed
@graphite-app graphite-app Bot deleted the refactor/codegen-cjs-module-lexer-module branch May 19, 2026 08:48
graphite-app Bot pushed a commit that referenced this pull request May 26, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-codegen Area - Code Generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants