Skip to content

fix(minifier): treat object spread of getters as having side effects#20380

Merged
Boshen merged 2 commits intomainfrom
fix/object-spread-getter-side-effects
Mar 14, 2026
Merged

fix(minifier): treat object spread of getters as having side effects#20380
Boshen merged 2 commits intomainfrom
fix/object-spread-getter-side-effects

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Mar 14, 2026

Summary

  • Object spread (...{ get prop() {} }) invokes getters at runtime, so it has side effects
  • The previous code only checked key/value AST nodes and missed the getter invocation
  • Setters are not invoked by spread, so they remain side-effect-free
  • Fixes the Rolldown test rollup@function@object-spread-side-effect

🤖 Generated with Claude Code

Spreading an object literal containing a getter invokes the getter at
runtime, so it must be treated as having side effects. The previous
logic only checked key/value nodes and missed the getter invocation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 14, 2026 14:34
@github-actions github-actions bot added A-minifier Area - Minifier C-bug Category - Bug labels Mar 14, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the minifier’s side-effect analysis for object spread so that spreading an object literal with a getter is treated as side-effectful (since the getter is invoked at runtime), matching Rollup/Rolldown expectations.

Changes:

  • Add regression tests asserting object spread invokes getters (but not setters).
  • Update MayHaveSideEffects for object spread to treat get properties as side-effectful during spreading.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs Adds tests for getter/setter behavior when spreading object literals.
crates/oxc_ecmascript/src/side_effects/expressions.rs Updates spread side-effect logic to flag getters as side-effectful during object spread.

You can also share your feedback on Copilot code review. Take the survey.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6d6b4c301

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 14, 2026

Merging this PR will not alter performance

✅ 53 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing fix/object-spread-getter-side-effects (430cffd) with main (f8fbd6e)2

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.

  2. No successful run was found on main (618a598) during the generation of this report, so f8fbd6e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Boshen Boshen merged commit e62524d into main Mar 14, 2026
29 checks passed
@Boshen Boshen deleted the fix/object-spread-getter-side-effects branch March 14, 2026 15:04
camc314 pushed a commit that referenced this pull request Mar 16, 2026
### 🐛 Bug Fixes

- edb8677 ecmascript: Treat collection constructor with variable arg as side-effectful (#20383) (Dunqing)
- 1f65c3f transformer: Emit design:paramtypes when class has static anonymous class expression (#20382) (bab)
- fa70d5c transformer: Use implementation signature for design:paramtypes when constructor is overloaded (#20394) (bab)
- ed5a7fb parser: Report syntax error for `new super()` (#20384) (Boshen)
- e62524d minifier: Treat object spread of getters as having side effects (#20380) (Boshen)
- f8fbd6e linter/plugins: Remove `hashbang` property from AST (#20365) (overlookmotel)

### ⚡ Performance

- 30a2b0f minifier: Use atom_from_strs_array for template literal concat (#20386) (Boshen)
- 690ce17 minifier: Use Vec::with_capacity for inline template expressions (#20389) (Boshen)
- 9cd612f linter/plugins: Recycle comment objects (#20362) (overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants