-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(es/decorators): Emit correct metadata for enum parameters #11154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This fixes decorator metadata emission to correctly handle enum parameters in decorated methods and constructors. Previously, SWC incorrectly emitted `typeof Options === "undefined" ? Object : Options` for enum parameters. Now it correctly emits: - `Number` for numeric enums - `String` for string enums - `Object` for mixed enums This matches TypeScript's behavior. Changes: - Updated `visit_mut_class_method` to check enum map before serializing parameter types - Updated `visit_mut_class` to check enum map for constructor parameters - Added test case for issue #11032 Fixes #11032 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
|
|
|
🦋 Changeset detectedLatest commit: 8970cc3 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
🤖 This pull request has been linked to AutoDev Task #774 View the task details and manage the automated development workflow in AutoDev. |
📋 AutoDev Task PromptObjectiveFix SWC's decorator metadata emission to correctly handle enum parameters in decorated methods. Currently, SWC incorrectly emits Issue ReferenceDocumentation & Resources
Root Cause AnalysisIn the file
The bug is an inconsistency: parameter types don't check the enum map, but return types and property types do. Technical DetailsHow Enum Tracking Works
The FixYou need to modify the Current code (lines 224-243): Generates paramtypes metadata without enum checking {
let dec = self.create_metadata_design_decorator(
"design:paramtypes",
ArrayLit {
span: DUMMY_SP,
elems: m
.function
.params
.iter()
.map(|v| {
Some(
serialize_type(self.class_name, get_type_ann_of_pat(&v.pat))
.as_arg(),
)
})
.collect(),
}
.as_arg(),
);
m.function.decorators.push(dec);
}Required change: Add enum checking before .map(|v| {
let param_type = get_type_ann_of_pat(&v.pat);
Some(
if let Some(kind) = self.enums.get_kind_as_str(param_type) {
quote_ident!(kind).as_arg()
} else {
serialize_type(self.class_name, param_type).as_arg()
}
)
})Also apply the same fix to constructor parameter metadata in the Testing Requirements
enum Options {
foo = 0
}
function decorate() {
return function() {}
}
class Foo {
@decorate()
foo(options: Options) {}
}
cargo test --package swc_ecma_transforms_proposal --test fixture
cargo test --package swc_ecma_transforms --test fixture -- legacy-metadataSuccess Criteria
Implementation Notes
This comment was automatically added by AutoDev. You can disable this feature in AutoDev Settings. |
|
Claude finished @DeLino[bot]'s task —— View job Code Review for PR #11154SummaryThis PR successfully fixes decorator metadata emission for enum parameters. The implementation correctly handles the inconsistency where parameter types weren't checking the enum map before serialization, while return types and property types were. ✅ Code Quality & Best PracticesExcellent Implementation:
Code Structure:
✅ Correctness & Bug AnalysisRoot Cause Properly Addressed:
Implementation Coverage:
✅ Test CoverageComprehensive Test Case:
Missing Test Cases (Low Priority):
✅ Performance ConsiderationsExcellent Performance Characteristics:
✅ Security AnalysisNo Security Concerns:
✅ Documentation & MaintainabilityGood Documentation:
Minor Observations
Final Assessment🎉 Excellent PR! This is a well-implemented fix that:
The fix is minimal, focused, and exactly what's needed to resolve issue #11032. The implementation demonstrates good understanding of the codebase architecture and TypeScript metadata emission requirements. Recommendation: ✅ APPROVE |
CodSpeed Performance ReportMerging #11154 will not alter performanceComparing Summary
Footnotes |
Fixes the metadata emission for enum parameters in decorators.
Summary
Fixes decorator metadata emission to correctly handle enum parameters in decorated methods and constructors.
Previously, SWC incorrectly emitted
typeof Options === "undefined" ? Object : Optionsfor enum parameters, but it should emit:Numberfor numeric enumsStringfor string enumsObjectfor mixed enumsThis matches TypeScript's behavior with
emitDecoratorMetadata.Changes
visit_mut_class_methodinmetadata.rsto check the enum map before serializing parameter typesvisit_mut_classinmetadata.rsto check the enum map for constructor parametersdesign:paramtypeis wrong for Enums in the same module #11032 demonstrating the fixRoot Cause
The bug was an inconsistency in how parameter types were handled vs. return types and property types:
self.enums.get_kind_as_str()before callingserialize_type()The fix applies the same pattern used for return types to parameter types as well.
Test Plan
tests/fixture/legacy-metadata/issues/11032/1/Numberinstead of the typeof checkcargo fmt --allRelated Issue
Fixes #11032
🤖 Generated with Claude Code