From c55a7a184fe0791280dfacf3dbfec652a042bb08 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 16:48:48 +0800 Subject: [PATCH] fix(compiler): lower all @angular/core member decorators into JIT propDecorators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In JIT mode, OXC dropped @angular/core member decorators that are not field decorators (@Inject, @Optional, @Attribute, @Self, @Host, and even @Component on a member) instead of lowering them into `static propDecorators` like ngtsc's `downlevel_decorators_transform` does. ngtsc's `isAngularDecorator` checks only `decorator.import.from === '@angular/core'` — the import decides, not the decorator name — and every matching member decorator (properties, methods, get/set accessors, static or instance) lands in `propDecorators` as `{type, args?}` in source order. Add `is_angular_core_decorator` (directive/property_decorators.rs), the nameless counterpart of `angular_core_decorator`: any named or default import from `@angular/core` (under any alias) or `ns.X()` through a namespace import qualifies. `extract_all_jit_member_decorators` now routes every core decorator into `propDecorators`; decorators from other modules keep going to `__decorate` as before. Also quote string-literal member keys ('x-y') in the emitted propDecorators object literal — previously they were spliced unquoted, producing invalid JS that made `strip_typescript` bail. Fixes #522 --- .../src/component/transform.rs | 123 ++++++++---------- .../oxc_angular_compiler/src/directive/mod.rs | 4 +- .../src/directive/property_decorators.rs | 35 +++++ .../tests/integration_test.rs | 27 +++- .../tests/jit_signal_initializer_test.rs | 65 ++++++++- ...t_angular_param_decorators_on_members.snap | 7 + 6 files changed, 174 insertions(+), 87 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index f2c8c6ba0..debc4b38b 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -882,6 +882,9 @@ struct JitParamDecorator { struct JitMemberDecorator { /// The property/member name. member_name: String, + /// Whether the member name came from a string-literal key (`'x-y'`) and + /// must be quoted in the `propDecorators` object literal. + quoted_key: bool, /// The Angular decorators on this member. decorators: std::vec::Vec, } @@ -1131,42 +1134,10 @@ fn extract_jit_ctor_params( (params, other_decorators) } -/// Angular's decorators. On a member, one imported from `@angular/core` (see -/// [`crate::directive::angular_core_decorator`]) goes to `propDecorators` if it's -/// a field decorator and is dropped otherwise (ngtsc lists those in -/// `propDecorators` too). Like ngtsc's JIT transform, where a decorator comes -/// from decides, not its name: another module's `@Inject` stays a `__decorate()` -/// call. -const ANGULAR_DECORATOR_NAMES: &[&str] = &[ - // Field decorators (→ propDecorators) - "Input", - "Output", - "HostBinding", - "HostListener", - "ViewChild", - "ViewChildren", - "ContentChild", - "ContentChildren", - // Parameter decorators (→ ctorParameters) - "Inject", - "Optional", - "Self", - "SkipSelf", - "Host", - "Attribute", - // Class decorators (→ class __decorate) - "Component", - "Directive", - "Pipe", - "Injectable", - "Service", - "NgModule", -]; - /// Extract all member decorators for JIT transformation in a single pass. /// /// Returns two collections: -/// - Angular field decorators → emitted as `static propDecorators = { ... }` +/// - Angular decorators (`@angular/core` imports) → `static propDecorators = { ... }` /// - Non-Angular decorators → emitted as `__decorate([...], target, "name", desc)` calls fn extract_all_jit_member_decorators( source: &str, @@ -1180,36 +1151,37 @@ fn extract_all_jit_member_decorators( let mut non_angular_members: std::vec::Vec = std::vec::Vec::new(); for element in &class.body.body { - let (member_name, is_static, is_property, decorators, initializer) = match element { - ClassElement::PropertyDefinition(prop) => { - let name = match &prop.key { - PropertyKey::StaticIdentifier(id) => id.name.to_string(), - PropertyKey::StringLiteral(s) => s.value.to_string(), - _ => continue, - }; - (name, prop.r#static, true, &prop.decorators, prop.value.as_ref()) - } - ClassElement::MethodDefinition(method) => { - if method.kind == MethodDefinitionKind::Constructor { - continue; + let (member_name, quoted_key, is_static, is_property, decorators, initializer) = + match element { + ClassElement::PropertyDefinition(prop) => { + let (name, quoted) = match &prop.key { + PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), + PropertyKey::StringLiteral(s) => (s.value.to_string(), true), + _ => continue, + }; + (name, quoted, prop.r#static, true, &prop.decorators, prop.value.as_ref()) } - let name = match &method.key { - PropertyKey::StaticIdentifier(id) => id.name.to_string(), - PropertyKey::StringLiteral(s) => s.value.to_string(), - _ => continue, - }; - (name, method.r#static, false, &method.decorators, None) - } - ClassElement::AccessorProperty(accessor) => { - let name = match &accessor.key { - PropertyKey::StaticIdentifier(id) => id.name.to_string(), - PropertyKey::StringLiteral(s) => s.value.to_string(), - _ => continue, - }; - (name, accessor.r#static, false, &accessor.decorators, None) - } - _ => continue, - }; + ClassElement::MethodDefinition(method) => { + if method.kind == MethodDefinitionKind::Constructor { + continue; + } + let (name, quoted) = match &method.key { + PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), + PropertyKey::StringLiteral(s) => (s.value.to_string(), true), + _ => continue, + }; + (name, quoted, method.r#static, false, &method.decorators, None) + } + ClassElement::AccessorProperty(accessor) => { + let (name, quoted) = match &accessor.key { + PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), + PropertyKey::StringLiteral(s) => (s.value.to_string(), true), + _ => continue, + }; + (name, quoted, accessor.r#static, false, &accessor.decorators, None) + } + _ => continue, + }; let mut angular_decs: std::vec::Vec = std::vec::Vec::new(); let mut non_angular_texts: std::vec::Vec = std::vec::Vec::new(); @@ -1244,21 +1216,21 @@ fn extract_all_jit_member_decorators( let name = source[callee.span().start as usize..callee.span().end as usize].to_string(); angular_decs.push(JitParamDecorator { name, args: call_args }); - } else if crate::directive::angular_core_decorator( - decorator, - Some(string_consts), - ANGULAR_DECORATOR_NAMES, - ) - .is_none() - { + } else if crate::directive::is_angular_core_decorator(decorator, Some(string_consts)) { + // Any other `@angular/core` decorator on a member (`@Inject`, + // `@Optional`, `@Attribute`, even `@Component`) also goes into + // propDecorators: ngtsc's `isAngularDecorator` checks only the + // import, not the decorator name. + let name = + source[callee.span().start as usize..callee.span().end as usize].to_string(); + angular_decs.push(JitParamDecorator { name, args: call_args }); + } else { // Non-Angular decorator (another module's `@Input` or `@Inject` // too) → goes into __decorate() call let expr_start = decorator.expression.span().start; let expr_end = decorator.expression.span().end; non_angular_texts.push(source[expr_start as usize..expr_end as usize].to_string()); } - // Angular non-field decorators (e.g. @Inject on a member) are silently dropped - // since they have no meaningful effect on members. } // Signal initializer-API lowering: synthesize @Input / @Output / @ViewChild / etc. @@ -1281,6 +1253,7 @@ fn extract_all_jit_member_decorators( if !angular_decs.is_empty() { angular_members.push(JitMemberDecorator { member_name: member_name.clone(), + quoted_key, decorators: angular_decs, }); } @@ -1622,7 +1595,13 @@ fn build_prop_decorators_text(members: &[JitMemberDecorator]) -> Option } }) .collect(); - entries.push(format!(" {}: [{}]", member.member_name, dec_strs.join(", "))); + // String-literal member keys ('x-y') need quotes in the object literal. + let key = if member.quoted_key { + format!("\"{}\"", escape_js_string(&member.member_name)) + } else { + member.member_name.clone() + }; + entries.push(format!(" {}: [{}]", key, dec_strs.join(", "))); } Some(format!("static propDecorators = {{\n{}\n}}", entries.join(",\n"))) diff --git a/crates/oxc_angular_compiler/src/directive/mod.rs b/crates/oxc_angular_compiler/src/directive/mod.rs index 172f7222c..522180bd5 100644 --- a/crates/oxc_angular_compiler/src/directive/mod.rs +++ b/crates/oxc_angular_compiler/src/directive/mod.rs @@ -47,8 +47,8 @@ pub use metadata::{ R3HostMetadata, R3InputMetadata, R3QueryMetadata, }; pub(crate) use property_decorators::{ - angular_class_decorator, angular_core_decorator, angular_member_decorator, - angular_param_decorator, extract_host_bindings_in, extract_host_listeners_in, + angular_class_decorator, angular_member_decorator, angular_param_decorator, + extract_host_bindings_in, extract_host_listeners_in, is_angular_core_decorator, parse_decorator_queries, try_parse_signal_input, try_parse_signal_model, try_parse_signal_output, unwrap_initializer_api_expr, }; diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 6eec25126..ab929dc38 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -95,6 +95,41 @@ pub(crate) fn angular_core_decorator( names.iter().copied().find(|n| *n == name) } +/// Whether `decorator` comes from `@angular/core`: `@X`/`@X()` with `X` +/// imported from it (by any alias or default), or `@ns.Y()` with +/// `import * as ns from '@angular/core'` (see [`is_core_namespace`]). +/// +/// This is ngtsc's `isAngularDecorator` +/// (`decorator.import.from === '@angular/core'`), the gate for the JIT +/// `propDecorators` lowering: the import decides, not the name, so even +/// `@Component` on a member is lowered into `propDecorators`. (The JIT +/// `ctorParameters` path still name-checks against [`PARAM_DECORATORS`].) +/// Without the file (`None`), any named decorator counts (same fallback as +/// [`angular_core_decorator`]'s name match). +pub(crate) fn is_angular_core_decorator( + decorator: &Decorator<'_>, + consts: Option<&super::StringConsts<'_>>, +) -> bool { + let callee = match &decorator.expression { + Expression::CallExpression(call) => &call.callee, + expr => expr, + }; + match callee { + Expression::Identifier(id) => match consts { + Some(consts) => consts + .scope() + .import(&id.name) + .is_some_and(|import| import.module == "@angular/core"), + None => true, + }, + Expression::StaticMemberExpression(m) => match &m.object { + Expression::Identifier(ns) => is_core_namespace(consts, &ns.name), + _ => false, + }, + _ => false, + } +} + /// Which of Angular's member decorators ([`MEMBER_DECORATORS`]) `decorator` /// is (see [`angular_core_decorator`]). Without the file (the public /// `extract_*` functions), any decorator with that name. diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 3ca7870ce..ada80cfbf 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -9064,11 +9064,12 @@ export class TestService { } #[test] -fn test_jit_angular_param_decorators_not_in_member_decorate() { - // Angular parameter decorators (@Inject, @Optional, @Self, @SkipSelf, @Host, @Attribute) - // should NOT be emitted in __decorate() calls if they appear on a member. - // While these are designed for constructor params, if someone puts them on a member, - // they should be treated as Angular decorators (not lowered via __decorate). +fn test_jit_angular_param_decorators_go_to_prop_decorators() { + // Angular parameter decorators (@Inject, @Optional, @Self, @SkipSelf, @Host, + // @Attribute) on a member go into `propDecorators`, like ngtsc's + // `isAngularDecorator` which checks only the `@angular/core` import, not the + // decorator name (issue #522). Only non-Angular decorators are lowered via + // __decorate() calls. let allocator = Allocator::default(); let source = r" import { Injectable, Inject, Optional } from '@angular/core'; @@ -9099,8 +9100,20 @@ export class MyService { result.code ); - // @Inject and @Optional should NOT appear in __decorate calls for members - // They are Angular decorators and should not be treated as non-Angular + // @Inject and @Optional should appear in propDecorators, like ngtsc + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + compact.contains("token:[{type:Inject,args:[\"TOKEN\"]}]"), + "propDecorators should contain the @Inject entry. Got:\n{}", + result.code + ); + assert!( + compact.contains("optionalDep:[{type:Optional}]"), + "propDecorators should contain the @Optional entry. Got:\n{}", + result.code + ); + + // They must not appear in member __decorate calls let member_decorate_calls: Vec<&str> = result .code .lines() diff --git a/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs b/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs index 3f20f49f5..21b4df99c 100644 --- a/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs +++ b/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs @@ -316,7 +316,8 @@ fn member_decorators_named_like_angular_ones_count_only_from_angular_core() { "Got:\n{out}" ); - // `@angular/core`'s `Inject` under an alias is treated like `@Inject`. + // `@angular/core`'s `Inject` under an alias is treated like `@Inject`: + // it goes to `propDecorators`, referenced by its written name (`Inj`). let aliased = compile_jit( "import { Component, Inject as Inj } from '@angular/core';\n\ @Component({ selector: 'c', template: '', standalone: true })\n\ @@ -327,10 +328,62 @@ fn member_decorators_named_like_angular_ones_count_only_from_angular_core() { @Component({ selector: 'c', template: '', standalone: true })\n\ export class C {\n @Inject(X) a: any;\n}\n", ); - assert!(!aliased.contains("Inj(X)"), "Got:\n{aliased}"); - assert_eq!( - aliased.replace("Inject as Inj", "Inject"), - plain, - "aliased:\n{aliased}\nplain:\n{plain}" + let compact = |s: String| s.chars().filter(|c| !c.is_whitespace()).collect::(); + assert!( + compact(aliased.clone()).contains("a:[{type:Inj,args:[X]}]"), + "aliased @Inject should go to propDecorators as written. Got:\n{aliased}" + ); + assert!( + compact(plain.clone()).contains("a:[{type:Inject,args:[X]}]"), + "@Inject should go to propDecorators. Got:\n{plain}" + ); +} + +#[test] +fn member_decorators_from_angular_core_all_go_to_prop_decorators() { + // Issue #522: ngtsc's `isAngularDecorator` only checks that the decorator is + // imported from `@angular/core`, not its name — so `@Inject`, `@Optional`, + // `@Attribute`, and even `@Component` on a member are lowered into + // `propDecorators`, keeping decorator order. + let out = compile_jit(&component( + " @Inject(TOKEN) @Optional() x: any;\n\ + @Attribute('a') y: any;\n\ + @Input() @Inject(TOKEN) z: any;\n\ + @Inject(TOKEN) 'x-y': any;\n", + "Inject, Optional, Attribute, Input", + )); + let compact: String = out.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + compact.contains("x:[{type:Inject,args:[TOKEN]},{type:Optional}]"), + "member @Inject/@Optional should be in propDecorators in order. Got:\n{out}" + ); + assert!( + compact.contains("y:[{type:Attribute,args:[\"a\"]}]"), + "member @Attribute should be in propDecorators. Got:\n{out}" + ); + assert!( + compact.contains("z:[{type:Input},{type:Inject,args:[TOKEN]}]"), + "mixed member should keep both entries in order. Got:\n{out}" + ); + assert!( + compact.contains("\"x-y\":[{type:Inject,args:[TOKEN]}]"), + "string-literal member key should be quoted in propDecorators. Got:\n{out}" + ); + + // A namespace-imported `@angular/core` decorator (`@core.Host()`) counts too. + let namespaced = compile_jit( + "import { Component } from '@angular/core';\n\ + import * as core from '@angular/core';\n\ + @Component({ selector: 'c', template: '', standalone: true })\n\ + export class C {\n @core.Host() h: any;\n @core.Component({}) m: any;\n}\n", + ); + let compact: String = namespaced.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + compact.contains("h:[{type:core.Host}]"), + "namespaced @core.Host() should be in propDecorators. Got:\n{namespaced}" + ); + assert!( + compact.contains("m:[{type:core.Component,args:[{}]}]"), + "namespaced @core.Component() on a member should be in propDecorators. Got:\n{namespaced}" ); } diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_param_decorators_on_members.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_param_decorators_on_members.snap index f4b64ba62..38ad55c83 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_param_decorators_on_members.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_param_decorators_on_members.snap @@ -11,6 +11,13 @@ let MyService = class MyService { token; optionalDep; customProp = ""; + static propDecorators = { + token: [{ + type: Inject, + args: ["TOKEN"] + }], + optionalDep: [{ type: Optional }] + }; }; __decorate([Custom()], MyService.prototype, "customProp", void 0); MyService = __decorate([Injectable()], MyService);