Skip to content

fix(transformer/legacy-decorator): abstract class doesn't work in metadata#10952

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata
May 12, 2025
Merged

fix(transformer/legacy-decorator): abstract class doesn't work in metadata#10952
graphite-app[bot] merged 1 commit intomainfrom
05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented May 12, 2025

close: #10926

Copy link
Member Author

Dunqing commented May 12, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-bug Category - Bug labels May 12, 2025
@Dunqing Dunqing marked this pull request as ready for review May 12, 2025 04:21
Copilot AI review requested due to automatic review settings May 12, 2025 04:21
@Dunqing Dunqing requested a review from overlookmotel as a code owner May 12, 2025 04:21
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 an issue in the legacy decorator transformation for abstract classes by updating type checks and ensuring proper metadata handling.

  • Updated conditional checks in both decorator and metadata modules to use the new "declare" property.
  • Adjusted test fixtures and updated snapshots to verify the correct behavior of abstract classes.

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
tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/abstrct-class/output.ts Transpiled output for abstract class metadata handling
tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/abstrct-class/input.ts Input fixture using abstract class syntax with legacy decorators
tasks/transform_conformance/snapshots/oxc.snap.md Updated snapshot to reflect changes in metadata processing
crates/oxc_transformer/src/decorator/legacy/mod.rs Adjusted class check condition to include "class.declare"
crates/oxc_transformer/src/decorator/legacy/metadata.rs Modified metadata processing condition to use "class.declare"
Comments suppressed due to low confidence (3)

tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/abstrct-class/input.ts:1

  • [nitpick] Consider renaming the test fixture directory/file from 'abstrct-class' to 'abstract-class' for clarity, unless the current naming is intentional for legacy support.
import { dce, type Dependency } from "mod";

crates/oxc_transformer/src/decorator/legacy/mod.rs:811

  • Verify that using 'class.declare' appropriately replaces 'class.is_typescript_syntax()' to correctly identify abstract classes, ensuring the legacy decorators metadata transformation works as intended.
if class.is_expression() || class.declare {

crates/oxc_transformer/src/decorator/legacy/metadata.rs:110

  • Confirm that the change to use 'class.declare' in lieu of 'class.is_typescript_syntax()' captures the intended cases for abstract classes in metadata processing. Consider if both properties should be checked if their semantics differ.
if class.is_expression() || class.declare {

@codspeed-hq
Copy link

codspeed-hq bot commented May 12, 2025

CodSpeed Instrumentation Performance Report

Merging #10952 will not alter performance

Comparing 05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata (a39eb85) with main (d036cf5)

Summary

✅ 36 untouched benchmarks

@Dunqing Dunqing force-pushed the 05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata branch from e959420 to a15b652 Compare May 12, 2025 12:33
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 12, 2025
Copy link
Member

Boshen commented May 12, 2025

Merge activity

@graphite-app graphite-app bot force-pushed the 05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata branch from a15b652 to a39eb85 Compare May 12, 2025 13:12
@graphite-app graphite-app bot merged commit a39eb85 into main May 12, 2025
26 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 12, 2025
@graphite-app graphite-app bot deleted the 05-12-fix_transformer_legacy-decorator_abstract_class_doesn_t_work_in_metadata branch May 12, 2025 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TS decorator metadata is not applied to abstract classes

4 participants