fix: accessor with decorators should be kept by Copilot · Pull Request #7499 · rolldown/rolldown

Copilot · 2025-12-15T03:49:28Z

Fix dce_of_decorators test - Add Accessor Decorator Support

Summary

This PR fixes the dce/dce_of_decorators test by adding missing decorator detection for AccessorProperty in the side effect detector. Accessor and static accessor decorators are now correctly preserved during dead code elimination.

Changes Made

Added decorator check for AccessorProperty (crates/rolldown/src/ast_scanner/side_effect_detector/mod.rs)
- The detect_side_effect_of_class function now checks if AccessorProperty has decorators, similar to existing checks for MethodDefinition and PropertyDefinition
Added comprehensive unit tests (crates/rolldown/src/ast_scanner/side_effect_detector/mod.rs)
- Tests verify that all decorator types are correctly detected as having side effects
- Covers class decorators, method decorators, field decorators, and accessor decorators
Updated test expectations (crates/rolldown/tests/esbuild/dce/dce_of_decorators/artifacts.snap)
- Snapshot now includes Accessor and StaticAccessor classes that were previously incorrectly removed
Updated esbuild diff files (just ued)
- Diff now accurately reflects that accessor decorators are preserved
- Summary updated to reflect remaining known limitation

Test Results

✅ Accessor decorators now work correctly:

class MyClass { @fn accessor x } - kept
class MyClass { @fn static accessor x } - kept

✅ All existing decorator functionality preserved:

Method decorators - still work
Field decorators - still work

⚠️ Known limitation (documented):

Class-level decorators on unused declarations (e.g., @fn class Class {}) still removed - requires deeper investigation into tree-shaking logic

Impact

This fix ensures that accessor decorators (a TC39 Stage 3 feature) are properly handled during tree-shaking, bringing Rolldown closer to feature parity with esbuild for the new decorator proposal.

Original prompt

Fix this test:

rolldown/scripts/src/esbuild-tests/reasons.ts

Line 5 in 92cac69

'dce/dce_of_decorators': 'decorators should not be removed',

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

netlify · 2025-12-15T03:50:28Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`7f005d9`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/693f938079d4100008f3e3a8

Co-authored-by: sapphi-red <[email protected]>

crates/rolldown/src/ast_scanner/side_effect_detector/mod.rs

Signed-off-by: 翠 <[email protected]>

Copilot

Pull request overview

This PR fixes a bug in the side effect detector where AccessorProperty elements with decorators were not being detected as having side effects, causing classes with accessor decorators to be incorrectly removed during dead code elimination (DCE). The fix adds a decorator check for AccessorProperty that matches the existing pattern used for MethodDefinition and PropertyDefinition.

Key changes:

Added early-return decorator check for AccessorProperty in detect_side_effect_of_class()
Added comprehensive unit tests covering all decorator types (class, method, field, accessor)
Updated test snapshots to reflect that accessor properties with decorators are now correctly preserved

Reviewed changes

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

Show a summary per file

File	Description
`crates/rolldown/src/ast_scanner/side_effect_detector/mod.rs`	Added decorator check for `AccessorProperty` (lines 127-129) and unit tests for decorator side effect detection
`crates/rolldown/tests/esbuild/dce/dce_of_decorators/artifacts.snap`	Updated snapshot showing `Accessor` and `StaticAccessor` classes are now preserved in output
`crates/rolldown/tests/esbuild/lower/java_script_decorators_bundle_issue3768/artifacts.snap`	Updated snapshot showing all accessor property variants with decorators are preserved
`crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap`	Hash updates reflecting changed output content for tests with accessor decorators
`scripts/src/esbuild-tests/reasons.ts`	Updated failure reason to be more specific about the remaining limitation with class-level decorators

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sapphi-red · 2025-12-15T04:47:08Z

@copilot update the esbuild diff with just ued

github-actions · 2025-12-15T04:47:53Z

Benchmarks Rust

target: main(92cac69)
pr: copilot/fix-esbuild-test-issue(7f005d9)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.08     68.8±2.32ms        ? ?/sec    1.00     64.0±1.68ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     72.1±2.16ms        ? ?/sec    1.02     73.4±1.82ms        ? ?/sec
bundle/bundle@rome_ts                                        1.02    111.2±1.83ms        ? ?/sec    1.00    108.7±2.23ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.02    124.1±2.33ms        ? ?/sec    1.00    121.8±2.95ms        ? ?/sec
bundle/bundle@threejs                                        1.01     40.7±1.05ms        ? ?/sec    1.00     40.2±1.06ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     43.9±0.85ms        ? ?/sec    1.00     43.7±1.04ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    398.7±6.49ms        ? ?/sec    1.02    405.7±8.56ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    454.5±3.77ms        ? ?/sec    1.01    458.4±7.43ms        ? ?/sec
scan/scan@rome_ts                                            1.03     87.1±1.80ms        ? ?/sec    1.00     84.6±1.80ms        ? ?/sec
scan/scan@threejs                                            1.00     28.8±0.89ms        ? ?/sec    1.01     29.1±1.84ms        ? ?/sec
scan/scan@threejs10x                                         1.01    303.2±5.97ms        ? ?/sec    1.00    299.2±4.94ms        ? ?/sec

Co-authored-by: sapphi-red <[email protected]>

Copilot · 2025-12-15T04:50:15Z

@copilot update the esbuild diff with just ued

Updated the esbuild diff with just ued in commit 7f005d9. The diff now accurately reflects that Accessor and StaticAccessor classes with decorators are preserved.

@sevenc-nanashi

## [1.0.0-beta.55] - 2025-12-17 ### 🚀 Features - add validation errors for incompatible `inlineDynamicImports` options (#7539) by @Copilot - rolldown_plugin_vite_reporter: truncate long importer lists in ineffective dynamic import warnings (#7528) by @Copilot - export `memfs` from `rolldown/experimental` for browser builds (#7490) by @Copilot - implement `postBanner` and `postFooter` (#7487) by @sevenc-nanashi - port getLogFilter helper from Rollup for advanced log filtering (#6890) by @taearls - dev: initialize `rolldown_plugin_lazy_compilation` (#7488) by @hyf0 - enable `experimental.transform_hires_sourcemap: 'boundary'` by default (#7478) by @sapphi-red - apply merge CJS namespace optimizations more generally (#7475) by @sapphi-red ### 🐛 Bug Fixes - avoid panic on invalid values for `output.exports`, `output.format`, `output.hash_characters` (#7542) by @sapphi-red - handle `__proto__` export for module namespace correctly (#7534) by @sapphi-red - handle `__proto__` export correctly (#7533) by @sapphi-red - rolldown_plugin_vite_resolve: acquire resolver lock before package json resolution (#7524) by @sapphi-red - disable syntax transform optimization for `minify: 'dce-only'` (#7520) by @sapphi-red - arrow function expression keep names (#7519) by @IWANABETHATGUY - skip symbols that are imported from other module when deconflicting module symbols (#7510) by @IWANABETHATGUY - preserve class names in assignment expressions with keepNames option (#7491) by @IWANABETHATGUY - accessor with decorators should be kept (#7499) by @Copilot - rolldown_plugin_vite_reporter: support `Infinity` for chunk_limit (#7497) by @shulaoda - change filename template validation errors from UNHANDLEABLE_ERROR to INVALID_OPTION (#7472) by @Copilot - `no entry found for key` error when merging CJS namespace exports (#7474) by @sapphi-red - generate output with syntax error when wrapped esm module using tla syntax (#7468) by @IWANABETHATGUY ### 🚜 Refactor - dev: rename `experimental.hmr` to `experimental.devMode` (#7527) by @hyf0 - pass addon option to rust as it is (#7526) by @hyf0 - remove unused stmtinfo meta flag (#7518) by @IWANABETHATGUY ### 📚 Documentation - clarify `closeBundle` behavior (#7525) by @sapphi-red ### 🧪 Testing - add tests for static string postBanner and postFooter (#7516) by @Copilot - disable pluginTimings in Rust integration tests to avoid snapshot noise (#7485) by @shulaoda - port Rollup JSX tests to `crates/rolldown/tests/rollup` (#7480) by @sapphi-red - re-triage some esbuild JSX preserve tests (#7479) by @sapphi-red - disable pluginTimings by default to avoid snapshot noise (#7471) by @shulaoda ### ⚙️ Miscellaneous Tasks - mark feature that will not be supported as ignored in rollup test status (#7535) by @sapphi-red - support `banner`/`footer`/`intro`/`outro` for config variants (#7532) by @sapphi-red - `dce/dce_of_decorators` esbuild test is now passing (#7531) by @sapphi-red - deps: update github-actions (major) (#7495) by @renovate[bot] - deps: update npm packages (#7493) by @renovate[bot] - add a check to verify failedReasons are updated (#7521) by @sapphi-red - deps: update test262 submodule for tests (#7523) by @sapphi-red - deps: update taiki-e/install-action action to v2.63.2 (#7496) by @renovate[bot] - deps: update oxc apps (#7511) by @renovate[bot] - deps: update oxc to v0.103.0 (#7513) by @camc314 - fix format in `.github/instructions/bug-investigation.instructions.md` (#7506) by @sapphi-red - add instructions for REPL decoding (#7502) by @sapphi-red - deps: update github-actions (#7492) by @renovate[bot] - deps: cargo-shear v1.9.0 (#7483) by @Boshen - deps: update dependency oxlint-tsgolint to v0.9.0 (#7484) by @renovate[bot] - deps: update dependency oxlint-tsgolint to v0.8.6 (#7470) by @renovate[bot] - add syntax validation for test output chunks that are not executed (#7466) by @IWANABETHATGUY - deps: update dependency rust to v1.92.0 (#7467) by @renovate[bot] - deps: update test262 submodule for tests (#7457) by @sapphi-red ### ❤️ New Contributors * @sevenc-nanashi made their first contribution in [#7487](#7487) * @taearls made their first contribution in [#6890](#6890) Co-authored-by: shulaoda <[email protected]>

Initial plan

3fb9d02

Copilot AI assigned Copilot and sapphi-red Dec 15, 2025

Copilot started work on behalf of sapphi-red December 15, 2025 03:49 View session

Copilot AI and others added 4 commits December 15, 2025 03:59

Add decorator check for AccessorProperty in side effect detector

83cc0c3

Co-authored-by: sapphi-red <[email protected]>

Remove dce_of_decorators from failedReasons

64dfc57

Co-authored-by: sapphi-red <[email protected]>

Add test for class decorators and document remaining issue

9d5f0b6

Co-authored-by: sapphi-red <[email protected]>

Expand decorator tests to cover all decorator types

28dbe9d

Co-authored-by: sapphi-red <[email protected]>

Copilot AI changed the title ~~[WIP] Fix test in esbuild reasons file~~ Fix decorator side effect detection for AccessorProperty Dec 15, 2025

Copilot finished work on behalf of sapphi-red December 15, 2025 04:22

Copilot AI requested a review from sapphi-red December 15, 2025 04:22

sapphi-red reviewed Dec 15, 2025

View reviewed changes

crates/rolldown/src/ast_scanner/side_effect_detector/mod.rs Outdated Show resolved Hide resolved

chore: update

fab9f39

Signed-off-by: 翠 <[email protected]>

sapphi-red marked this pull request as ready for review December 15, 2025 04:33

Copilot AI review requested due to automatic review settings December 15, 2025 04:33

sapphi-red changed the title ~~Fix decorator side effect detection for AccessorProperty~~ fix: accessor with decorators should be kept Dec 15, 2025

Copilot started reviewing on behalf of sapphi-red December 15, 2025 04:33 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

IWANABETHATGUY approved these changes Dec 15, 2025

View reviewed changes

Copilot started work on behalf of sapphi-red December 15, 2025 04:47 View session

Update esbuild diff files to reflect accessor decorator fix

7f005d9

Co-authored-by: sapphi-red <[email protected]>

Copilot finished work on behalf of sapphi-red December 15, 2025 04:51

Copilot AI requested a review from sapphi-red December 15, 2025 04:51

sapphi-red enabled auto-merge (squash) December 15, 2025 05:00

sapphi-red merged commit d9804e1 into main Dec 15, 2025
35 of 45 checks passed

sapphi-red deleted the copilot/fix-esbuild-test-issue branch December 15, 2025 05:08

This was referenced Dec 17, 2025

release: v1.0.0-beta.55 #7536

Closed

release: v1.0.0-beta.55 #7543

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

fix: accessor with decorators should be kept#7499

fix: accessor with decorators should be kept#7499
sapphi-red merged 7 commits intomainfrom
copilot/fix-esbuild-test-issue

Copilot AI commented Dec 15, 2025 •

edited

Loading

Uh oh!

netlify bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

sapphi-red commented Dec 15, 2025

Uh oh!

github-actions bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

Copilot AI commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

Copilot AI commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix dce_of_decorators test - Add Accessor Decorator Support

Summary

Changes Made

Test Results

Impact

Uh oh!

netlify bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs canceled.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sapphi-red commented Dec 15, 2025

Uh oh!

github-actions bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

Uh oh!

Copilot AI commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 15, 2025 •

edited

Loading

netlify bot commented Dec 15, 2025 •

edited

Loading

github-actions bot commented Dec 15, 2025 •

edited

Loading