Skip to content

fix(codegen): preserve string quotes in require() calls during minification#22475

Merged
Dunqing merged 3 commits into
oxc-project:mainfrom
zennnnnnn11:fix/codegen-require-backtick
May 18, 2026
Merged

fix(codegen): preserve string quotes in require() calls during minification#22475
Dunqing merged 3 commits into
oxc-project:mainfrom
zennnnnnn11:fix/codegen-require-backtick

Conversation

@zennnnnnn11

@zennnnnnn11 zennnnnnn11 commented May 15, 2026

Copy link
Copy Markdown
Contributor

Closes #22463

Summary

  • Add try_print_require_call to force plain string literals (no backticks) for require("...") calls during minification
  • Prevents cjs-module-lexer from failing to detect CJS reexport sources

Problem

When minifying, StringLiteral::gen passes allow_backtick: true, which can convert:

__exportStar(require("./decorators"), exports);

into:

__exportStar(require(`./decorators`),exports);

Node.js uses cjs-module-lexer for CJS/ESM interop. This lexer uses a fast heuristic parser that does not recognize template literals as require() arguments, causing named reexports to silently disappear from the module's export list.

Fix

Add try_print_require_call (following the existing try_print_cjs_define_property_call pattern) that detects require("...") calls via the existing CallExpression::common_js_require() helper and prints the string argument with allow_backtick: false.

Test plan

  • require("./decorators") stays quoted when minified
  • require('./foo') normalizes to double quotes (not backticks)
  • require(foo) (dynamic) is unaffected
  • Non-require calls like foo("./bar") still get backtick optimization
  • cargo test -p oxc_codegen passes
  • Follows existing CJS special-case pattern in codegen

🤖 Generated with Claude Code

@camc314 camc314 added the A-codegen Area - Code Generation label May 17, 2026

@Dunqing Dunqing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

zennnnq and others added 2 commits May 18, 2026 08:13
…fication

When minifying, the codegen could convert `require("./foo")` to
`require(`./foo`)` using backtick template literals. Node.js's
`cjs-module-lexer` cannot detect reexport sources with template
literal arguments, breaking ESM imports of CJS named exports.

Add `try_print_require_call` that forces `allow_backtick: false` on
`require()` string arguments, following the same pattern as the
existing `try_print_cjs_define_property_call`.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add tests for dynamic require (not affected) and single-quoted
require (normalized to double quotes).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@Dunqing Dunqing force-pushed the fix/codegen-require-backtick branch from 71637d6 to b224597 Compare May 18, 2026 00:13
@codspeed-hq

codspeed-hq Bot commented May 18, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing zennnnnnn11:fix/codegen-require-backtick (43c44df) with main (a9049fd)

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.

- Clippy needless_raw_string_hashes on single-quoted require test.
- Update tasks/minsize/minsize.snap: gzip size for antd.js and typescript.js each grew by 1 byte after require(...) stopped collapsing to backticks.
@Dunqing Dunqing merged commit b9615b2 into oxc-project:main May 18, 2026
36 checks passed
camc314 pushed a commit that referenced this pull request May 18, 2026
### 🐛 Bug Fixes

- 0f26de6 ecmascript: Resolve identifier value type via tracked
constants (#22234) (Alexander Lichter)
- c27a8cf minifier: Normalize `{ x: x }` shorthand so adjacent-if merge
is idempotent (#22401) (Dunqing)
- e431a0e parser: Break extends clause loop on fatal error (#22517)
(Boshen)
- e9ec7c6 minifier: Fold optional chains by base nullishness (#22236)
(Alexander Lichter)
- e6090e7 transformer: Keep enum IIFE when a non-inlinable value
reference remains (#22501) (Dunqing)
- 931b7d6 transformer: Inline const enum members through type-cast
wrappers (#22500) (Dunqing)
- b9615b2 codegen: Preserve string quotes in require() calls during
minification (#22475) (zennnnnnn11)
- c73c159 transformer/async-to-generator: Reparent parameter initializer
scopes (#22507) (camc314)
- ecfd3ca transformer/async-to-generator: Move only parameter bindings
(#22503) (camc314)
- 3ce3431 transformer/explicit-resource-managment: Preserve shadowed
for-head block (#22451) (camc314)

### ⚡ Performance

- ce92c6c semantic: `#[inline]` `Scoping::get_binding` (#22414)
(Dunqing)
- 98be95c regular_expression: Track regex flags via bitflags (#22427)
(Boshen)
- dbbc059 jsdoc: Skip should_attach_jsdoc when no remaining comments
(#22409) (Boshen)
- 217d7d8 minifier: Index `SymbolValues` by `SymbolId` (#22441)
(Dunqing)
- d782b78 minifier: Use BitSet for LiveUsageCollector live references
(#22425) (Boshen)
graphite-app Bot pushed a commit that referenced this pull request May 19, 2026
…22551)

## Summary

- Force plain string quotes for `"default"` / `"__esModule"` operands of `==`/`===`/`!=`/`!==` during minification, so `cjs-module-lexer` can recognise the `Object.keys(mod).forEach(key => key === "default" || ...)` re-export filter emitted by Babel / TypeScript / Rollup.
- Follow-up to #22475 (which fixed `require("…")`). Same root cause: `calculate_quote_maybe_backtick`'s equal-cost tie-breaker prefers backtick, which the lexer's heuristic parser doesn't accept.
- Caught by monitor-oxc's `whitespace` runner: minifying `@babel/types/lib/index.js` previously broke `import { isIdentifier } from "@babel/types"` under `--experimental-require-module`. Raw minified size is unchanged across all `minsize` fixtures.

## Example

Input:

```js
Object.keys(mod).forEach(function (key) {
  if (key === "default" || key === "__esModule") return;
});
```

Before (broken — `cjs-module-lexer` skips the re-export):

```js
Object.keys(t).forEach(function(e){if(e===`default`||e===`__esModule`)return});
```

After:

```js
Object.keys(t).forEach(function(e){if(e==="default"||e==="__esModule")return});
```

AI disclosure: drafted with Claude Code, reviewed manually.
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.

codegen: minified CommonJS require() with backticks breaks Node named export detection

3 participants