fix(codegen): keep Object.defineProperty property name as plain string in minify#22400
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
|
2b7165a to
05353f5
Compare
|
If we must continue using |
Merge activity
|
…ing in minify (#22400) Fix `cjs-module-lexer` failing to detect `Object.defineProperty(exports, "name", ...)` named exports after oxc minify. `calculate_quote_maybe_backtick` favored backtick when all three quote costs tied at zero, so a plain ASCII property name like `"getInclusionReasons"` was emitted as a backtick template. [`cjs-module-lexer`](https://github.com/nodejs/cjs-module-lexer) (used by Node for CJS/ESM interop) only matches plain string literals there, so the affected exports silently dropped out of its export set — downstream ESM consumers then failed with `Named export 'X' not found`. In `CallExpression::gen_expr`, detect `Object.defineProperty` / `Reflect.defineProperty` and print the 2nd argument via `print_string_literal(name, false)` (the existing `allow_backtick: false` path). Other arguments — and nested strings inside the descriptor — still benefit from backtick optimization. Minify-only: outside minify, `print_string_literal` uses `self.quote` (never backtick), and `print_arguments` is needed there to preserve argument comments. ### Before / after ```js // input Object.defineProperty(exports, "getInclusionReasons", { ... }); ``` ```js // before Object.defineProperty(exports,`getInclusionReasons`,{...}); // after Object.defineProperty(exports,"getInclusionReasons",{...}); ``` Closes #22342 AI assisted.
05353f5 to
18edc2c
Compare
…22402) Stacked on #22400. Closes the second `cjs-module-lexer` gap. After #22400, `Object.defineProperty(exports, "name", ...)` is fine, but `exports["name"] = …` / `module.exports["name"] = …` were still being emitted with backtick keys in minify. The lexer recognises this form too — it just couldn't see them past the template literal. Valid-identifier keys (`exports["foo"]`) are already collapsed to dot access by the minifier earlier in the pipeline, so only non-identifier keys (`exports["has-dash"]`, `module.exports["__esModule"]`, …) survive to the codegen stage and were affected. In `AssignmentExpression::gen_expr`, detect when the LHS is a computed member expression on `exports` / `module.exports` whose key is a `StringLiteral`, and print the key via `print_string_literal(key, false)`. Other computed access (`otherObj["x"]`) is unaffected. ### Where each cjs-module-lexer named-export form lands | Pattern | Status | |---|---| | `exports.name = …` | identifier — never affected | | `module.exports.name = …` | identifier — never affected | | `module.exports = { "name": … }` | object property keys already use `allow_backtick: false` | | `Object.defineProperty(exports, "name", …)` | fixed in #22400 | | `exports["name"] = …` / `module.exports["name"] = …` | **fixed here** | ### Before / after ```js // input exports["has-dash"] = a; module.exports["__esModule"] = true; otherObj["not-exports"] = d; ``` ```js // before exports[`has-dash`]=a,module.exports[`__esModule`]=!0,otherObj[`not-exports`]=d; // after exports["has-dash"]=a,module.exports.__esModule=!0,otherObj[`not-exports`]=d; ``` (`module.exports.__esModule` collapses to dot via a separate minifier pass; `otherObj["x"]` correctly stays backtick.) Refs #22342 AI assisted.
### 🚀 Features - bc91a17 codegen: Expose `Codegen::with_source_type` method (#22432) (camc314) ### 🐛 Bug Fixes - 5ac7e79 minifier: Drop unused-var-init pure IIFEs and preserve annotation for downstream (#22349) (Dunqing) - 4ab57eb allocator: Fixed-size allocators use `VirtualAlloc` on Windows (#22124) (overlookmotel) - 66d77eb allocator: Fix segfault on Linux MUSL with fixed-size allocators (#22388) (overlookmotel) - b8fbc1f transformer/object-rest-spread: Correct scope id when moving bindings (#22419) (camc314) - 18edc2c codegen: Keep `Object.defineProperty` property name as plain string in minify (#22400) (Dunqing) - dda33de transformer/explicit-resource-management: Align lexical binding scopes (#22320) (camc314) - 8e79de8 transformer: Preserve for-await statement bodies (#22361) (camc314) - 0cba210 transformer/class: Replace `new.target` in static blocks (#22360) (camc314) - 67ab1c9 transformer/es2018/for-await: Hoist for-await generated bindings (#22355) (camc314) - c3ceb4a transformer/object-rest-spread: Use hoisted scope for `for-of` temp refs (#22347) (camc314) ### ⚡ Performance - 73a9043 allocator/bitset: Avoid temp heap `String` allocation (#22403) (camc314) - 8b2f4f9 transformer/object-rest-spread: Collect `Vec<SymbolId` over `Vec<BindingIdentifier>` (#22418) (camc314) - 83679ea parser: Split TriviaBuilder::handle_token hot/cold paths (#22415) (Boshen) - 2c7d781 codegen: Inline identifier-name accessors (#22411) (Boshen) - 618bc76 diagnostics: Inline `OxcDiagnosticInner` to avoid heap allocation (#22406) (Boshen) - 0b4e158 parser: Reserve cap `2` for sequence expressions vec (#22374) (camc314) - 5f3bdd0 codegen: Add `#[inline]` to `code`, `code_len` (#22373) (camc314) Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>

Fix
cjs-module-lexerfailing to detectObject.defineProperty(exports, "name", ...)named exports after oxc minify.calculate_quote_maybe_backtickfavored backtick when all three quote costs tied at zero, so a plain ASCII property name like"getInclusionReasons"was emitted as a backtick template.cjs-module-lexer(used by Node for CJS/ESM interop) only matches plain string literals there, so the affected exports silently dropped out of its export set — downstream ESM consumers then failed withNamed export 'X' not found.In
CallExpression::gen_expr, detectObject.defineProperty/Reflect.definePropertyand print the 2nd argument viaprint_string_literal(name, false)(the existingallow_backtick: falsepath). Other arguments — and nested strings inside the descriptor — still benefit from backtick optimization. Minify-only: outside minify,print_string_literalusesself.quote(never backtick), andprint_argumentsis needed there to preserve argument comments.Before / after
Closes #22342
AI assisted.