fix(ecmascript): mark new Symbol as non side-effect free#17568
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. |
There was a problem hiding this comment.
Pull request overview
This PR fixes the incorrect classification of new Symbol as side-effect free. According to the ECMAScript specification, Symbol is not a constructor and throws a TypeError when invoked with the new operator, making it a side effect that must be preserved during minification.
- Removed
Symbolfrom theis_pure_constructorfunction to correctly identifynew Symbolas having side effects - Updated test expectations to reflect that
new Symbolshould be marked as having side effects (true), whileSymbol()withoutnewremains side-effect free
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_ecmascript/src/side_effects/expressions.rs | Removed Symbol from the is_pure_constructor list since it throws when used with new |
| crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs | Moved new Symbol test from side-effect free (false) to has side effects (true) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #17568 will not alter performanceComparing Summary
Footnotes
|
Merge activity
|
 `new Symbol` throws an error
0262b49 to
1044116
Compare
### 🚀 Features - 659c23e linter: Init note field boilerplate (#17589) (Shrey Sudhir) - 6870b64 parser: Add TS1363 error code (#17609) (Sysix) - 23680a3 mangler: Skip mangling only in scopes affected by direct eval (#17612) (camc314) - a7e1643 parser: Add TS2528 error code to duplicate_default_export diagnostic (#17558) (camc314) ### 🐛 Bug Fixes - 1044116 ecmascript: Mark `new Symbol` as non side-effect free (#17568) (camc314) - ab5e4ca isolated-declarations: Strip default values from rest parameter binding patterns (#17602) (camc314) - 68b2e54 minifier: Prevent incorrect ??= transformation when member base is mutated (#17472) (copilot-swe-agent) ### ⚡ Performance - 6067143 semantic: Remove hash when checking identifier (#17564) (camchenry) - a28ab3d semantic: Avoid bounds check when checking string literal (#17545) (camc314) - 04809d1 semantic: Use SIMD for finding backslashes in `check_string_literal` (#17534) (camchenry) - 49ad2f0 semantic: Mark all diagnostic functions as `#[cold]` (#17487) (camc314) - ea82b50 transformer: Mark all diagnostic functions as `#[cold]` (#17486) (camc314) - d968e51 semantic: Mark `checker::check` as `inline(always)` (#17459) (camc314)
new Symbolthrows an error