refactor(transformer/legacy-decorator): eliminate unreliable identification of metadata #13227
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
A-transformer
Member
Author
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. |
Dunqing
changed the title
CodSpeed Instrumentation Performance ReportMerging #13227 will not alter performanceComparing Summary
Footnotes |
Dunqing
force-pushed
the
08-20-refactor_transformer_decorator_eliminate_unreliable_identification_of_metadata
branch
from
16f8e20 to
3c27130
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the legacy decorator metadata handling by replacing unreliable span-based identification with a proper collection and storage system. Instead of checking span.is_unspanned() to identify metadata decorators, the code now collects metadata in a VecDeque during class traversal and retrieves it when transforming class decorators.
- Introduces a
VecDeque-based metadata storage system usingNonEmptyStack - Replaces span-based metadata decorator identification with proper collection/retrieval
- Adds support for constructor parameter metadata handling
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_transformer/src/decorator/legacy/metadata.rs |
Implements the new metadata collection system with VecDeque storage and MethodMetadata enum |
crates/oxc_transformer/src/decorator/legacy/mod.rs |
Updates decorator transformation logic to use the new metadata retrieval system |
tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/params/input.ts |
Adds test case with constructor parameter decorator |
tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/params/output.js |
Expected output for the new test case |
tasks/transform_conformance/snapshots/oxc.snap.md |
Updates test snapshots to reflect the new behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Member
Merge activity
|
…cation of metadata (#13227) Fixes: #13215 (review) Collect metadata and store them in a `VecDeque`, and take them when transforming the class decorators in the `exit_class` side, which can eliminate the unreliable check of `span.is_unspanned` for metadata decorators. However, it introduces a little complexity.
graphite-app
bot
force-pushed
the
08-20-refactor_transformer_decorator_eliminate_unreliable_identification_of_metadata
branch
from
3c27130 to
e7a49ed
Compare
graphite-app
bot
deleted the
08-20-refactor_transformer_decorator_eliminate_unreliable_identification_of_metadata
branch
graphite-app bot
pushed a commit
that referenced
this pull request
Sep 11, 2025
…or parameter decorators (#13632) Fixes rolldown/rolldown#6079. Regressed introduced by #13227. The root cause is that the class plugin will insert a constructor method in some cases, but metadata handling is processed before the constructor is inserted, which causes the method count not to be exactly the same as the method metadata count. Thus, decorator metadata is inserted in the incorrect place. This PR refactors the core transformation logic to ensure that the new methods and properties inserted by other plugins won't affect the decorator transformation. ### Transformation Flow Changes Before: Complex upfront analysis with separate processing paths - Used check_class_has_decorated() to analyze entire class tree - Separate handling for class vs element decorators - Multiple disconnected stacks for state tracking After: Unified incremental processing - Process decorators as we traverse (exit_method_definition, exit_property_definition, etc.) - Accumulate decoration statements in single stack during traversal - Unified processing in transform_class() using consolidated state
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #13215 (review)
Collect metadata and store them in a
VecDeque, and take them when transforming the class decorators in theexit_classside, which can eliminate the unreliable check ofspan.is_unspannedfor metadata decorators. However, it introduces a little complexity.