From c06a2a746e6d73559e54b5f6ceab4b09e17972de Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:46:20 +0800 Subject: [PATCH 01/14] fix(compiler): emit host listeners for @HostListener on accessor members ngtsc's `reflectClassMember` reports a TS `accessor` field as a PropertyDeclaration, so `filterToMembersWithDecorator` collects a `@HostListener` on one (typescript.ts, used by directive/src/shared.ts:693). OXC's `extract_host_listeners_in` only matched `MethodDefinition` and `PropertyDefinition`, so an `AccessorProperty` member was silently dropped. It now matches `ClassElement::AccessorProperty` too, mirroring `extract_host_bindings_in` which already handled it. Statics stay excluded via `r#static`, per `filterToMembersWithDecorator`'s `!member.isStatic` filter (#544). Fixes #546 --- .../src/directive/property_decorators.rs | 20 ++++++++--- .../tests/integration_test.rs | 34 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index ab929dc38..1998bf925 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -1635,17 +1635,29 @@ pub(crate) fn extract_host_listeners_in<'a>( let mut listeners = Vec::new_in(&allocator); for element in &class.body.body { - // Handle both MethodDefinition and PropertyDefinition (for arrow function handlers) - let (decorators, property_name) = match element { + // Handle MethodDefinition, PropertyDefinition (for arrow function + // handlers) and AccessorProperty: ngtsc's `reflectClassMember` reports + // a TS `accessor` field as a PropertyDeclaration, so + // `filterToMembersWithDecorator` sees @HostListener on one. + let (decorators, property_name, is_static) = match element { ClassElement::MethodDefinition(method) => { - (&method.decorators, get_property_key_name(&method.key)) + (&method.decorators, get_property_key_name(&method.key), method.r#static) } ClassElement::PropertyDefinition(prop) => { - (&prop.decorators, get_property_key_name(&prop.key)) + (&prop.decorators, get_property_key_name(&prop.key), prop.r#static) + } + ClassElement::AccessorProperty(prop) => { + (&prop.decorators, get_property_key_name(&prop.key), prop.r#static) } _ => continue, }; + // ngtsc's `filterToMembersWithDecorator` ignores static members: a + // `@HostListener()` on one is not a host listener. + if is_static { + continue; + } + let Some(decorator) = find_decorator_by_name(decorators, "HostListener", consts) else { continue; }; diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..babdd9f81 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -6137,6 +6137,40 @@ export class TestComponent { ); } +/// @HostListener on a TS `accessor` field is a host listener: ngtsc's +/// `reflectClassMember` reports auto-accessors as PropertyDeclarations, so +/// `filterToMembersWithDecorator` collects them (typescript.ts:695). A +/// `static accessor` stays excluded per the `!member.isStatic` filter. +#[test] +fn test_host_listener_on_accessor_member() { + let allocator = Allocator::default(); + let source = r" +import { Directive, HostListener } from '@angular/core'; + +@Directive({ selector: '[d]' }) +export class D { + @HostListener('click', ['$event']) accessor onClick = ($event: any) => {}; + @HostListener('scroll') static accessor onScroll = () => {}; +} +"; + + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + let code = &result.code; + let compact: String = code.chars().filter(|c| !c.is_whitespace()).collect(); + + // Instance accessor member produces a listener. + assert!( + compact.contains(r#"ɵɵlistener("click""#), + "instance @HostListener on an accessor should emit a listener. Got:\n{code}" + ); + // Static accessor member is ignored, like other static members. + assert!( + !compact.contains(r#"listener("scroll""#), + "static @HostListener on an accessor should not emit a listener. Got:\n{code}" + ); +} + /// `setClassMetadata`'s `propDecorators` mirrors ngtsc's `extractClassMetadata` /// (metadata.ts): static and ECMAScript-private members are excluded, and /// string-literal member keys are emitted quoted (`shouldQuoteName`). From ee27d35eddaa2a66079280da524e33118c675225 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:47:15 +0800 Subject: [PATCH 02/14] fix(compiler): fall back to ng-component for empty component selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc normalizes `selector: ''` to the default selector (`resolved === '' ? defaultSelector : resolved` in annotations/directive/src/shared.ts). For @Component the default is 'ng-component', identical to a missing selector. OXC kept '' as an explicit selector, producing `selectors: []` in ɵcmp and `""` in the .d.ts ComponentDeclaration type parameter — and the runtime reads `componentDef.selectors[0][0]` when creating a component without a host element. Normalize the empty selector to absent at metadata extraction so every emit path (ɵcmp selectors, partial ɵɵngDeclareComponent, .d.ts) applies the same 'ng-component' fallback used for a missing selector. @Directive is unchanged: upstream resolves '' to the `null` default, which raises NG2004 — not the component fallback. Directive selector diagnostics are tracked separately. Fixes #514 --- .../src/component/decorator.rs | 23 +++++- .../tests/integration_test.rs | 71 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index e28787906..c797893ff 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -90,7 +90,12 @@ pub fn extract_component_metadata<'a>( match key_name.as_str() { "selector" => { metadata.selector = - crate::directive::extract_string_value(allocator, &prop.value, consts); + crate::directive::extract_string_value(allocator, &prop.value, consts) + // ngtsc maps `selector: ''` to the default selector + // ('ng-component' for components), same as a missing + // selector. See annotations/directive/src/shared.ts + // (`resolved === '' ? defaultSelector : resolved`). + .filter(|s| !s.as_str().is_empty()); } "template" => { metadata.template = @@ -1266,6 +1271,22 @@ mod tests { }); } + #[test] + fn test_extract_empty_selector_falls_back_to_default() { + // ngtsc maps `selector: ''` to the default selector ('ng-component'), + // same as a missing selector. Storing None lets every emit path + // (ɵcmp selectors, partial ɵɵngDeclareComponent, .d.ts) apply that + // default. See https://github.com/voidzero-dev/oxc-angular-compiler/issues/514 + let code = r#" + import {Component} from '@angular/core'; + @Component({ selector: '', template: '' }) + class EmptySelectorComponent {} + "#; + assert_metadata(code, |meta| { + assert!(meta.selector.is_none(), "Empty selector should normalize to None"); + }); + } + #[test] fn test_extract_class_name() { let code = r#" diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..127c31df6 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -12258,6 +12258,77 @@ export class UnresolvedComponent {} ); } +// ============================================================================= +// Issue #514: `selector: ''` on @Component falls back to `ng-component` +// ============================================================================= +// ngtsc maps `selector: ''` to the default selector (`resolved === '' ? +// defaultSelector : resolved` in annotations/directive/src/shared.ts). For +// components the default is `ng-component`, identical to a missing selector; +// for directives the default is `null`, which raises NG2004 (directive +// diagnostics are tracked separately — OXC emits no `selectors` entry here, +// which is also the closest non-error output). + +#[test] +fn component_empty_selector_falls_back_to_ng_component() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ selector: '', template: '' }) +export class C {} +"#; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let cmp_start = result.code.find("ɵɵdefineComponent({").expect("ɵcmp missing"); + let cmp_section = &result.code[cmp_start..]; + let cmp_end = cmp_section.find("})").expect("ɵcmp not terminated"); + let cmp_def = &cmp_section[..cmp_end]; + assert!( + cmp_def.contains(r#"selectors:[["ng-component"]]"#), + "Empty selector should fall back to `ng-component`.\nɵcmp:\n{cmp_def}" + ); + + // The .d.ts selector type parameter must also fall back. + let decl = result.dts_declarations.iter().find(|d| d.class_name == "C").expect("d.ts missing"); + assert!( + decl.members.contains("\"ng-component\""), + "d.ts selector should be `ng-component`.\nMembers:\n{}", + decl.members + ); +} + +#[test] +fn directive_empty_selector_emits_no_invalid_selectors() { + // Upstream, `selector: ''` on @Directive resolves to the `null` default + // selector and raises NG2004. Until directive diagnostics land, the best + // OXC can do is not emit an invalid `selectors` array like `[[""]]` — + // and it must NOT fall back to `ng-component` (that fallback is + // component-only). + let allocator = Allocator::default(); + let source = r#" +import { Directive } from '@angular/core'; + +@Directive({ selector: '' }) +export class D {} +"#; + let result = transform_angular_file(&allocator, "d.directive.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let dir_start = result.code.find("ɵɵdefineDirective({").expect("ɵdir missing"); + let dir_section = &result.code[dir_start..]; + let dir_end = dir_section.find("})").expect("ɵdir not terminated"); + let dir_def = &dir_section[..dir_end]; + assert!( + !dir_def.contains("selectors"), + "Empty directive selector must not emit `selectors`.\nɵdir:\n{dir_def}" + ); + assert!( + !dir_def.contains("ng-component"), + "Directives must not get the component default selector.\nɵdir:\n{dir_def}" + ); +} + // ============================================================================= // Issue #287: TDZ-safe hoisting of consts referenced by emitted Ivy definitions // ============================================================================= From dc70632436caccc3b865c53af84d191c7e63b313 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:49:09 +0800 Subject: [PATCH 03/14] fix(compiler): read @Inject on @NgModule constructor parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NgModule ɵfac path matched the Injectable one for the flag decorators (@Optional/@Self/@SkipSelf/@Host) and @Attribute, but never matched 'Inject', so an @Inject(TOKEN) ctor param fell back to the type annotation (tokenless type -> ɵɵinvalidFactoryDep -> NG0202 at runtime; class type -> injects the class instead of the token). Extract the decorator's argument like injectable/decorator.rs does, under any alias ('import {Inject as Inj}') or 'ng.Inject', since angular_param_decorator resolves the imported name. The partial-declaration path also hardcoded 'deps: []' for every NgModule factory; it now carries the extracted deps so the linker can build the factory. Unskips ctor-mod-named/-alias/-ns in the ngtsc ctor-param fixture. Fixes #519 --- .../src/ng_module/decorator.rs | 27 +++- .../src/ng_module/definition.rs | 116 +++++++++++++++++- .../src/partial/ng_module.rs | 44 +++++-- .../ctor_param_decorator_identity_test.rs | 2 +- .../fixtures/ctor_param_decorators_ngtsc.json | 9 +- 5 files changed, 173 insertions(+), 25 deletions(-) diff --git a/crates/oxc_angular_compiler/src/ng_module/decorator.rs b/crates/oxc_angular_compiler/src/ng_module/decorator.rs index 949fde1d3..37575ad58 100644 --- a/crates/oxc_angular_compiler/src/ng_module/decorator.rs +++ b/crates/oxc_angular_compiler/src/ng_module/decorator.rs @@ -289,7 +289,7 @@ pub(crate) fn extract_ng_module_metadata_in<'a>( } // Extract constructor dependencies - metadata.deps = extract_constructor_deps(allocator, class, consts); + metadata.deps = extract_constructor_deps(allocator, class, source_text, consts); Some(metadata) } @@ -428,7 +428,8 @@ fn extract_identifier_array<'a>( /// - `None` if the class has no constructor (use inherited factory pattern) /// - `Some(vec)` with the dependencies if constructor exists /// -/// Handles parameter decorators like `@Optional()`, `@SkipSelf()`, `@Self()`, `@Host()`. +/// Handles parameter decorators like `@Inject()`, `@Optional()`, `@SkipSelf()`, +/// `@Self()`, `@Host()`. /// /// Example: /// ```typescript @@ -440,6 +441,7 @@ fn extract_identifier_array<'a>( pub fn extract_constructor_deps<'a>( allocator: &'a Allocator, class: &'a Class<'a>, + source_text: Option<&'a str>, consts: Option<&StringConsts<'_>>, ) -> Option>> { // Find the constructor method @@ -457,7 +459,7 @@ pub fn extract_constructor_deps<'a>( let mut deps = Vec::with_capacity_in(params.items.len(), &allocator); for param in ¶ms.items { - let dep = extract_param_dependency(allocator, param, consts); + let dep = extract_param_dependency(allocator, param, source_text, consts); deps.push(dep); } @@ -470,18 +472,29 @@ pub fn extract_constructor_deps<'a>( fn extract_param_dependency<'a>( allocator: &'a Allocator, param: &oxc_ast::ast::FormalParameter<'a>, + source_text: Option<&'a str>, consts: Option<&StringConsts<'_>>, ) -> R3DependencyMetadata<'a> { - // Extract flags from decorators + // Extract flags and @Inject token from decorators let mut optional = false; let mut skip_self = false; let mut self_ = false; let mut host = false; + let mut inject_token: Option> = None; let mut attribute_name: Option> = None; for decorator in ¶m.decorators { if let Some(name) = crate::directive::angular_param_decorator(decorator, consts) { match name { + "Inject" => { + // @Inject(TOKEN) - extract the token + if let Expression::CallExpression(call) = &decorator.expression { + if let Some(arg) = call.arguments.first() { + inject_token = + convert_oxc_expression(allocator, arg.to_expression(), source_text); + } + } + } "Optional" => optional = true, "SkipSelf" => skip_self = true, "Self" => self_ = true, @@ -499,8 +512,10 @@ fn extract_param_dependency<'a>( } } - // Extract the token (type annotation or parameter name) - let token = extract_param_token(allocator, param); + // Determine the token: + // 1. If @Inject(TOKEN) is present, use TOKEN + // 2. Otherwise, use the type annotation + let token = inject_token.or_else(|| extract_param_token(allocator, param)); // Handle @Attribute decorator if let Some(attr_name) = attribute_name { diff --git a/crates/oxc_angular_compiler/src/ng_module/definition.rs b/crates/oxc_angular_compiler/src/ng_module/definition.rs index 0e8033bed..0eb4371ae 100644 --- a/crates/oxc_angular_compiler/src/ng_module/definition.rs +++ b/crates/oxc_angular_compiler/src/ng_module/definition.rs @@ -215,7 +215,11 @@ pub fn generate_full_ng_module_definition<'a>( // statement upstream-mandated; the linker re-creates remote // scoping at link time if needed. See // `compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts:971`. - let fac_definition = compile_declare_factory_for_ng_module(allocator, &r3_metadata); + let fac_definition = compile_declare_factory_for_ng_module( + allocator, + &r3_metadata, + metadata.deps.as_ref(), + ); let mod_definition = compile_declare_ng_module_from_metadata(allocator, &r3_metadata); // Build the injector metadata using the same conversion the // full path uses (`generate_ng_module_inj`'s builder), then @@ -644,6 +648,116 @@ mod tests { ); } + /// Extract `NgModuleMetadata` from `code`'s single class declaration. + fn module_metadata_from_code<'a>( + allocator: &'a Allocator, + code: &'a str, + ) -> crate::ng_module::decorator::NgModuleMetadata<'a> { + use crate::ng_module::decorator::extract_ng_module_metadata; + use oxc_ast::ast::{Declaration, Statement}; + use oxc_parser::Parser; + use oxc_span::SourceType; + + let parser_ret = Parser::new(allocator, code, SourceType::tsx()).parse(); + let program = allocator.alloc(parser_ret.program); + let class = program + .body + .iter() + .find_map(|stmt| match stmt { + Statement::ClassDeclaration(class) => Some(class.as_ref()), + Statement::ExportDeclaration(export) => match &export.declaration { + Declaration::ClassDeclaration(class) => Some(class.as_ref()), + _ => None, + }, + _ => None, + }) + .expect("Should find class declaration"); + extract_ng_module_metadata(allocator, class, Some(code)) + .expect("Should extract NgModule metadata") + } + + #[test] + fn test_ng_module_inject_decorator_token() { + // Issue #519: @Inject(TOKEN) on an @NgModule ctor param must emit + // ɵɵinject(TOKEN), not ɵɵinvalidFactoryDep. + let allocator = Allocator::default(); + let code = r#" + @NgModule({}) + class M { + constructor(@Inject(TOKEN) x: unknown) {} + } + "#; + + let metadata = module_metadata_from_code(&allocator, code); + let deps = metadata.deps.as_ref().expect("Should have constructor deps"); + assert_eq!(deps.len(), 1); + assert!(deps[0].token.is_some(), "@Inject(TOKEN) should give the dep a token"); + + let definition = + generate_full_ng_module_definition(&allocator, &metadata, CompilationMode::Full) + .expect("Should generate definition"); + let js = emit_full_ng_module_definition("M", &definition); + assert!( + !js.contains("invalidFactoryDep"), + "Factory should NOT contain invalidFactoryDep, got: {js}" + ); + assert!( + js.contains("ɵɵinject(TOKEN"), + "Factory should inject the @Inject token (ɵɵinject for NgModule), got: {js}" + ); + assert!( + !js.contains("ɵɵdirectiveInject"), + "NgModule deps use ɵɵinject, not ɵɵdirectiveInject, got: {js}" + ); + } + + #[test] + fn test_ng_module_inject_decorator_overrides_type() { + // @Inject(Other) overrides the token a `Real` type annotation would give. + let allocator = Allocator::default(); + let code = r#" + @NgModule({}) + class M { + constructor(@Inject(Other) x: Real) {} + } + "#; + + let metadata = module_metadata_from_code(&allocator, code); + let definition = + generate_full_ng_module_definition(&allocator, &metadata, CompilationMode::Full) + .expect("Should generate definition"); + let js = emit_full_ng_module_definition("M", &definition); + assert!(js.contains("ɵɵinject(Other"), "Should inject Other, got: {js}"); + assert!(!js.contains("ɵɵinject(Real"), "Should not inject Real, got: {js}"); + } + + #[test] + fn test_ng_module_partial_factory_carries_deps() { + // Partial mode must carry ctor deps into the ɵɵngDeclareFactory call — + // the linker builds the factory from them. + let allocator = Allocator::default(); + let code = r#" + @NgModule({}) + class M { + constructor(@Inject(TOKEN) x: unknown) {} + } + "#; + + let metadata = module_metadata_from_code(&allocator, code); + let definition = + generate_full_ng_module_definition(&allocator, &metadata, CompilationMode::Partial) + .expect("Should generate definition"); + let js = emit_full_ng_module_definition("M", &definition); + assert!( + js.contains("token:TOKEN") || js.contains("token: TOKEN"), + "Partial ɵfac should carry the @Inject token, got: {js}" + ); + assert!( + !js.contains("deps:[]") && !js.contains("deps: []"), + "Partial ɵfac should not drop ctor deps, got: {js}" + ); + } + #[test] fn test_ng_module_definition_is_pure() { // Test that the generated expression has pure=true set on the function call. diff --git a/crates/oxc_angular_compiler/src/partial/ng_module.rs b/crates/oxc_angular_compiler/src/partial/ng_module.rs index 98ac25762..e006f0189 100644 --- a/crates/oxc_angular_compiler/src/partial/ng_module.rs +++ b/crates/oxc_angular_compiler/src/partial/ng_module.rs @@ -79,26 +79,48 @@ pub fn compile_declare_ng_module_from_metadata<'a>( } /// Builds the partial ɵfac factory paired with this NgModule. +/// +/// `deps` is the module's extracted constructor dependencies (`None` when it +/// has no constructor). A class without a constructor gets `deps: []` to +/// match upstream's golden behavior (see compiler-cli/test/compliance/ +/// test_cases/r3_view_compiler/hello_world/GOLDEN_PARTIAL.js — every ɵfac +/// for an NgModule carries `deps: []`). The linker then generates the +/// simple `new MyModule()` factory. OXC doesn't track base classes, so the +/// no-ctor `deps: null` upstream uses for the inherited-factory pattern +/// becomes `[]` here — a suboptimal-but-correct factory. pub fn compile_declare_factory_for_ng_module<'a>( allocator: &'a Allocator, meta: &R3NgModuleMetadata<'a>, + deps: Option<&Vec<'a, crate::factory::R3DependencyMetadata<'a>>>, ) -> OutputExpression<'a> { - // R3NgModuleMetadata doesn't carry constructor deps — the OXC - // NgModule analyzer doesn't extract them since NgModule classes are - // virtually always parameterless. Emit `deps: []` to match upstream's - // golden behavior (see compiler-cli/test/compliance/test_cases/ - // r3_view_compiler/hello_world/GOLDEN_PARTIAL.js — every ɵfac for - // an NgModule carries `deps: []`). The linker then generates the - // simple `new MyModule()` factory. - // - // An NgModule that extends another class is exotic enough that - // accepting a suboptimal-but-correct factory there is fine. + let factory_deps = match deps { + Some(deps) => { + let mut factory_deps: Vec<'a, crate::factory::R3DependencyMetadata<'a>> = + Vec::with_capacity_in(deps.len(), &allocator); + for dep in deps { + factory_deps.push(crate::factory::R3DependencyMetadata { + token: dep.token.as_ref().map(|t| t.clone_in(allocator)), + attribute_name_type: dep + .attribute_name_type + .as_ref() + .map(|a| a.clone_in(allocator)), + host: dep.host, + optional: dep.optional, + self_: dep.self_, + skip_self: dep.skip_self, + type_only_invalid: dep.type_only_invalid, + }); + } + R3FactoryDeps::Valid(factory_deps) + } + None => R3FactoryDeps::Valid(Vec::new_in(&allocator)), + }; let factory_meta = R3FactoryMetadata::Constructor(R3ConstructorFactoryMetadata { name: Ident::from("NgModuleFactory"), type_expr: meta.r#type.value.clone_in(allocator), type_decl: meta.r#type.value.clone_in(allocator), type_argument_count: 0, - deps: R3FactoryDeps::Valid(Vec::new_in(&allocator)), + deps: factory_deps, target: FactoryTarget::NgModule, }); compile_declare_factory_function(allocator, &factory_meta) diff --git a/crates/oxc_angular_compiler/tests/ctor_param_decorator_identity_test.rs b/crates/oxc_angular_compiler/tests/ctor_param_decorator_identity_test.rs index 0ff1c11ec..a1a06420f 100644 --- a/crates/oxc_angular_compiler/tests/ctor_param_decorator_identity_test.rs +++ b/crates/oxc_angular_compiler/tests/ctor_param_decorator_identity_test.rs @@ -220,7 +220,7 @@ fn ctor_param_decorators_match_ngtsc() { failures.len(), failures.join("\n\n") ); - assert_eq!(compared, 29, "fixtures compared"); + assert_eq!(compared, 32, "fixtures compared"); } /// A parameter decorator left in the output keeps its import: another module's diff --git a/crates/oxc_angular_compiler/tests/fixtures/ctor_param_decorators_ngtsc.json b/crates/oxc_angular_compiler/tests/fixtures/ctor_param_decorators_ngtsc.json index 405a8df55..b97df11de 100644 --- a/crates/oxc_angular_compiler/tests/fixtures/ctor_param_decorators_ngtsc.json +++ b/crates/oxc_angular_compiler/tests/fixtures/ctor_param_decorators_ngtsc.json @@ -518,8 +518,7 @@ "ctorParameters": "[{type:i1.Dep,decorators:[{type:Inject,args:[TOKEN]}]},{type:i1.Dep,decorators:[{type:Optional}]},{type:i1.Dep,decorators:[{type:Self}]},{type:i1.Dep,decorators:[{type:SkipSelf}]},{type:i1.Dep,decorators:[{type:Host}]},{type:i1.Dep,decorators:[{type:Attribute,args:[\"x\"]}]}]", "paramDecoratorsLeft": [], "jitCtorParameters": "[{type:Dep,decorators:[{type:Inject,args:[TOKEN]}]},{type:Dep,decorators:[{type:Optional}]},{type:Dep,decorators:[{type:Self}]},{type:Dep,decorators:[{type:SkipSelf}]},{type:Dep,decorators:[{type:Host}]},{type:Dep,decorators:[{type:Attribute,args:[\"x\"]}]}]", - "jitParamDecorators": [], - "skip": "pre-existing: oxc's `@NgModule` factory ignores `@Inject(TOKEN)` and injects the parameter's type, whatever the decorator is called" + "jitParamDecorators": [] }, { "name": "ctor-mod-alias", @@ -533,8 +532,7 @@ "ctorParameters": "[{type:i1.Dep,decorators:[{type:InX,args:[TOKEN]}]},{type:i1.Dep,decorators:[{type:OpX}]},{type:i1.Dep,decorators:[{type:Slf}]},{type:i1.Dep,decorators:[{type:SkS}]},{type:i1.Dep,decorators:[{type:HoX}]},{type:i1.Dep,decorators:[{type:AtX,args:[\"x\"]}]}]", "paramDecoratorsLeft": [], "jitCtorParameters": "[{type:Dep,decorators:[{type:InX,args:[TOKEN]}]},{type:Dep,decorators:[{type:OpX}]},{type:Dep,decorators:[{type:Slf}]},{type:Dep,decorators:[{type:SkS}]},{type:Dep,decorators:[{type:HoX}]},{type:Dep,decorators:[{type:AtX,args:[\"x\"]}]}]", - "jitParamDecorators": [], - "skip": "pre-existing: see `ctor-mod-named`" + "jitParamDecorators": [] }, { "name": "ctor-mod-ns", @@ -548,8 +546,7 @@ "ctorParameters": "[{type:i1.Dep,decorators:[{type:ng.Inject,args:[TOKEN]}]},{type:i1.Dep,decorators:[{type:ng.Optional}]},{type:i1.Dep,decorators:[{type:ng.Self}]},{type:i1.Dep,decorators:[{type:ng.SkipSelf}]},{type:i1.Dep,decorators:[{type:ng.Host}]},{type:i1.Dep,decorators:[{type:ng.Attribute,args:[\"x\"]}]}]", "paramDecoratorsLeft": [], "jitCtorParameters": "[{type:Dep,decorators:[{type:ng.Inject,args:[TOKEN]}]},{type:Dep,decorators:[{type:ng.Optional}]},{type:Dep,decorators:[{type:ng.Self}]},{type:Dep,decorators:[{type:ng.SkipSelf}]},{type:Dep,decorators:[{type:ng.Host}]},{type:Dep,decorators:[{type:ng.Attribute,args:[\"x\"]}]}]", - "jitParamDecorators": [], - "skip": "pre-existing: see `ctor-mod-named`" + "jitParamDecorators": [] }, { "name": "ctor-mod-foreign", From c77ba0f2783f2d0d9b42d01d06ab85a1f2111d16 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:50:13 +0800 Subject: [PATCH 04/14] fix(compiler): emit unevaluated query predicates as written Like ngtsc's WrappedNodeExpr, a query predicate that evaluates to a reference or a value that can't be statically evaluated is emitted verbatim, so @ViewChild(class Foo {}), new ViewChild(class Foo {}) in queries: metadata, and viewChild(class Foo {}) now emit the class expression in place instead of erroring with "predicate cannot be interpreted" or dropping the query. Fixes #516 --- .../src/directive/property_decorators.rs | 16 +++- .../src/output/oxc_converter.rs | 6 +- .../tests/decorator_metadata_ngtsc_test.rs | 82 +++++++++++++++++++ 3 files changed, 100 insertions(+), 4 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index ab929dc38..51ccb1875 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -22,7 +22,7 @@ use oxc_str::Ident; use super::evaluator::{Evaluator, Value}; use super::metadata::{QueryPredicate, R3InputMetadata, R3QueryMetadata}; use crate::output::ast::OutputExpression; -use crate::output::oxc_converter::convert_oxc_expression; +use crate::output::oxc_converter::{convert_oxc_expression, make_raw_source}; use crate::util::is_metadata_property; // ============================================================================ @@ -980,7 +980,9 @@ fn parse_query_config<'a>( list.push(lit.value.clone().into()); Some(QueryPredicate::Selectors(list)) } - _ => convert_oxc_expression(allocator, node, source_text).map(QueryPredicate::Type), + _ => convert_oxc_expression(allocator, node, source_text) + .or_else(|| make_raw_source(allocator, source_text, node.span())) + .map(QueryPredicate::Type), }; // Parse options from second argument if present @@ -1100,7 +1102,10 @@ fn try_parse_signal_query<'a>( let expr = predicate_arg.to_expression(); // Unwrap forwardRef if present - Angular doesn't include forwardRef in compiled output let unwrapped_expr = try_unwrap_forward_ref(expr, consts).unwrap_or(expr); - let output_expr = convert_oxc_expression(allocator, unwrapped_expr, source_text)?; + // ngtsc emits a non-string locator as written (`WrappedNodeExpr`), + // so fall back to the source text for what can't be converted. + let output_expr = convert_oxc_expression(allocator, unwrapped_expr, source_text) + .or_else(|| make_raw_source(allocator, source_text, unwrapped_expr.span()))?; QueryPredicate::Type(output_expr) } }; @@ -1976,8 +1981,13 @@ fn decorator_query<'a>( let node = try_unwrap_forward_ref(first, Some(consts)).unwrap_or(first); let at_node = |message: String| (message, node.span()); let predicate = match evaluator.evaluate(node) { + // Like ngtsc, a reference or a value that can't be statically + // evaluated is wrapped in a `WrappedNodeExpr` and emitted as written, + // so the source text is the fallback for nodes the output AST can't + // represent (`class Foo {}`, an import, ...). Value::Reference { .. } | Value::Dynamic | Value::Function(_) => { let expr = convert_oxc_expression(allocator, node, source_text) + .or_else(|| make_raw_source(allocator, source_text, node.span())) .ok_or_else(|| at_node(format!("@{name} predicate cannot be interpreted")))?; QueryPredicate::Type(expr) } diff --git a/crates/oxc_angular_compiler/src/output/oxc_converter.rs b/crates/oxc_angular_compiler/src/output/oxc_converter.rs index 59f661562..2559639f5 100644 --- a/crates/oxc_angular_compiler/src/output/oxc_converter.rs +++ b/crates/oxc_angular_compiler/src/output/oxc_converter.rs @@ -104,6 +104,10 @@ pub fn convert_oxc_expression<'a>( // Function expressions - fall back to raw source if available Expression::FunctionExpression(func) => make_raw_source(allocator, source_text, func.span), + // Class expressions - fall back to raw source if available + // (ngtsc emits them as written, e.g. a `@ViewChild(class Foo {})` predicate) + Expression::ClassExpression(class) => make_raw_source(allocator, source_text, class.span), + // Member expressions Expression::StaticMemberExpression(member) => { let receiver = convert_oxc_expression(allocator, &member.object, source_text)?; @@ -715,7 +719,7 @@ fn convert_oxc_binary_operator(op: oxc_ast::ast::BinaryOperator) -> Option( +pub(crate) fn make_raw_source<'a>( allocator: &'a Allocator, source_text: Option<&'a str>, span: Span, diff --git a/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs index 52999089b..ddacc2778 100644 --- a/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs +++ b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs @@ -1277,3 +1277,85 @@ export class Dir { assert!(code.contains(r#"inputs:{a:"a"},outputs:{b:"b"}}"#), "{}", result.code); assert!(code.contains("{a:[{type:In}],b:[{type:core.Output}]}"), "{}", result.code); } + +/// A query predicate that isn't statically evaluable is emitted as written +/// (ngtsc's `WrappedNodeExpr`, github issue #516): `extractQueryMetadata` +/// wraps `Reference`/`DynamicValue` predicates and the signal queries' +/// `parseLocator` wraps any non-string locator. A class expression is such a +/// value, so `@ViewChild(class Foo {})` is `ɵɵviewQuery(class Foo {}, 5)`, +/// not "predicate cannot be interpreted". +#[test] +fn unevaluated_query_predicates_are_emitted_as_written() { + // Member decorator and `queries:` metadata. + let source = "import {Component, ViewChild} from '@angular/core'; +@Component({selector: 'c', template: '', queries: {q2: new ViewChild(class Bar {})}}) +export class C { + @ViewChild(class Foo {}) q1: any; + q2: any; +} +"; + let result = transform(source); + assert!(errors(&result, source).is_empty(), "{:?}", errors(&result, source)); + let code = strip(&result.code); + // Member queries precede `queries:` ones in one chained `ɵɵviewQuery`. + assert!(code.contains("ɵɵviewQuery(classFoo{},5)(classBar{},5)"), "{}", result.code); + + // Signal query locator (upstream `parseLocator` wraps non-strings as written). + let source = "import {Component, viewChild} from '@angular/core'; +@Component({selector: 'c', template: ''}) +export class C { + q = viewChild(class Foo {}); +} +"; + let result = transform(source); + assert!(errors(&result, source).is_empty(), "{:?}", errors(&result, source)); + assert!(strip(&result.code).contains("ɵɵviewQuerySignal(ctx.q,classFoo{}"), "{}", result.code); +} + +/// The predicates ngtsc rejects are still rejected: anything that is neither +/// a string, a string array, nor a reference/dynamic value it can emit +/// verbatim (a number, an object literal, a non-string array member, ...). +#[test] +fn uninterpretable_query_predicates_are_still_errors() { + let cases = [ + ("@ViewChild(42) q: any;", "q", "42"), + ("@ViewChild({}) q: any;", "q", "{}"), + ("@ViewChild(true) q: any;", "q", "true"), + ("@ViewChild(null) q: any;", "q", "null"), + ]; + for (member, _, predicate) in cases { + let source = format!( + "import {{Component, ViewChild}} from '@angular/core'; +@Component({{selector: 'c', template: ''}}) +export class C {{ + {member} +}} +" + ); + let message = match predicate { + "{}" => "@ViewChild predicate cannot be interpreted Value is of type '{}'.", + "null" => "@ViewChild predicate cannot be interpreted Value is of type 'null'.", + "true" => "@ViewChild predicate cannot be interpreted Value is of type 'boolean'.", + _ => "@ViewChild predicate cannot be interpreted Value is of type 'number'.", + }; + assert_eq!( + errors(&transform(&source), &source), + vec![(message.to_string(), predicate.to_string())], + "{member}" + ); + } + + // The options argument is still checked literally (NG1001), even when it + // names a same-file const: ngtsc requires an object literal node. + let source = "import {Component, ViewChild} from '@angular/core'; +const OPTS = {static: true}; +@Component({selector: 'c', template: ''}) +export class C { + @ViewChild('a', OPTS) q: any; +} +"; + assert_eq!( + errors(&transform(source), source), + vec![("@ViewChild options must be an object literal".to_string(), "OPTS".to_string())] + ); +} From ff4c2fa1881b9da4215ca44c505b9bb005d9feb4 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:50:34 +0800 Subject: [PATCH 05/14] fix(compiler): gate ctor param and class-metadata decorators on the @angular/core import alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc's `isAngularDecorator` checks `decorator.import.from === '@angular/core'` with no name check. The JIT `ctorParameters` extraction and `setClassMetadata`'s `ctorParameters`/`propDecorators` filters still name-checked against the DI/member decorator lists, so `@angular/core` decorators whose names aren't listed (`@Component()` or any custom name on a constructor parameter or member) were dropped — or, in JIT, wrongly kept as `__param(index, dec)` decorators. All three paths now filter with `is_angular_core_decorator` when the file's imports are known (`Some(consts)`), matching `isAngularDecorator` in `downlevel_decorators_transform.ts` and `metadata.ts`. The `consts: None` fallbacks keep the known-name lists: without the file's imports OXC can't prove provenance, so a bare name-match is all that's safe. Foreign decorators on ctor params still lower to `__param` in JIT and stay dropped in AOT metadata as before. Fixes #538 --- .../src/class_metadata/builders.rs | 34 ++-- .../src/component/transform.rs | 17 +- .../src/directive/property_decorators.rs | 11 +- .../tests/integration_test.rs | 146 ++++++++++++++++++ ...lass_metadata_angular_core_decorators.snap | 46 ++++++ ...core_param_decorators_ctor_parameters.snap | 34 ++++ 6 files changed, 268 insertions(+), 20 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap create mode 100644 crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 1656baabc..12cc37df5 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -369,9 +369,13 @@ pub fn build_ctor_params_metadata<'a>( /// [`build_ctor_params_metadata`] for a class in the file `consts` was /// collected from. /// -/// A parameter decorator (`@Inject()`, `@Optional()`, ...) is listed only when -/// it's Angular's, imported from `@angular/core` by name, under any alias, or -/// through a namespace import (see [`crate::directive::angular_param_decorator`]). +/// A parameter decorator is listed only when it's Angular's, imported from +/// `@angular/core` — by name, under any alias, or through a namespace import — +/// whatever its name (`isAngularDecorator`, see +/// [`crate::directive::is_angular_core_decorator`]): `@Component()` on a +/// parameter counts too. Without the file's imports (`consts` is `None`), the +/// known-name list ([`crate::directive::angular_param_decorator`]) is used: +/// provenance can't be proven, so no `@Component()` on a parameter is listed. /// Like ngtsc, a parameter that has decorators, none of them Angular's, gets /// `decorators: []`. pub fn build_ctor_params_metadata_in<'a>( @@ -534,12 +538,17 @@ pub fn build_prop_decorators_metadata_in<'a>( continue; }; - // Filter to Angular property decorators: with the file's imports, only - // `@angular/core`'s (see `angular_member_decorator`), like ngtsc. + // Filter to Angular property decorators: with the file's imports, any + // `@angular/core` decorator counts (`isAngularDecorator`, see + // [`crate::directive::is_angular_core_decorator`]) — `@Inject()` or + // `@Component()` on a member too, not only the known member decorator + // names. Without them (`None`), the known-name list stays: the import + // can't be checked, so accepting every named decorator would mislist + // foreign ones. let angular_decorators: std::vec::Vec<_> = decorators .iter() .filter(|d| match consts { - Some(_) => crate::directive::angular_member_decorator(d, consts).is_some(), + Some(_) => crate::directive::is_angular_core_decorator(d, consts), None => get_decorator_name(d).is_some_and(|n| ANGULAR_PROP_DECORATORS.contains(&n)), }) .collect(); @@ -962,8 +971,12 @@ fn extract_param_type_expression<'a>( } } -/// Extract Angular's decorators from a constructor parameter (see -/// [`crate::directive::angular_param_decorator`]). +/// Extract Angular's decorators from a constructor parameter. With the file +/// (`Some(consts)`), any decorator imported from `@angular/core` is Angular's, +/// whatever its name — ngtsc's `isAngularDecorator` (`metadata.ts`), see +/// [`crate::directive::is_angular_core_decorator`]. Without it, only the known +/// parameter decorator names ([`crate::directive::angular_param_decorator`]): +/// provenance can't be proven, so a bare name-match is all that's safe. fn extract_angular_decorators_from_param<'a, 'b>( param: &'b FormalParameter<'a>, consts: Option<&StringConsts<'a>>, @@ -971,7 +984,10 @@ fn extract_angular_decorators_from_param<'a, 'b>( param .decorators .iter() - .filter(|d| crate::directive::angular_param_decorator(d, consts).is_some()) + .filter(|d| match consts { + Some(consts) => crate::directive::is_angular_core_decorator(d, Some(consts)), + None => crate::directive::angular_param_decorator(d, consts).is_some(), + }) .collect() } diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 5c7a261b2..97b40767f 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -1066,11 +1066,13 @@ fn find_angular_decorator<'a>( /// /// Like Angular's JIT transform (`downlevel_decorators_transform.ts`), a /// parameter decorator goes into `ctorParameters` only when it's Angular's, -/// imported from `@angular/core` (see -/// [`crate::directive::angular_param_decorator`]), listed as written -/// (`{ type: Inj }`, `{ type: ng.Optional }`). Any other one stays a decorator -/// of the class, lowered as `__param(index, decorator)`; those are returned -/// second, in source order. +/// imported from `@angular/core` — the gate is the import alone, not the +/// decorator name (`isAngularDecorator`, see +/// [`crate::directive::is_angular_core_decorator`]), so even `@Component()` on +/// a parameter counts, listed as written (`{ type: Inj }`, +/// `{ type: ng.Optional }`). Any other one stays a decorator of the class, +/// lowered as `__param(index, decorator)`; those are returned second, in +/// source order. fn extract_jit_ctor_params( source: &str, class: &oxc_ast::ast::Class<'_>, @@ -1101,9 +1103,12 @@ fn extract_jit_ctor_params( .and_then(|ann| extract_type_name_from_annotation(&ann.type_annotation)); // Angular's decorators go into ctorParameters, by their written name. + // The gate is the `@angular/core` import alone, like ngtsc's + // `isAngularDecorator`: `@Component()` or a custom name imported from + // it on a parameter counts too. let mut decorators = std::vec::Vec::new(); for decorator in ¶m.decorators { - if crate::directive::angular_param_decorator(decorator, Some(consts)).is_none() { + if !crate::directive::is_angular_core_decorator(decorator, Some(consts)) { let expr = decorator.expression.span(); other_decorators.push(format!( "__param({index}, {})", diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index ab929dc38..304e8dd7a 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -101,11 +101,12 @@ pub(crate) fn angular_core_decorator( /// /// 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). +/// `propDecorators`/`ctorParameters` lowering and `setClassMetadata`'s +/// decorator lists: the import decides, not the name, so even `@Component` on +/// a member or constructor parameter is listed. Without the file (`None`), +/// any named decorator counts (same fallback as +/// [`angular_core_decorator`]'s name match); callers that can't prove +/// provenance keep the known-name lists instead. pub(crate) fn is_angular_core_decorator( decorator: &Decorator<'_>, consts: Option<&super::StringConsts<'_>>, diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..bc05ba45c 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -9197,6 +9197,152 @@ export class MyService { insta::assert_snapshot!("jit_angular_param_decorators_on_members", result.code); } +#[test] +fn test_jit_any_angular_core_param_decorator_goes_to_ctor_parameters() { + // Issue #538: ngtsc's `isAngularDecorator` on constructor parameters checks + // only the `@angular/core` import, not the decorator name + // (`downlevel_decorators_transform.ts:475`). So a `@angular/core` decorator + // whose name isn't a DI one — `@Component()`, or any name imported from it + // (`@CustomDec`) — is listed in `ctorParameters` instead of staying a + // `__param` decorator. Only a foreign decorator is lowered as + // `__param(index, dec)`. + let allocator = Allocator::default(); + let source = r" +import { Component, Component as Cmp, Inject, Injectable } from '@angular/core'; +import { Inject as ForeignInject } from 'not-angular'; + +const TOKEN = 'token'; + +@Component({ selector: 'app-test', template: '' }) +export class TestComponent { + constructor( + @Inject(TOKEN) known: any, + @Cmp() componentDec: any, + @Injectable customDec: any, + @ForeignInject(TOKEN) foreign: any, + ) {} +} +"; + + let options = ComponentTransformOptions { jit: true, ..Default::default() }; + let result = + transform_angular_file(&allocator, "test.component.ts", source, Some(&options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + + // All @angular/core decorators land in ctorParameters, listed as written. + assert!( + compact.contains( + "{type:undefined,decorators:[{type:Inject,args:[TOKEN]}]},\ + {type:undefined,decorators:[{type:Cmp}]},\ + {type:undefined,decorators:[{type:Injectable}]}," + ), + "Angular core param decorators should be in ctorParameters. Got:\n{}", + result.code + ); + + // The foreign @Inject is not Angular's: no ctorParameters decorators for + // param 3, and it's lowered as __param. + assert!( + compact.contains("{type:undefined}]"), + "foreign-decorated param should have no decorators entry. Got:\n{}", + result.code + ); + assert!( + compact.contains("__param(3,ForeignInject(TOKEN))"), + "Foreign @Inject should be lowered as __param. Got:\n{}", + result.code + ); + assert!( + !compact.contains("__param(0,") + && !compact.contains("__param(1,") + && !compact.contains("__param(2,"), + "Angular core param decorators must not be lowered as __param. Got:\n{}", + result.code + ); + + insta::assert_snapshot!("jit_angular_core_param_decorators_ctor_parameters", result.code); +} + +#[test] +fn test_class_metadata_lists_any_angular_core_decorator() { + // Issue #538: `setClassMetadata`'s `ctorParameters` and `propDecorators` + // filter member/param decorators by the `@angular/core` import alone + // (`metadata.ts:174-176` and `:190-191` — `isAngularDecorator`), not by a + // decorator-name list. A member decorated `@Component()` and a parameter + // decorated `@Injectable()` (both imported from `@angular/core`) are + // listed; a same-named decorator from another module is not. + // + // Note: ngtsc rejects a non-DI `@angular/core` decorator on a ctor param + // in DI analysis (DECORATOR_UNEXPECTED), so upstream never emits this + // metadata — oxc lists it, matching the metadata filter's own semantics. + let allocator = Allocator::default(); + let source = r" +import { Component, Inject, Injectable } from '@angular/core'; +import { Inject as ForeignInject } from 'not-angular'; + +const TOKEN = 'token'; + +@Component({ selector: 'app-test', template: '' }) +export class TestComponent { + @Component() + componentMember: any; + + @Inject(TOKEN) + knownMember: any; + + @ForeignInject(TOKEN) + foreignMember: any; + + constructor( + @Inject(TOKEN) known: any, + @Injectable() customDec: any, + @ForeignInject(TOKEN) foreign: any, + ) {} +} +"; + + let options = ComponentTransformOptions::default(); + let result = + transform_angular_file(&allocator, "test.component.ts", source, Some(&options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + + // propDecorators: any @angular/core member decorator counts, even + // @Component(); the foreign one doesn't. + assert!( + compact.contains("componentMember:[{type:Component}]"), + "@Component() member should be in propDecorators. Got:\n{}", + result.code + ); + assert!( + compact.contains("knownMember:[{type:Inject,args:[TOKEN]}]"), + "@Inject member should be in propDecorators. Got:\n{}", + result.code + ); + assert!( + !compact.contains("foreignMember:["), + "foreign-decorated member should not be in propDecorators. Got:\n{}", + result.code + ); + + // ctorParameters: same import-only gate; a param with only foreign + // decorators still gets `decorators: []`, like ngtsc. + assert!( + compact.contains( + "{type:undefined,decorators:[{type:Inject,args:[TOKEN]}]},\ + {type:undefined,decorators:[{type:Injectable}]},\ + {type:undefined,decorators:[]}" + ), + "ctorParameters should list @angular/core decorators and [] for the foreign one. Got:\n{}", + result.code + ); + + insta::assert_snapshot!("class_metadata_angular_core_decorators", result.code); +} + // ========================================================================= // Reference output comparison tests // ========================================================================= diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap new file mode 100644 index 000000000..8b3dccab4 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap @@ -0,0 +1,46 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: result.code +--- + +import { Component, Inject, Injectable } from '@angular/core'; +import { Inject as ForeignInject } from 'not-angular'; +import * as i0 from '@angular/core'; + +const TOKEN = 'token'; + +export class TestComponent { + @Component() + componentMember: any; + + @Inject(TOKEN) + knownMember: any; + + @ForeignInject(TOKEN) + foreignMember: any; + + constructor( + known: any, + @Injectable() customDec: any, + @ForeignInject(TOKEN) foreign: any, + ) {} + +static ɵfac = function TestComponent_Factory(__ngFactoryType__) { + return new (__ngFactoryType__ || TestComponent)(i0.ɵɵdirectiveInject(TOKEN),i0.ɵɵinvalidFactoryDep(1), + i0.ɵɵinvalidFactoryDep(2)); +}; +static ɵcmp = /*@__PURE__*/ i0.ɵɵdefineComponent({type:TestComponent,selectors:[["app-test"]],decls:0, + vars:0,template:function TestComponent_Template(rf,ctx) { + },encapsulation:2}); +} +(() =>{ + (((typeof ngDevMode === "undefined") || ngDevMode) && i0.ɵsetClassDebugInfo(TestComponent, + {className:"TestComponent",filePath:"test.component.ts",lineNumber:8})); +})(); +(() =>{ + (((typeof ngDevMode === "undefined") || ngDevMode) && i0.ɵsetClassMetadata(TestComponent, + [{type:Component,args:[{selector:"app-test",template:""}]}],() =>[{type:undefined, + decorators:[{type:Inject,args:[TOKEN]}]},{type:undefined,decorators:[{type:Injectable}]}, + {type:undefined,decorators:[]}],{componentMember:[{type:Component}],knownMember:[{type:Inject, + args:[TOKEN]}]})); +})(); diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap new file mode 100644 index 000000000..0b8cdd651 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap @@ -0,0 +1,34 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: result.code +--- +import { Component, Component as Cmp, Inject, Injectable } from "@angular/core"; +import { Inject as ForeignInject } from "not-angular"; +import { __decorate, __param } from "tslib"; +const TOKEN = "token"; +let TestComponent = class TestComponent { + constructor(known, componentDec, customDec, foreign) {} + static ctorParameters = () => [ + { + type: undefined, + decorators: [{ + type: Inject, + args: [TOKEN] + }] + }, + { + type: undefined, + decorators: [{ type: Cmp }] + }, + { + type: undefined, + decorators: [{ type: Injectable }] + }, + { type: undefined } + ]; +}; +TestComponent = __decorate([Component({ + selector: "app-test", + template: "" +}), __param(3, ForeignInject(TOKEN))], TestComponent); +export { TestComponent }; From 381e15299485024afb6f4bceccd9db3a3f1cce3f Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:51:34 +0800 Subject: [PATCH 06/14] fix(compiler): evaluate ambient globals as truthy references like ngtsc An identifier this file doesn't declare or import (a lib global like `window`, `atob` or `document`, or a name declared nowhere) was treated as dynamic wherever it appeared, so `transform: window ? atob : btoa` errored with "Input transform must be a function". ngtsc resolves such a name to a `Reference` to its declaration, which is truthy like any reference, and picks the branch: the transform compiles to `atob`. `RefKind::Ambient` now stands as that truthy reference where only truthiness or the name matters (`?:`, `&&`, `||` and unary operators) while staying dynamic where a concrete value would be read from it (member access, calls, spreads), which is also what ngtsc's `accessHelper` and call path do for a reference to a declaration in another file. Fixes #517 --- .../src/directive/evaluator.rs | 26 ++++--- .../tests/decorator_metadata_ngtsc_test.rs | 73 +++++++++++++++++++ 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/evaluator.rs b/crates/oxc_angular_compiler/src/directive/evaluator.rs index 8fc6710a4..368a03ddc 100644 --- a/crates/oxc_angular_compiler/src/directive/evaluator.rs +++ b/crates/oxc_angular_compiler/src/directive/evaluator.rs @@ -1154,10 +1154,12 @@ pub(crate) enum RefKind<'a> { Global, /// Any other identifier this file doesn't declare or import: a global /// declared outside it, in a lib such as the DOM's (`atob`, `window`) or in - /// a project `.d.ts`, which ngtsc resolves to that declaration, or a name - /// declared nowhere, which ngtsc can't resolve. oxc can't tell these apart, - /// so it's dynamic (see [`Value::is_dynamic`]), except that an input - /// transform assumes it's a function, as it does an imported one. + /// a project `.d.ts`, which ngtsc resolves to a reference to that + /// declaration, or a name declared nowhere, which ngtsc can't resolve + /// (`DynamicValue`). oxc can't tell these apart, so the reference stands + /// where one can (`?:`, `&&`, `||`, an input transform: a truthy name), + /// and is dynamic where a concrete value would be read (member access, + /// calls, spreads, ..., see [`Value::is_dynamic`]). Ambient, /// A static getter or setter, or a static property without an /// initializer, declared at the span. @@ -1252,8 +1254,12 @@ impl<'a> Value<'a> { } } - /// Whether ngtsc's value is unknown: dynamic, or a global declared outside - /// the file, which is only kept as a name so a transform can refer to it. + /// Whether ngtsc's value is unknown: dynamic, or a name declared outside + /// the file or nowhere (`RefKind::Ambient`), which is kept only as a name + /// (`Value could not be determined statically`) where a concrete value + /// would be read. Where only truthiness or the name matters (`?:`, `&&`, + /// `||`, a transform) it's the truthy reference ngtsc resolves a global + /// to; callers that read it as a value check `Value::Dynamic` instead. pub(crate) fn is_dynamic(&self) -> bool { matches!(self, Value::Dynamic | Value::Reference { kind: RefKind::Ambient, .. }) } @@ -1823,7 +1829,8 @@ impl<'s, 'a> Evaluator<'s, 'a> { frame: &Frame<'a>, ) -> Value<'a> { match self.eval(&c.test, depth, frame) { - test if test.is_dynamic() => Value::Dynamic, + // A reference (a global included) is truthy: `window ? a : b` is `a`. + Value::Dynamic => Value::Dynamic, test if test.is_import() => test.computed(), test if test.truthy() => self.eval(&c.consequent, depth, frame), _ => self.eval(&c.alternate, depth, frame), @@ -1842,7 +1849,7 @@ impl<'s, 'a> Evaluator<'s, 'a> { return Value::Dynamic; } match self.eval(&u.argument, depth, frame) { - value if value.is_dynamic() => Value::Dynamic, + Value::Dynamic => Value::Dynamic, value if value.is_import() => value.computed(), value => unary(u.operator, &value), } @@ -1881,7 +1888,8 @@ impl<'s, 'a> Evaluator<'s, 'a> { let left = self.eval(&l.left, depth, frame); let right = self.eval(&l.right, depth, frame); match (left, right) { - (left, right) if left.is_dynamic() || right.is_dynamic() => Value::Dynamic, + // A reference (a global included) is truthy: `window && f` is `f`. + (Value::Dynamic, _) | (_, Value::Dynamic) => Value::Dynamic, (left, _) if left.is_import() => left.computed(), (left, right) => match (l.operator, left.truthy()) { (LogicalOperator::And, true) | (LogicalOperator::Or, false) => right, diff --git a/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs index 52999089b..087b91322 100644 --- a/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs +++ b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs @@ -1029,6 +1029,79 @@ export class Dir { ); } +/// A global declared outside the file (`window`, `atob`, `document`, ... from +/// a lib or `.d.ts`) resolves to a reference in ngtsc's evaluator, which is +/// truthy like any reference: `window ? atob : btoa` is `atob` (ngtsc 22.1.7 +/// compiles `transform: window ? atob : btoa` to `atob`, #517). oxc can't +/// tell a lib's global from a name declared nowhere, so an ambient name is +/// evaluated the same way in `?:`, `&&` and `||`, but stays dynamic where a +/// value would be read from it (`x.y`, `x[k]`, `x()`). +#[test] +fn ambient_globals_are_truthy_references() { + // Each ambient name picks a branch of `?:`, `||` or `&&`; `undefined` + // isn't a reference and a declared const evaluates as itself. + let cases = [ + ("window ? atob : btoa", "atob"), + ("document ? btoa : atob", "btoa"), + ("atob || btoa", "atob"), + ("unknownGlobal && atob", "atob"), + ("undefined ? atob : btoa", "btoa"), + ("no ? atob : btoa", "btoa"), + ("yes ? unknownGlobal : btoa", "unknownGlobal"), + ]; + let members: String = cases + .iter() + .enumerate() + .map(|(i, (expr, _))| format!("@Input({{transform: {expr}}}) i{i}: any;\n")) + .collect(); + let source = format!( + "import {{Directive, Input}} from '@angular/core'; +const no = false; +const yes = true; +@Directive({{selector: '[d]'}}) +export class Dir {{\n {members}}}\n" + ); + let result = transform(&source); + assert!(errors(&result, &source).is_empty(), "{:?}", errors(&result, &source)); + let code = strip(&result.code); + for (i, (_, transform_name)) in cases.iter().enumerate() { + assert!( + code.contains(&format!(r#"i{i}:[2,"i{i}","i{i}",{transform_name}]"#)), + "{transform_name} expected for input i{i}\n{code}" + ); + } + + // The same in `inputs:` metadata. + let source = "import {Directive} from '@angular/core'; +@Directive({selector: '[d]', inputs: [{name: 'x', transform: window ? atob : btoa}]}) +export class Dir {} +"; + let result = transform(source); + assert!(errors(&result, source).is_empty(), "{:?}", errors(&result, source)); + assert!(strip(&result.code).contains(r#"inputs:{x:[2,"x","x",atob]}"#), "{}", result.code); + + // A member read from a global is still dynamic (`window.location`), so the + // transform is reported unresolvable, as is `??`, which ngtsc doesn't + // evaluate either. + for expr in ["window.location ? atob : btoa", "window ?? atob"] { + let source = format!( + "import {{Directive}} from '@angular/core'; +@Directive({{selector: '[d]', inputs: [{{name: 'x', transform: {expr}}}]}}) +export class Dir {{}} +" + ); + assert_eq!( + errors(&transform(&source), &source), + vec![( + "Input transform must be a function Value could not be determined statically." + .to_string(), + expr.to_string() + )], + "{expr}" + ); + } +} + /// ngtsc checks an overloaded static method's first declaration, not its /// implementation (checked with @angular/compiler-cli 22.1.7, which compiles /// this; the snapshot can't hold it because ngtsc emits the method's bare name, From 6fe24c31fef8d01476eda0487adff6581b7c07c9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:52:06 +0800 Subject: [PATCH 07/14] fix(compiler): keep imports referenced by setClassMetadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ɵsetClassMetadata's ctorParameters callback names ctor-param decorators ({type: Optional}), @Inject(TOKEN) tokens (args: [TOKEN]) and member decorators ({type: Input}) as bare identifiers, but import elision dropped them unconditionally. TestBed.overrideComponent and friends then throw ReferenceError when they invoke ctorParameters (issue #520). Pass whether metadata will be emitted (emit_class_metadata && !advanced_optimizations) into ImportElisionAnalyzer::analyze and skip the ctor-param-decorator/@Inject-arg/declare-prop elision set in that case — matching ngtsc, which keeps these imports. Elision still applies when no setClassMetadata is emitted. Fixes #520 --- .../src/component/import_elision.rs | 82 +++++- .../src/component/transform.rs | 9 +- .../tests/integration_test.rs | 57 +++-- .../set_class_metadata_import_elision_test.rs | 239 ++++++++++++++++++ 4 files changed, 365 insertions(+), 22 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs diff --git a/crates/oxc_angular_compiler/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 7c91bcf07..526a9f902 100644 --- a/crates/oxc_angular_compiler/src/component/import_elision.rs +++ b/crates/oxc_angular_compiler/src/component/import_elision.rs @@ -21,6 +21,11 @@ //! - Constructor parameter decorators (`@Inject`, `@Optional`, etc.) - Angular removes these //! - DI tokens used only in `@Inject(TOKEN)` arguments //! +//! The last two apply only when no `ɵsetClassMetadata` is emitted: the +//! metadata's `ctorParameters` callback names the param decorators and +//! `@Inject` tokens as bare identifiers, so their imports must survive +//! (matching ngtsc, which keeps them). +//! //! ## What gets preserved //! //! - Decorators used at runtime (@Component, @Input, etc.) @@ -63,7 +68,16 @@ impl<'a> ImportElisionAnalyzer<'a> { /// /// This builds a semantic model from the program and checks each import /// specifier to see if it has any non-type references. - pub fn analyze(program: &'a Program<'a>) -> Self { + /// + /// `emit_metadata` must mirror whether the transform will emit + /// `ɵsetClassMetadata` for this file (`emit_class_metadata && + /// !advanced_optimizations`). The metadata callback names constructor + /// parameter decorators (`{type: Optional}`), `@Inject(TOKEN)` argument + /// tokens (`args: [TOKEN]`), and member decorators (`{type: Input}`) by + /// their local (imported) name, so those imports must survive elision — + /// like ngtsc, which keeps them. When no metadata is emitted, the + /// decorators are stripped outright and the imports can be dropped. + pub fn analyze(program: &'a Program<'a>, emit_metadata: bool) -> Self { let semantic_ret = SemanticBuilder::new().build(program); let semantic = &semantic_ret.semantic; @@ -71,9 +85,14 @@ impl<'a> ImportElisionAnalyzer<'a> { // First, collect all symbols that are used ONLY in constructor parameter decorators. // These should be elided because Angular removes these decorators during compilation. + // When class metadata is emitted, `ctorParameters`/`propDecorators` reference the + // same names, so nothing collected here is elidable. let consts = crate::directive::StringConsts::declarations_of(program); - let ctor_param_decorator_only = - Self::collect_ctor_param_decorator_only_imports(program, &consts); + let ctor_param_decorator_only = if emit_metadata { + FxHashSet::default() + } else { + Self::collect_ctor_param_decorator_only_imports(program, &consts) + }; // Analyze each import declaration for stmt in &program.body { @@ -726,7 +745,9 @@ impl<'a> ImportElisionAnalyzer<'a> { file_path: &Path, cross_file_analyzer: &mut super::cross_file_elision::CrossFileAnalyzer, ) -> Self { - let mut analyzer = Self::analyze(program); + // Compare-test helper: no `setClassMetadata` context is available, so + // analyze as if no class metadata is emitted. + let mut analyzer = Self::analyze(program, false); // Enhanced analysis: check cross-file for remaining imports for stmt in &program.body { @@ -1031,10 +1052,14 @@ mod tests { } fn analyze_source(source: &str) -> FxHashSet { + analyze_source_with_metadata(source, false) + } + + fn analyze_source_with_metadata(source: &str, emit_metadata: bool) -> FxHashSet { let allocator = Allocator::default(); let source_type = SourceType::ts(); let parser_ret = Parser::new(&allocator, source, source_type).parse(); - let analyzer = ImportElisionAnalyzer::analyze(&parser_ret.program); + let analyzer = ImportElisionAnalyzer::analyze(&parser_ret.program, emit_metadata); analyzer.type_only_specifiers().iter().map(|a| a.to_string()).collect() } @@ -1100,7 +1125,7 @@ const service = new AuthService(); let allocator = Allocator::default(); let source_type = SourceType::ts(); let parser_ret = Parser::new(&allocator, source, source_type).parse(); - let analyzer = ImportElisionAnalyzer::analyze(&parser_ret.program); + let analyzer = ImportElisionAnalyzer::analyze(&parser_ret.program, false); filter_imports(source, &parser_ret.program, &analyzer) } @@ -1628,6 +1653,51 @@ export class TestComponent { assert!(!type_only.contains("Component"), "Component should be preserved"); } + #[test] + fn test_param_decorators_and_inject_token_kept_when_metadata_emitted() { + // Same source as test_multiple_param_decorators_elided, but analyzed as + // if `ɵsetClassMetadata` will be emitted: `ctorParameters` names all of + // these bare, so none is type-only (issue #520; ngtsc keeps them). + let source = r#" +import { Component, Inject, Optional, Self, SkipSelf, Host } from "@angular/core"; +import { LOCALE_ID } from "@angular/core"; + +@Component({ selector: 'app-test' }) +export class TestComponent { + constructor( + @Optional() @Self() service1: Service1, + @Optional() @SkipSelf() service2: Service2, + @Host() service3: Service3, + @Inject(LOCALE_ID) locale: string, + ) {} +} +"#; + let type_only = analyze_source_with_metadata(source, true); + + for name in ["Inject", "Optional", "Self", "SkipSelf", "Host", "LOCALE_ID"] { + assert!( + !type_only.contains(name), + "{name} must survive while setClassMetadata is emitted" + ); + } + + // Type-only imports are still elided under metadata emission: the + // parameter type is referenced via a namespace import (`i1.MyService`), + // not by its bare name. + let source2 = r#" +import { Component, Optional } from "@angular/core"; +import { MyService } from "./service"; + +@Component({ selector: 'app-test' }) +export class TestComponent { + constructor(@Optional() service: MyService) {} +} +"#; + let type_only2 = analyze_source_with_metadata(source2, true); + assert!(!type_only2.contains("Optional"), "Optional must survive"); + assert!(type_only2.contains("MyService"), "MyService is type-only — still elided"); + } + #[test] fn test_inject_token_used_elsewhere_preserved() { // If a token is used outside @Inject, it should be preserved diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 5c7a261b2..890e77f2f 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -2445,7 +2445,14 @@ pub fn transform_angular_file( // Run import elision analysis on the original program. // This identifies type-only imports that can be removed. // Must run BEFORE transformation to capture correct type vs value references. - let import_elision = ImportElisionAnalyzer::analyze(&parser_ret.program); + // `ɵsetClassMetadata` names ctor-param decorators (`{type: Optional}`), + // `@Inject` tokens (`args: [TOKEN]`) and member decorators (`{type: Input}`) + // by their imported names, so those imports must stay whenever metadata + // will be emitted (ngtsc keeps them too). + let import_elision = ImportElisionAnalyzer::analyze( + &parser_ret.program, + options.emit_class_metadata && !options.advanced_optimizations, + ); // Collect class definitions by class name. // Each entry is (class_name, static_definitions_to_insert, external_declarations) diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..8cbe3eea0 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -3689,8 +3689,10 @@ export class TestComponent { ); } -/// Test that constructor parameter decorators (@Optional, @Inject, etc.) are elided from imports. -/// Angular removes these decorators during compilation and encodes them in factory metadata. +/// Test that constructor parameter decorators (@Optional, @Inject, etc.) keep their imports +/// while `ɵsetClassMetadata` is emitted (the default), and are elided once it is disabled. +/// The metadata's `ctorParameters` callback names them as bare identifiers, so dropping the +/// imports leaves unbound references (issue #520). ngtsc keeps them too. #[test] fn test_import_elision_ctor_param_decorators() { let allocator = Allocator::default(); @@ -3727,16 +3729,15 @@ export class BitLabelComponent { .unwrap(); println!("Angular core import line: {import_line}"); - // Optional should be elided (only used as ctor param decorator) + // ctorParameters emits `{type: Optional}` / `{type: Inject, args: [DOCUMENT]}` + // bare, so both imports must be retained while setClassMetadata is emitted. assert!( - !import_line.contains("Optional"), - "Optional should be elided from imports. Import line: {import_line}" + import_line.contains("Optional"), + "Optional should be kept in imports (named by setClassMetadata). Import line: {import_line}" ); - - // Inject should be elided (only used as ctor param decorator) assert!( - !import_line.contains("Inject"), - "Inject should be elided from imports. Import line: {import_line}" + import_line.contains("Inject"), + "Inject should be kept in imports (named by setClassMetadata). Import line: {import_line}" ); // ElementRef should be elided (only used in type annotation, DI comes from namespace) @@ -3751,17 +3752,43 @@ export class BitLabelComponent { "Component should be in imports. Import line: {import_line}" ); - // Find the @angular/common import line (if it exists) + // DOCUMENT is named bare inside ctorParameters `args`, so its import stays too. let common_import = code.lines().find(|l| l.starts_with("import") && l.contains("@angular/common")); + let common_line = + common_import.unwrap_or_else(|| panic!("DOCUMENT import missing. Code: {code}")); + assert!( + common_line.contains("DOCUMENT"), + "DOCUMENT should be kept in imports (named by setClassMetadata). Import line: {common_line}" + ); + + // With class metadata emission disabled, the same imports are elided again. + let no_metadata = ComponentTransformOptions { + emit_class_metadata: false, + ..ComponentTransformOptions::default() + }; + let result = + transform_angular_file(&allocator, "test.component.ts", source, Some(&no_metadata), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + let code = &result.code; + assert!(!code.contains("setClassMetadata"), "metadata should not be emitted: {code}"); - // DOCUMENT should be elided (only used in @Inject argument) - // If the import line exists, it should not contain DOCUMENT - // Or the entire import should be removed - if let Some(common_line) = common_import { + let import_line = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + for decorator in ["Optional", "Inject"] { + assert!( + !import_line.contains(decorator), + "{decorator} should be elided when no setClassMetadata is emitted. Import line: {import_line}" + ); + } + if let Some(common_line) = + code.lines().find(|l| l.starts_with("import") && l.contains("@angular/common")) + { assert!( !common_line.contains("DOCUMENT"), - "DOCUMENT should be elided from imports. Import line: {common_line}" + "DOCUMENT should be elided when no setClassMetadata is emitted. Import line: {common_line}" ); } } diff --git a/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs b/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs new file mode 100644 index 000000000..2365a38e7 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs @@ -0,0 +1,239 @@ +//! Regression tests for issue #520: `ɵsetClassMetadata`'s `ctorParameters` +//! callback names ctor-parameter decorators (`{type: Optional}`), `@Inject` +//! tokens (`args: [TOKEN]`), and member decorators (`{type: Input}`) as bare +//! identifiers. Import elision must keep those imports whenever metadata is +//! emitted (the default), like ngtsc — otherwise `TestBed.overrideComponent` +//! and friends throw `ReferenceError` when they invoke `ctorParameters`. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{TransformOptions, transform_angular_file}; + +fn compile(source: &str) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(!result.has_errors(), "unexpected errors: {:?}", result.diagnostics); + result.code +} + +fn compile_with(source: &str, options: &TransformOptions) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, Some(options), None); + assert!(!result.has_errors(), "unexpected errors: {:?}", result.diagnostics); + result.code +} + +/// Slice out the `ctorParameters`/`propDecorators` metadata argument text so a +/// test can assert about the names the callback references. +fn metadata_block(code: &str) -> &str { + let start = code.find("setClassMetadata").expect("setClassMetadata missing"); + &code[start..] +} + +/// The issue's exact repro: `@Optional() @Inject(TOKEN)` on a @Component ctor +/// parameter must keep `Inject`, `Optional`, and `TOKEN` imported. +#[test] +fn ctor_param_decorators_keep_imports_with_class_metadata() { + let source = r#" +import {Component, Inject, Optional} from '@angular/core'; +import {TOKEN} from './tokens'; +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Optional() @Inject(TOKEN) x: unknown) {} +} +"#; + let code = compile(source); + + assert!( + code.contains("import {Component, Inject, Optional}"), + "Inject/Optional imports must survive: {code}" + ); + assert!(code.contains("import {TOKEN} from './tokens'"), "TOKEN import must survive: {code}"); + + // The metadata still names them bare — now bound by the kept imports. + let metadata = metadata_block(&code); + assert!(metadata.contains("{type:Optional}"), "{metadata}"); + assert!(metadata.contains("{type:Inject,args:[TOKEN]}"), "{metadata}"); +} + +/// Same repro with `emit_class_metadata` off: the decorators are stripped and +/// their imports are elided, as before. +#[test] +fn ctor_param_decorators_elided_without_class_metadata() { + let source = r#" +import {Component, Inject, Optional} from '@angular/core'; +import {TOKEN} from './tokens'; +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Optional() @Inject(TOKEN) x: unknown) {} +} +"#; + let options = TransformOptions { emit_class_metadata: false, ..TransformOptions::default() }; + let code = compile_with(source, &options); + + assert!(!code.contains("setClassMetadata"), "{code}"); + let core_import = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + assert_eq!(core_import, "import { Component } from \"@angular/core\";"); + assert!(!code.contains("import {TOKEN}"), "TOKEN should be elided: {code}"); +} + +/// `@Inject(TOKEN)` must also keep the decorator imports for @Directive, +/// @Injectable, @Pipe, and @NgModule classes — they all get setClassMetadata. +#[test] +fn ctor_param_decorators_kept_for_all_decorated_kinds() { + let source = r#" +import {Directive, Injectable, Inject, Optional} from '@angular/core'; +import {TOKEN} from './tokens'; +@Directive({selector: '[d]'}) +export class D { + constructor(@Optional() @Inject(TOKEN) x: unknown) {} +} +@Injectable() +export class S { + constructor(@Inject(TOKEN) y: unknown) {} +} +"#; + let code = compile(source); + + assert!(code.contains("Inject"), "Inject import must survive: {code}"); + assert!(code.contains("Optional"), "Optional import must survive: {code}"); + assert!(code.contains("import {TOKEN} from './tokens'"), "TOKEN import must survive: {code}"); + assert_eq!(code.matches("setClassMetadata").count(), 2, "{code}"); +} + +/// `import { Inject as Inj }`: the metadata emits the local name `{type: Inj}`, +/// so the aliased specifier must be kept (it already was — regression guard). +#[test] +fn aliased_param_decorators_keep_imports() { + let source = r#" +import {Component, Inject as Inj, Optional as Opt} from '@angular/core'; +import {TOKEN} from './tokens'; +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Opt() @Inj(TOKEN) x: unknown) {} +} +"#; + let code = compile(source); + + assert!( + code.contains("import {Component, Inject as Inj, Optional as Opt}"), + "aliased imports must survive: {code}" + ); + assert!(code.contains("import {TOKEN} from './tokens'"), "{code}"); + let metadata = metadata_block(&code); + assert!(metadata.contains("{type:Opt}"), "{metadata}"); + assert!(metadata.contains("{type:Inj,args:[TOKEN]}"), "{metadata}"); +} + +/// `@Attribute(ATTR)`: the decorator name dangled before the fix even though +/// its argument's import was kept by semantic analysis. +#[test] +fn attribute_decorator_keeps_import() { + let source = r#" +import {Component, Attribute} from '@angular/core'; +import {ATTR} from './tokens'; +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Attribute(ATTR) x: string) {} +} +"#; + let code = compile(source); + + let core_import = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + assert!(core_import.contains("Attribute"), "Attribute must stay imported: {code}"); + let metadata = metadata_block(&code); + assert!(metadata.contains("type:Attribute"), "{metadata}"); + assert!(metadata.contains("args:[ATTR]"), "{metadata}"); +} + +/// A token declared in the same file needs no import, but the param decorators +/// still name `Inject`/`Optional` bare. +#[test] +fn local_token_keeps_decorator_imports() { + let source = r#" +import {Component, Inject, Optional, InjectionToken} from '@angular/core'; +const TOKEN = new InjectionToken('TOKEN'); +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Optional() @Inject(TOKEN) x: unknown) {} +} +"#; + let code = compile(source); + + let core_import = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + assert!(core_import.contains("Inject"), "{code}"); + assert!(core_import.contains("Optional"), "{code}"); +} + +/// A member decorator on a `declare`d field is still emitted into +/// `propDecorators` (`{x: [{type: Input}]}`), so its import must survive too. +#[test] +fn declare_prop_decorator_keeps_import() { + let source = r#" +import {Component, Input} from '@angular/core'; +@Component({selector: 'c', template: ''}) +export class C { + @Input() declare x: string; +} +"#; + let code = compile(source); + + let core_import = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + assert!(core_import.contains("Input"), "Input must stay imported: {code}"); + let metadata = metadata_block(&code); + assert!(metadata.contains("{x:[{type:Input}]}"), "{metadata}"); +} + +/// Param decorators on an *undecorated* class are left in the emitted source, +/// so their imports must survive regardless of class metadata. +#[test] +fn param_decorators_on_undecorated_class_keep_imports() { + let source = r#" +import {Inject, Optional} from '@angular/core'; +export class NotDecorated { + constructor(@Optional() @Inject(String) x: unknown) {} +} +"#; + let code = compile(source); + + assert!(code.contains("import {Inject, Optional}"), "{code}"); + assert!( + code.contains("@Optional() @Inject(String)"), + "decorators should be left in place: {code}" + ); +} + +/// With no class metadata AND advanced optimizations, elision still applies. +#[test] +fn ctor_param_decorators_elided_with_advanced_optimizations() { + let source = r#" +import {Component, Inject, Optional} from '@angular/core'; +import {TOKEN} from './tokens'; +@Component({selector: 'c', template: ''}) +export class C { + constructor(@Optional() @Inject(TOKEN) x: unknown) {} +} +"#; + let options = TransformOptions { advanced_optimizations: true, ..TransformOptions::default() }; + let code = compile_with(source, &options); + + assert!(!code.contains("setClassMetadata"), "{code}"); + let core_import = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) + .unwrap(); + assert!(!core_import.contains("Inject"), "Inject should be elided: {code}"); + assert!(!core_import.contains("Optional"), "Optional should be elided: {code}"); + assert!(!code.contains("import {TOKEN}"), "TOKEN should be elided: {code}"); +} From 9574429ebc42708c83b34dd3b2f99ba9c0a911f9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:52:34 +0800 Subject: [PATCH 08/14] fix(compiler): list all @angular/core class decorators in setClassMetadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two @Component decorators on one class: ngtsc gives no diagnostic — it compiles the first, leaves the second on the class (downleveled by TS into __decorate), and lists BOTH in ɵsetClassMetadata's decorators array, since extractClassMetadata filters class decorators on isAngularDecorator (imported from @angular/core), not on which one the component handler took. OXC already compiled the first decorator and kept the second's text on the emitted class, but passed only the compiled decorator to build_decorator_metadata_array, so setClassMetadata listed just one entry. - transform_angular_file (@Component branch) and build_set_class_metadata_decls (@Directive/@Pipe/@Injectable/ @NgModule/@Service) now collect every class decorator that is_angular_core_decorator accepts, in source order — the same filter extractClassMetadata uses — instead of a single primary decorator. - build_decorator_metadata_array takes the compiled @Component decorator so the resource-inlining gate mirrors transformDecoratorResources: when the compiled decorator's config references external resources, the args of EVERY decorator literally named `Component` are replaced by the transformed map (upstream `{...dec, args: [literal]}`); the unresolvable template-literal field drop now also applies to the verbatim args of duplicate @Component decorators. Fixes #521 --- .../src/class_metadata/builders.rs | 165 +++++---- .../src/component/transform.rs | 329 ++++++++++++++---- 2 files changed, 355 insertions(+), 139 deletions(-) diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 1656baabc..122401032 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -29,26 +29,48 @@ use crate::output::oxc_converter::{ /// /// When `inlined_template` and/or `inlined_styles` are provided (typically for /// `@Component` decorators with `templateUrl`/`styleUrls`/`styleUrl` resolved -/// via `ResolvedResources`), the first argument of the first decorator (the -/// component config object literal) is rewritten so that `templateUrl` becomes -/// `template` (with content inlined) and `styleUrls`/`styleUrl` are folded into -/// the `styles` array. This matches Angular's `transformDecoratorResources` (see -/// `inline_component_resources` below for the source-cited semantics) and is -/// required for TestBed JIT recompilation, since Angular's -/// `componentNeedsResolution(metadata)` check throws when `templateUrl` is set -/// without a sibling `template` field, or when `styleUrls?.length > 0`, even -/// though the AOT-compiled `ɵcmp` already has the template baked in. +/// via `ResolvedResources`), the component config object literal is rewritten +/// so that `templateUrl` becomes `template` (with content inlined) and +/// `styleUrls`/`styleUrl` are folded into the `styles` array. This matches +/// Angular's `transformDecoratorResources` (see `inline_component_resources` +/// below for the source-cited semantics) and is required for TestBed JIT +/// recompilation, since Angular's `componentNeedsResolution(metadata)` check +/// throws when `templateUrl` is set without a sibling `template` field, or +/// when `styleUrls?.length > 0`, even though the AOT-compiled `ɵcmp` already +/// has the template baked in. +/// +/// `component_decorator` is the `@Component` decorator that was compiled (the +/// first one on the class, as `find_component_decorator` resolves it). It is +/// the decorator whose evaluated metadata map `transformDecoratorResources` +/// reads (`component/src/handler.ts` passes the compiled `component` map), so +/// it decides whether resource inlining applies. When its config object +/// references external resources, upstream rewrites the `args` of EVERY +/// class decorator literally named `Component` to that same transformed map — +/// including a second `@Component` the compiler did not take (issue #521). +/// Decorators spelled differently (`@Cmp` aliases) stay verbatim, matching +/// upstream's `if (dec.name !== 'Component') return dec;`. pub fn build_decorator_metadata_array<'a>( allocator: &'a Allocator, decorators: &[&Decorator<'a>], source_text: Option<&'a str>, inlined_template: Option<&'a str>, inlined_styles: Option<&[Ident<'a>]>, + component_decorator: Option<&Decorator<'a>>, consts: Option<&StringConsts<'a>>, ) -> OutputExpression<'a> { + // Whether `transformDecoratorResources` rewrites the `args` of decorators + // named `Component`: the compiled decorator's config object references + // external resources (component/src/resources.ts checks the evaluated + // `component` map for `templateUrl`/`styleUrls`/`styleUrl`/`styles`). + let component_source_obj = component_decorator.and_then(|d| match &d.expression { + Expression::CallExpression(call) => call.arguments.first().and_then(|a| a.as_expression()), + _ => None, + }); + let transform_resources = matches!(component_source_obj, Some(Expression::ObjectExpression(obj)) if has_resource_property(obj)); + let mut decorator_entries = AllocVec::new_in(&allocator); - for (decorator_idx, decorator) in decorators.iter().enumerate() { + for decorator in decorators.iter() { let mut map_entries = AllocVec::new_in(&allocator); // Get decorator type name @@ -88,72 +110,85 @@ pub fn build_decorator_metadata_array<'a>( // Add "type" entry map_entries.push(LiteralMapEntry::new(Ident::from("type"), type_expr, false)); - // Add "args" entry if the decorator has arguments - if let Expression::CallExpression(call) = &decorator.expression - && !call.arguments.is_empty() + // Gate resource inlining on the decorator's written name, matching + // Angular's `if (dec.name !== 'Component') return dec;` at the top of + // `transformDecoratorResources` — an `@Cmp` alias stays verbatim even + // when the compiled decorator (resolved by import) has resources. + // Without this, other decorators that happen to use resource-shaped + // keys (e.g. `@Inject({ templateUrl: … })`, legal TS even if + // nonsensical) would get their literals stripped. + let is_component_decorator = + get_decorator_name(decorator).is_some_and(|n| n == "Component"); + + let mut args = AllocVec::new_in(&allocator); + let mut args_emitted = false; + + // When the compiled decorator's config references external resources, + // upstream replaces `args` of every `Component`-named decorator with a + // single object literal rebuilt from the COMPILED decorator's metadata + // map (`{...dec, args: [createObjectLiteralExpression(newMetadataFields)]}`) + // — a duplicate `@Component` gets the same transformed args, not its own. + if transform_resources + && is_component_decorator + && let Some(obj) = component_source_obj { - // Gate resource inlining on the decorator's name, matching Angular's - // `if (dec.name !== 'Component') return dec;` at the top of - // `transformDecoratorResources`. Without this, other decorators that - // happen to use resource-shaped keys (e.g. `@Inject({ templateUrl: … })`, - // legal TS even if nonsensical) get their literals stripped. - let is_component_decorator = - get_decorator_name(decorator).is_some_and(|n| n == "Component"); - - let mut args = AllocVec::new_in(&allocator); + // ngtsc rebuilds the metadata from its plain properties when it + // inlines resources, so methods and accessors are dropped. + let mut converted = match obj { + Expression::ObjectExpression(o) => { + convert_plain_properties(allocator, o, source_text) + } + _ => convert_oxc_expression(allocator, obj, source_text), + }; + if let Some(converted) = &mut converted { + inline_component_resources(&allocator, converted, inlined_template, inlined_styles); + // Drop config fields whose value is a template literal with an + // unresolvable `${…}` interpolation, matching the AOT `ɵcmp` path + // (which drops e.g. an unresolved `selector`). Otherwise the raw + // template literal would leak verbatim into `setClassMetadata`. + if let Some(consts) = consts { + drop_unresolvable_template_literal_fields(&allocator, converted, obj, consts); + } + } + if let Some(converted) = converted { + args.push(converted); + args_emitted = true; + } + } + + if !args_emitted && let Expression::CallExpression(call) = &decorator.expression { for (arg_idx, arg) in call.arguments.iter().enumerate() { let expr = arg.to_expression(); - let converted = match expr { - // ngtsc rebuilds the metadata from its plain properties when it - // inlines resources, so methods and accessors are dropped. - Expression::ObjectExpression(obj) - if is_component_decorator - && decorator_idx == 0 - && arg_idx == 0 - && has_resource_property(obj) => - { - convert_plain_properties(allocator, obj, source_text) - } - _ => convert_oxc_expression(allocator, expr, source_text), - }; + let converted = convert_oxc_expression(allocator, expr, source_text); if let Some(mut converted) = converted { - // Inline resolved templates/styles into the first arg of the - // first @Component decorator. Other decorators / other args - // are left alone. - if is_component_decorator && decorator_idx == 0 && arg_idx == 0 { - inline_component_resources( + // Same template-literal drop as the transformed path, applied + // to the config object of each `Component`-named decorator + // left verbatim (issue #521: a second `@Component` too). + if is_component_decorator + && arg_idx == 0 + && let Some(consts) = consts + { + drop_unresolvable_template_literal_fields( &allocator, &mut converted, - inlined_template, - inlined_styles, + expr, + consts, ); - // Drop config fields whose value is a template literal with an - // unresolvable `${…}` interpolation, matching the AOT `ɵcmp` path - // (which drops e.g. an unresolved `selector`). Otherwise the raw - // template literal would leak verbatim into `setClassMetadata`. - if let Some(consts) = consts { - drop_unresolvable_template_literal_fields( - &allocator, - &mut converted, - expr, - consts, - ); - } } args.push(converted); } } + } - if !args.is_empty() { - map_entries.push(LiteralMapEntry::new( - Ident::from("args"), - OutputExpression::LiteralArray(Box::new_in( - LiteralArrayExpr { entries: args, source_span: None }, - &allocator, - )), - false, - )); - } + if !args.is_empty() { + map_entries.push(LiteralMapEntry::new( + Ident::from("args"), + OutputExpression::LiteralArray(Box::new_in( + LiteralArrayExpr { entries: args, source_span: None }, + &allocator, + )), + false, + )); } // Create the decorator object: { type: ..., args: [...] } @@ -428,6 +463,7 @@ pub fn build_ctor_params_metadata_in<'a>( None, None, None, + None, ); map_entries.push(LiteralMapEntry::new( Ident::from("decorators"), @@ -553,6 +589,7 @@ pub fn build_prop_decorators_metadata_in<'a>( None, None, None, + None, ); prop_entries.push(LiteralMapEntry::new(prop_name, decorators_array, quoted)); continue; diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 5c7a261b2..e8c348ab3 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -750,7 +750,7 @@ fn find_last_import_end(program_body: &[Statement<'_>]) -> Option { } /// Build the `ɵsetClassMetadata(...)` declaration string for a non-`@Component` -/// decorated class (`@Directive`/`@Pipe`/`@Injectable`/`@NgModule`). +/// decorated class (`@Directive`/`@Pipe`/`@Injectable`/`@NgModule`/`@Service`). /// /// Mirrors the `@Component` metadata block (without template/style inlining): /// emits the decorator metadata, `ctorParameters` (reflected from the class, with @@ -758,12 +758,16 @@ fn find_last_import_end(program_body: &[Statement<'_>]) -> Option { /// (real `@Input`/`@Output`/query plus synthesized initializer-API). Returns an /// empty string when metadata emission is disabled. Matches ngc, which emits /// `setClassMetadata` for all decorated classes (needed for TestBed overrides). +/// +/// The decorators array lists ALL of the class's `@angular/core` decorators in +/// source order — ngtsc's `extractClassMetadata` filters on `isAngularDecorator` +/// (imported from `@angular/core`), so a duplicate `@Directive` or a co-located +/// `@Injectable` appears alongside the decorator that was compiled (issue #521). #[allow(clippy::too_many_arguments)] fn build_set_class_metadata_decls<'a>( allocator: &'a Allocator, class: &oxc_ast::ast::Class<'a>, class_name: &str, - decorator: &oxc_ast::ast::Decorator<'a>, options: &TransformOptions, source: &'a str, string_consts: &crate::directive::StringConsts<'a>, @@ -774,6 +778,15 @@ fn build_set_class_metadata_decls<'a>( return String::new(); } + let angular_decorators: std::vec::Vec<_> = class + .decorators + .iter() + .filter(|d| crate::directive::is_angular_core_decorator(d, Some(string_consts))) + .collect(); + if angular_decorators.is_empty() { + return String::new(); + } + let type_expr = OutputExpression::ReadVar(oxc_allocator::Box::new_in( ReadVarExpr { name: Ident::from(class_name), source_span: None }, &allocator, @@ -782,10 +795,11 @@ fn build_set_class_metadata_decls<'a>( r#type: type_expr, decorators: build_decorator_metadata_array( &allocator, - &[decorator], + &angular_decorators, Some(source), None, None, + None, Some(string_consts), ), ctor_parameters: build_ctor_params_metadata_in( @@ -2778,9 +2792,24 @@ pub fn transform_angular_file( // Add class metadata for TestBed support (after debug info, before HMR) // Only emit when enabled and not in advanced optimizations mode if options.emit_class_metadata && !options.advanced_optimizations { - if let Some(decorator) = - find_component_decorator(&class.decorators, &string_consts) - { + // List ALL of the class's `@angular/core` decorators, in + // source order — ngtsc's `extractClassMetadata` filters on + // `isAngularDecorator` (imported from `@angular/core`), so + // a second `@Component` (kept on the emitted class like + // ngtsc's `__decorate` entry) or a co-located `@Injectable` + // appears alongside the decorator that was compiled + // (issue #521). + let angular_decorators: std::vec::Vec<_> = class + .decorators + .iter() + .filter(|d| { + crate::directive::is_angular_core_decorator( + d, + Some(&string_consts), + ) + }) + .collect(); + if !angular_decorators.is_empty() { let emitter = JsEmitter::new(); // Build the type expression: reference to the class @@ -2804,10 +2833,11 @@ pub fn transform_angular_file( r#type: type_expr, decorators: build_decorator_metadata_array( &allocator, - &[decorator], + &angular_decorators, Some(source), Some(template), Some(metadata.styles.as_slice()), + component_decorator, Some(&string_consts), ), ctor_parameters: build_ctor_params_metadata_in( @@ -3078,22 +3108,16 @@ pub fn transform_angular_file( // Emit setClassMetadata for TestBed support (overrideDirective + // signal members), mirroring the @Component path. - let decls_after_class = - find_directive_decorator(&class.decorators, Some(&string_consts)) - .map(|decorator| { - build_set_class_metadata_decls( - &allocator, - class, - &class_name, - decorator, - options, - source, - &string_consts, - &import_map, - &mut file_namespace_registry, - ) - }) - .unwrap_or_default(); + let decls_after_class = build_set_class_metadata_decls( + &allocator, + class, + &class_name, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); class_positions.push(( class_name.clone(), @@ -3194,22 +3218,16 @@ pub fn transform_angular_file( )); // Emit setClassMetadata for TestBed support (overridePipe). - let decls_after_class = - find_pipe_decorator(&class.decorators, Some(&string_consts)) - .map(|decorator| { - build_set_class_metadata_decls( - &allocator, - class, - &class_name, - decorator, - options, - source, - &string_consts, - &import_map, - &mut file_namespace_registry, - ) - }) - .unwrap_or_default(); + let decls_after_class = build_set_class_metadata_decls( + &allocator, + class, + &class_name, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); class_positions.push(( class_name.clone(), @@ -3324,26 +3342,21 @@ pub fn transform_angular_file( // Emit setClassMetadata for TestBed support (overrideModule), // appended after the NgModule's external declarations. - if let Some(decorator) = - find_ng_module_decorator(&class.decorators, Some(&string_consts)) - { - let metadata = build_set_class_metadata_decls( - &allocator, - class, - &class_name, - decorator, - options, - source, - &string_consts, - &import_map, - &mut file_namespace_registry, - ); - if !metadata.is_empty() { - if !external_decls.is_empty() { - external_decls.push('\n'); - } - external_decls.push_str(&metadata); + let metadata = build_set_class_metadata_decls( + &allocator, + class, + &class_name, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); + if !metadata.is_empty() { + if !external_decls.is_empty() { + external_decls.push('\n'); } + external_decls.push_str(&metadata); } // NgModule: external_decls go AFTER the class (they reference the class name) @@ -3433,7 +3446,6 @@ pub fn transform_angular_file( &allocator, class, &class_name, - service_decorator, options, source, &string_consts, @@ -3511,22 +3523,16 @@ pub fn transform_angular_file( )); // Emit setClassMetadata for TestBed support. - let decls_after_class = - find_injectable_decorator(&class.decorators, Some(&string_consts)) - .map(|decorator| { - build_set_class_metadata_decls( - &allocator, - class, - &class_name, - decorator, - options, - source, - &string_consts, - &import_map, - &mut file_namespace_registry, - ) - }) - .unwrap_or_default(); + let decls_after_class = build_set_class_metadata_decls( + &allocator, + class, + &class_name, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); class_positions.push(( class_name.clone(), @@ -5598,6 +5604,179 @@ export class SecondComponent {} ); } + /// The decorators array inside `ɵsetClassMetadata(...)` on a single line, + /// for whitespace-insensitive comparison. + fn set_class_metadata_decorators(code: &str) -> String { + let start = code.find("ɵsetClassMetadata(").expect("code should contain ɵsetClassMetadata"); + code[start..].split_whitespace().collect() + } + + #[test] + fn test_duplicate_component_decorators() { + // Issue #521: two @Component decorators on one class. ngtsc gives no + // diagnostic — it compiles the FIRST @Component, leaves the second on + // the class (downleveled by TS into `__decorate`), and lists BOTH in + // `setClassMetadata`'s decorators array. + let allocator = Allocator::default(); + let source = r#"import {Component} from '@angular/core'; +@Component({selector: 'a-cmp', template: 'first'}) +@Component({selector: 'b-cmp', template: 'second'}) +export class C {} +"#; + + let result = transform_angular_file(&allocator, "c.ts", source, None, None); + + assert_eq!(result.component_count, 1); + assert!(!result.has_errors(), "diagnostics: {:?}", result.diagnostics); + + // The first decorator is compiled: ɵcmp selects 'a-cmp' with the first template. + assert!( + result.code.contains("selectors:[[\"a-cmp\"]]"), + "ɵcmp should compile the first @Component, but got:\n{}", + result.code + ); + + // The second decorator stays on the emitted class (like ngtsc's + // __decorate entry for the decorator the compiler did not take). + assert!( + result.code.contains("@Component({selector: 'b-cmp', template: 'second'})"), + "the second @Component should be preserved on the class, but got:\n{}", + result.code + ); + + // setClassMetadata lists BOTH decorators in source order. + let metadata = set_class_metadata_decorators(&result.code); + assert!( + metadata.contains( + "ɵsetClassMetadata(C,[{type:Component,args:[{selector:\"a-cmp\",template:\"first\"}]},{type:Component,args:[{selector:\"b-cmp\",template:\"second\"}]}]" + ), + "setClassMetadata should list both @Component decorators, but got:\n{}", + result.code + ); + } + + #[test] + fn test_duplicate_directive_decorators() { + // Same as the @Component duplicate: ngtsc compiles the first + // @Directive and lists both in setClassMetadata. + let allocator = Allocator::default(); + let source = r#"import {Directive} from '@angular/core'; +@Directive({selector: '[a-dir]'}) +@Directive({selector: '[b-dir]'}) +export class D {} +"#; + + let result = transform_angular_file(&allocator, "d.ts", source, None, None); + + assert!(!result.has_errors(), "diagnostics: {:?}", result.diagnostics); + assert!( + result.code.contains("selectors:[[\"\",\"a-dir\",\"\"]]"), + "ɵdir should compile the first @Directive, but got:\n{}", + result.code + ); + assert!( + result.code.contains("@Directive({selector: '[b-dir]'})"), + "the second @Directive should be preserved on the class, but got:\n{}", + result.code + ); + let metadata = set_class_metadata_decorators(&result.code); + assert!( + metadata.contains( + "[{type:Directive,args:[{selector:\"[a-dir]\"}]},{type:Directive,args:[{selector:\"[b-dir]\"}]}]" + ), + "setClassMetadata should list both @Directive decorators, but got:\n{}", + result.code + ); + } + + #[test] + fn test_duplicate_injectable_decorators() { + // A standalone @Injectable lists both decorators in setClassMetadata. + let allocator = Allocator::default(); + let source = r#"import {Injectable} from '@angular/core'; +@Injectable({providedIn: 'root'}) +@Injectable({providedIn: 'platform'}) +export class S {} +"#; + + let result = transform_angular_file(&allocator, "s.ts", source, None, None); + + assert!(!result.has_errors(), "diagnostics: {:?}", result.diagnostics); + assert!( + result.code.contains("@Injectable({providedIn: 'platform'})"), + "the second @Injectable should be preserved on the class, but got:\n{}", + result.code + ); + let metadata = set_class_metadata_decorators(&result.code); + assert!( + metadata.contains( + "[{type:Injectable,args:[{providedIn:\"root\"}]},{type:Injectable,args:[{providedIn:\"platform\"}]}]" + ), + "setClassMetadata should list both @Injectable decorators, but got:\n{}", + result.code + ); + } + + #[test] + fn test_co_located_injectable_decorator_in_class_metadata() { + // ngtsc's `isAngularDecorator` filter admits every decorator imported + // from `@angular/core`, so a co-located @Injectable is listed in + // setClassMetadata too (it is also the one removed alongside the + // compiled @Component's). + let allocator = Allocator::default(); + let source = r#"import {Component, Injectable} from '@angular/core'; +@Component({selector: 'a-cmp', template: 'x'}) +@Injectable() +export class C {} +"#; + + let result = transform_angular_file(&allocator, "c.ts", source, None, None); + + assert!(!result.has_errors(), "diagnostics: {:?}", result.diagnostics); + assert!( + !result.code.contains("@Injectable"), + "the @Injectable decorator is compiled into ɵprov and removed, but got:\n{}", + result.code + ); + let metadata = set_class_metadata_decorators(&result.code); + assert!( + metadata.contains( + "[{type:Component,args:[{selector:\"a-cmp\",template:\"x\"}]},{type:Injectable}]" + ), + "setClassMetadata should list @Component and @Injectable, but got:\n{}", + result.code + ); + } + + #[test] + fn test_non_angular_decorator_not_in_class_metadata() { + // A decorator not imported from `@angular/core` is not Angular's: + // ngtsc's `isAngularDecorator` filter excludes it from setClassMetadata + // while the decorator itself stays on the emitted class. + let allocator = Allocator::default(); + let source = r#"import {Component} from '@angular/core'; +declare function MyDec(): ClassDecorator; +@Component({selector: 'a-cmp', template: 'x'}) +@MyDec() +export class C {} +"#; + + let result = transform_angular_file(&allocator, "c.ts", source, None, None); + + assert!(!result.has_errors(), "diagnostics: {:?}", result.diagnostics); + assert!( + result.code.contains("@MyDec()"), + "the non-Angular decorator should be preserved on the class, but got:\n{}", + result.code + ); + let metadata = set_class_metadata_decorators(&result.code); + assert!( + !metadata.contains("MyDec"), + "setClassMetadata should not list a non-Angular decorator, but got:\n{}", + result.code + ); + } + #[test] fn test_transform_with_host_bindings() { let allocator = Allocator::default(); From e6069d25b60fc570261d36cc4785b03941f2fd3c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:54:36 +0800 Subject: [PATCH 09/14] fix(compiler): report Angular io/query decorators on static members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc's extractDirectiveMetadata rejects inputs, outputs and queries on static members with INCORRECTLY_DECLARED_ON_STATIC_MEMBER (shared.ts parseInputFields/parseOutputFields/parseQueriesOfClassFields), while @HostBinding/@HostListener on statics are ignored (filterToMembersWithDecorator drops static members upstream, so they never reach the io/query parsers either). decorator_io_errors now mirrors those checks for @Component/@Directive — and @Pipe, whose upstream handler runs the same extraction: - `Input "x" is incorrectly declared as static member of "D".` for an @Input decorator or an input()/model() initializer on a static member, after the decorator's own checks, like upstream. - `Output is incorrectly declared on a static class member.` for an @Output decorator (methods included, which upstream also accepts) or an output()/outputFromObservable()/model() initializer on a static member, on the decorator or the call. tryParseInitializerBasedOutput's rejection of output.required() is mirrored too, before the static check. - `Query is incorrectly declared on a static class member.` for a @ViewChild/@ViewChildren/@ContentChild/@ContentChildren decorator or a viewChild()/viewChildren()/contentChild()/contentChildren() initializer on a static member, and the signal-query errors that come first upstream ('No locator specified.', 'Argument needs to be an object literal.', and the @ViewChild + signal query collision). Fixes #547 --- .../src/directive/decorator.rs | 82 +++++-- .../src/directive/property_decorators.rs | 76 ++++-- .../tests/integration_test.rs | 217 +++++++++++++++++- 3 files changed, 345 insertions(+), 30 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 4ebbf81b3..176b15153 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -1041,9 +1041,10 @@ fn upsert_input<'a>(inputs: &mut Vec<'a, R3InputMetadata<'a>>, input: R3InputMet upsert_meta(inputs, input, |i| i.class_property_name.as_str()); } -/// Angular's `@Component` / `@Directive` decorator on `class` (imported from -/// `@angular/core`, in the file `consts` was collected from), its metadata -/// object (if any) and its name. +/// Angular's `@Component` / `@Directive` / `@Pipe` decorator on `class` +/// (imported from `@angular/core`, in the file `consts` was collected from), +/// its metadata object (if any) and its name. `@Pipe` is here because ngtsc +/// runs `extractDirectiveMetadata` — and its io/query checks — for pipes too. pub(crate) fn angular_decorator_config<'a>( class: &'a Class<'a>, consts: &StringConsts<'_>, @@ -1052,6 +1053,9 @@ pub(crate) fn angular_decorator_config<'a>( .map(|d| (d, "Component")) .or_else(|| { find_directive_decorator(&class.decorators, Some(consts)).map(|d| (d, "Directive")) + }) + .or_else(|| { + crate::pipe::find_pipe_decorator(&class.decorators, Some(consts)).map(|d| (d, "Pipe")) })?; let config = match &decorator.expression { Expression::CallExpression(call) => match call.arguments.first() { @@ -1091,12 +1095,17 @@ pub fn decorator_io_errors<'a>( }; let evaluator = Evaluator::new(consts); + let class_name = class.id.as_ref().map_or(String::new(), |id| id.name.to_string()); let input_members = || { class.body.body.iter().find_map(|element| { - let (key, decorators, value) = match element { - ClassElement::PropertyDefinition(p) => (&p.key, &p.decorators, p.value.as_ref()), - ClassElement::AccessorProperty(p) => (&p.key, &p.decorators, p.value.as_ref()), - ClassElement::MethodDefinition(m) => (&m.key, &m.decorators, None), + let (key, decorators, value, is_static) = match element { + ClassElement::PropertyDefinition(p) => { + (&p.key, &p.decorators, p.value.as_ref(), p.r#static) + } + ClassElement::AccessorProperty(p) => { + (&p.key, &p.decorators, p.value.as_ref(), p.r#static) + } + ClassElement::MethodDefinition(m) => (&m.key, &m.decorators, None, m.r#static), _ => return None, }; let name = key.static_name()?; @@ -1122,6 +1131,22 @@ pub fn decorator_io_errors<'a>( if error.is_some() { return error; } + // ngtsc's `parseInputFields` rejects an input on a static member + // once the mapping parsed (INCORRECTLY_DECLARED_ON_STATIC_MEMBER): + // an `@Input` decorator or an `input()`/`model()` initializer. + if is_static { + let mapped = decorator.is_some() + || value.is_some_and(|value| { + is_initializer_api_call(value, consts, &[INPUT_API, MODEL_API]) + }); + if mapped { + let message = format!( + "Input \"{name}\" is incorrectly declared as static member of \ + \"{class_name}\"." + ); + return Some((message, element.span())); + } + } // A signal input only collides with a metadata entry of the same name. let value = value.filter(|_| meta_inputs.contains(&name.as_ref()))?; let is_input = is_initializer_api_call(value, consts, &[INPUT_API, MODEL_API]); @@ -1135,12 +1160,17 @@ pub fn decorator_io_errors<'a>( }; let output_members = || { class.body.body.iter().find_map(|element| { - // `@Output(...)`, as ngtsc's `tryParseDecoratorOutput` reads it, on - // the members an output is compiled from. - let (decorators, value) = match element { - ClassElement::PropertyDefinition(p) => (Some(&p.decorators), p.value.as_ref()), - ClassElement::AccessorProperty(p) => (Some(&p.decorators), p.value.as_ref()), - _ => (None, None), + // `@Output(...)`, as ngtsc's `tryParseDecoratorOutput` reads it; it + // accepts any member kind, so a decorated (static) method counts. + let (decorators, value, is_static) = match element { + ClassElement::PropertyDefinition(p) => { + (Some(&p.decorators), p.value.as_ref(), p.r#static) + } + ClassElement::AccessorProperty(p) => { + (Some(&p.decorators), p.value.as_ref(), p.r#static) + } + ClassElement::MethodDefinition(m) => (Some(&m.decorators), None, m.r#static), + _ => (None, None, false), }; let decorator = decorators.and_then(|decorators| { super::property_decorators::member_decorator(decorators, "Output", consts) @@ -1148,6 +1178,15 @@ pub fn decorator_io_errors<'a>( if let Some(error) = decorator.and_then(|d| output_decorator_error(d, &evaluator)) { return Some(error); } + // ngtsc's `tryParseInitializerBasedOutput` rejects `output.required()` + // while parsing the member, before the checks below. + if let Some((_, true, call)) = initializer_api_call( + value?, + Some(consts), + &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API], + ) { + return Some(("Output does not support \".required()\".".to_string(), call.span)); + } // Then `@Output` on an `output()` or a model, like ngtsc's // `parseOutputFields`. if let (Some(decorator), Some(value)) = (decorator, value) { @@ -1163,6 +1202,23 @@ pub fn decorator_io_errors<'a>( return Some((message.to_string(), decorator.span)); } } + // ngtsc's `parseOutputFields` rejects an output on a static member + // (INCORRECTLY_DECLARED_ON_STATIC_MEMBER): an `@Output` decorator + // or an `output()`/`outputFromObservable()`/`model()` initializer, + // on the decorator or the call. + if is_static { + let apis = [OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, MODEL_API]; + let node = decorator.map(|d| d.span).or_else(|| { + value + .and_then(|value| initializer_api_call(value, Some(consts), &apis)) + .map(|(_, _, call)| call.span) + }); + if let Some(span) = node { + let message = + "Output is incorrectly declared on a static class member.".to_string(); + return Some((message, span)); + } + } let ClassElement::PropertyDefinition(prop) = element else { return None }; let (value, name) = (prop.value.as_ref()?, prop.key.static_name()?); if !meta_outputs.contains(&name.as_ref()) { diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index ab929dc38..9d21e58bb 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -1928,9 +1928,12 @@ fn member_query<'a>( decorator_query(allocator, consts, &evaluator, name, args, span, "", source_text) } -/// The first error ngtsc raises for a class's query member decorators -/// (`parseQueriesOfClassFields`), in member order: the one -/// [`member_query`] reports, on the node ngtsc points at. +/// The first error ngtsc raises for a class's query members +/// (`parseQueriesOfClassFields`), in member order: the one [`member_query`] +/// reports for a decorator, the ones `tryParseSignalQueryFromInitializer` +/// reports for a signal initializer, or — once a member is a query — it being +/// on a static member (`INCORRECTLY_DECLARED_ON_STATIC_MEMBER`), each on the +/// node ngtsc points at. pub(crate) fn member_query_error<'a>( allocator: &'a Allocator, class: &'a Class<'a>, @@ -1938,19 +1941,66 @@ pub(crate) fn member_query_error<'a>( consts: &super::StringConsts<'a>, ) -> Option<(String, Span)> { class.body.body.iter().find_map(|element| { - let (decorators, span) = match element { - ClassElement::PropertyDefinition(prop) => (&prop.decorators, prop.span), - ClassElement::MethodDefinition(method) - if matches!(method.kind, MethodDefinitionKind::Set | MethodDefinitionKind::Get) => - { - (&method.decorators, method.span) + let (decorators, value, is_static, span) = match element { + ClassElement::PropertyDefinition(prop) => { + (&prop.decorators, prop.value.as_ref(), prop.r#static, prop.span) + } + ClassElement::AccessorProperty(accessor) => { + (&accessor.decorators, accessor.value.as_ref(), accessor.r#static, accessor.span) + } + ClassElement::MethodDefinition(method) => { + (&method.decorators, None, method.r#static, method.span) } _ => return None, }; - let (decorator, name) = QUERY_TYPES.iter().find_map(|name| { - Some((find_decorator_by_name(decorators, name, Some(consts))?, *name)) - })?; - member_query(allocator, decorator, name, span, source_text, consts).err() + let decorator = QUERY_TYPES.iter().find_map(|name| { + find_decorator_by_name(decorators, name, Some(consts)).zip(Some(*name)) + }); + // A query decorator's own errors come first (`tryGetQueryFromFieldDecorator`). + if let Some((decorator, name)) = decorator + && let Some(error) = + member_query(allocator, decorator, name, span, source_text, consts).err() + { + return Some(error); + } + // Then the signal query's own errors (`tryParseSignalQueryFromInitializer`). + let signal = value.and_then(|value| { + super::decorator::initializer_api_call( + value, + Some(consts), + &super::decorator::QUERY_APIS, + ) + }); + if let Some((_, _, call)) = signal { + if call.arguments.first().and_then(Argument::as_expression).is_none() { + return Some(("No locator specified.".to_string(), call.span)); + } + if let Some(options) = call.arguments.get(1) + && !matches!(options, Argument::ObjectExpression(_)) + { + return Some(( + "Argument needs to be an object literal.".to_string(), + options.span(), + )); + } + } + // ngtsc rejects `@ViewChild` (and friends) with a signal query initializer. + if let (Some((decorator, name)), Some(_)) = (decorator, signal) { + let message = format!("Using @{name} with a signal-based query is not allowed."); + return Some((message, decorator.span)); + } + // A query on a static member (INCORRECTLY_DECLARED_ON_STATIC_MEMBER), + // on the decorator or the call. + if is_static { + let node = decorator + .map(|(decorator, _)| decorator.span) + .or(signal.map(|(_, _, call)| call.span)); + if let Some(span) = node { + let message = "Query is incorrectly declared on a static class member.".to_string(); + return Some((message, span)); + } + } + None }) } diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..b9394e0cc 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -6144,8 +6144,8 @@ export class TestComponent { fn test_set_class_metadata_prop_decorators_member_shape() { let allocator = Allocator::default(); let source = r" -import { Component, Input } from '@angular/core'; -import { input } from '@angular/core'; +import { Component, HostBinding, Input } from '@angular/core'; +import { signal } from '@angular/core'; @Component({ selector: 'test-comp', @@ -6154,10 +6154,10 @@ import { input } from '@angular/core'; }) export class TestComponent { @Input() instanceProp: any; - @Input() static staticProp: any; + @HostBinding('class.b') static staticProp: any; @Input() #priv: any; @Input() 'str-key': any; - static statSignal = input(0); + static statSignal = signal(0); } "; @@ -15485,3 +15485,212 @@ export class CounterService {} decl.members ); } + +// ============================================================================ +// Angular decorators on `static` members (INCORRECTLY_DECLARED_ON_STATIC_MEMBER) +// ============================================================================ +// ngtsc's `extractDirectiveMetadata` rejects inputs, outputs and queries on +// static members (shared.ts `parseInputFields` / `parseOutputFields` / +// `parseQueriesOfClassFields`). `decorator_io_errors` mirrors those checks for +// @Component/@Directive/@Pipe classes. HostBinding/HostListener on statics are +// ignored upstream (`filterToMembersWithDecorator` drops static members), so +// they stay silent. + +fn expect_diagnostics(source: &str) -> Vec { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + result.diagnostics.iter().map(|d| format!("{d}")).collect() +} + +#[test] +fn test_static_input_member_is_diagnostic() { + // `@Input` on a static field, setter and signal/model initializer all map + // to the same upstream error, with the member and class names. + for member in [ + "@Input() static x = 0;", + "@Input() static set x(v: number) {}", + "static x = input(0);", + "static x = input.required();", + "static x = model(0);", + ] { + let source = format!( + "import {{ Directive, Input, input, model }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + let expected = "Input \"x\" is incorrectly declared as static member of \"D\"."; + assert!( + diagnostics.iter().any(|d| d.contains(expected)), + "`{member}` should report {expected:?}. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_static_input_on_component_and_pipe_is_diagnostic() { + // extractDirectiveMetadata runs for @Component and @Pipe too. + for decorator in ["@Component({ selector: 'c', template: '' })", "@Pipe({ name: 'p' })"] { + let source = format!( + "import {{ Component, Directive, Input, Pipe }} from '@angular/core';\n\ + {decorator}\n\ + export class C {{\n @Input() static x = 0;\n}}" + ); + let diagnostics = expect_diagnostics(&source); + let expected = "Input \"x\" is incorrectly declared as static member of \"C\"."; + assert!( + diagnostics.iter().any(|d| d.contains(expected)), + "{decorator} should report {expected:?}. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_static_output_member_is_diagnostic() { + // `model()` is covered by the input test: ngtsc's `parseInputFields` runs + // first, so a static model reports the Input error, not the Output one. + for member in [ + "@Output() static y = new EventEmitter();", + "static y = output();", + "static y = outputFromObservable(of(0));", + ] { + let source = format!( + "import {{ Directive, EventEmitter, Output, model, output }} from '@angular/core';\n\ + import {{ outputFromObservable }} from '@angular/core/rxjs-interop';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + let expected = "Output is incorrectly declared on a static class member."; + assert!( + diagnostics.iter().any(|d| d.contains(expected)), + "`{member}` should report {expected:?}. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_static_query_member_is_diagnostic() { + for member in [ + "@ViewChild('el') static z: any;", + "@ViewChildren('el') static z: any;", + "@ContentChild('el') static z: any;", + "@ContentChildren('el') static z: any;", + "static z = viewChild('el');", + "static z = viewChildren('el');", + "static z = contentChild.required('el');", + "static z = contentChildren('el');", + ] { + let source = format!( + "import {{ Component, ContentChild, ContentChildren, ViewChild, ViewChildren, \ + contentChild, contentChildren, viewChild, viewChildren }} from '@angular/core';\n\ + @Component({{ selector: 'c', template: '' }})\n\ + export class C {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + let expected = "Query is incorrectly declared on a static class member."; + assert!( + diagnostics.iter().any(|d| d.contains(expected)), + "`{member}` should report {expected:?}. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_static_host_binding_and_listener_produce_no_diagnostic() { + // ngtsc's `filterToMembersWithDecorator` drops static members before host + // metadata is collected: @HostBinding/@HostListener on statics are ignored + // upstream, not an error (#544), so no diagnostic is emitted for them. + let allocator = Allocator::default(); + let source = r" +import { Directive, HostBinding, HostListener } from '@angular/core'; + +@Directive({ selector: '[d]' }) +export class D { + @HostBinding('class.b') static b = true; + @HostListener('scroll') static onScroll() {} +} +"; + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!( + !result.has_errors(), + "@HostBinding/@HostListener on statics must not produce a diagnostic: {:?}", + result.diagnostics + ); +} + +#[test] +fn test_static_member_checks_keep_ngtsc_error_order() { + // `tryParseInputFieldMapping` reports a decorator+initializer collision + // before `parseInputFields` gets to the static check. + let diagnostics = expect_diagnostics( + "import { Directive, Input, input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Input() static x = input(0);\n}", + ); + assert!( + diagnostics.iter().any(|d| d.contains("Using @Input with a signal input is not allowed.")), + "@Input + input() should report the collision, not the static error. Got: {diagnostics:?}" + ); + + // `tryParseInitializerBasedOutput` rejects output.required() before the + // static check (INITIALIZER_API_NO_REQUIRED_FUNCTION). + let diagnostics = expect_diagnostics( + "import { Directive, output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n static y = output.required();\n}", + ); + assert!( + diagnostics.iter().any(|d| d.contains("Output does not support \".required()\".")), + "output.required() should report the required() error. Got: {diagnostics:?}" + ); + + // `tryParseSignalQueryFromInitializer` reports its own errors and the + // decorator+signal collision before the static check. + let diagnostics = expect_diagnostics( + "import { Directive, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n static z = viewChild();\n}", + ); + assert!( + diagnostics.iter().any(|d| d.contains("No locator specified.")), + "viewChild() without a locator should error. Got: {diagnostics:?}" + ); + let diagnostics = expect_diagnostics( + "import { Directive, ViewChild, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @ViewChild('a') static z = viewChild('b');\n}", + ); + assert!( + diagnostics + .iter() + .any(|d| d.contains("Using @ViewChild with a signal-based query is not allowed.")), + "@ViewChild + viewChild() should report the collision. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_instance_input_output_query_members_have_no_static_diagnostic() { + // The same declarations without `static` stay valid. + let allocator = Allocator::default(); + let source = r" +import { Component, EventEmitter, Input, Output, ViewChild, input, output, viewChild } + from '@angular/core'; + +@Component({ selector: 'c', template: '' }) +export class C { + @Input() x = 0; + @Output() y = new EventEmitter(); + @ViewChild('el') z: any; + xi = input(0); + yo = output(); + vq = viewChild('el'); +} +"; + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!( + !result.has_errors(), + "instance members must not produce the static-member diagnostic: {:?}", + result.diagnostics + ); +} From 57bd77923f8edccbff3bf0a0283701221aa4be30 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 21:57:25 +0800 Subject: [PATCH 10/14] feat(compiler): honor `jit: true` in decorator metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@Component({jit: true})`, `@Directive({jit: true})` and `@NgModule({jit: true})` opt the class out of AOT compilation. ngtsc returns jitForced from metadata extraction, produces no analysis (no ɵcmp/ɵdir/ɵmod/ɵfac/ɵsetClassMetadata) and registers the class in jitDeclarationRegistry so the JIT application transform downlevels its decorators. The AOT path now detects the jit-forced primary decorator and lowers the class exactly like the JIT mode pipeline: decorators are collected into a tslib __decorate() call (templateUrl/styleUrl(s) rewritten to angular:jit:file; imports), constructor parameters become static ctorParameters, member decorators become static propDecorators or __decorate() member calls, and the class is restructured as a class expression with a trailing re-export. The per-class collection/emission is shared with transform_angular_file_jit via collect_jit_class_info and jit_class_edits. Details handled to match ngtsc + TypeScript emit: - A co-located @Injectable still compiles (upstream's InjectableHandler is independent of the jitForced short-circuit): ɵfac/ɵprov statics and the injectable setClassMetadata/d.ts survive. - @Pipe/@Injectable/@Service have no upstream jit opt-out and are unaffected; dispatch order (Component→Directive→Pipe→NgModule) is respected so a jit property on a decorator that doesn't own the class can't skip compilation. - Import elision preserves symbols that only appear in constructor parameters of jit-forced classes, since they are re-emitted inside ctorParameters/__param — the same result TypeScript's elision produces on ngtsc's rewritten AST. - tslib __decorate/__param imports are only added when the file doesn't already import them, and import * as i0 is only emitted when the jit-forced class actually references the @angular/core namespace (synthesized signal-API propDecorators or injectable statics). Fixes #515 --- .../src/component/import_elision.rs | 13 + .../src/component/transform.rs | 696 +++++++++++++----- .../tests/integration_test.rs | 279 +++++++ 3 files changed, 812 insertions(+), 176 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 7c91bcf07..85f5a53e7 100644 --- a/crates/oxc_angular_compiler/src/component/import_elision.rs +++ b/crates/oxc_angular_compiler/src/component/import_elision.rs @@ -694,6 +694,19 @@ impl<'a> ImportElisionAnalyzer<'a> { self.type_only_specifiers.contains(&Ident::from(name)) } + /// Keep `name`'s import even though the source only references it in + /// positions the analysis marks elidable. + /// + /// `jit: true` classes are downleveled like JIT-mode output: constructor + /// parameter decorators and types move into `ctorParameters`/`__param`, + /// so they remain value references in the emitted code even though the + /// source position made them look removable. Upstream sees the same + /// result because TypeScript's import elision runs after ngtsc's + /// transforms, on the rewritten AST. + pub fn preserve(&mut self, name: &'a str) { + self.type_only_specifiers.remove(&Ident::from(name)); + } + /// Get the set of type-only specifier names. pub fn type_only_specifiers(&self) -> &FxHashSet> { &self.type_only_specifiers diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 5c7a261b2..4dd117960 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -1062,6 +1062,69 @@ fn find_angular_decorator<'a>( None } +/// Whether `decorator`'s options object opts out of AOT compilation via +/// `jit: true`. +/// +/// Mirrors ngtsc's `extractDirectiveMetadata` / `NgModuleDecoratorHandler.analyze` +/// (`shared.ts:176`, `ng_module/handler.ts:352`): presence of the `jit` +/// property forces JIT — the interface only allows `true`, so any value that +/// isn't statically `false` is treated as opt-out. +fn decorator_forces_jit(decorator: &oxc_ast::ast::Decorator<'_>) -> bool { + let Expression::CallExpression(call) = &decorator.expression else { return false }; + let Some(Argument::ObjectExpression(config)) = call.arguments.first() else { + return false; + }; + for prop in &config.properties { + let ObjectPropertyKind::ObjectProperty(prop) = prop else { continue }; + if !crate::util::is_metadata_property(prop) { + continue; + } + let is_jit = match &prop.key { + PropertyKey::StaticIdentifier(id) => id.name.as_str() == "jit", + PropertyKey::StringLiteral(s) => s.value.as_str() == "jit", + _ => false, + }; + if is_jit { + return !matches!(&prop.value, Expression::BooleanLiteral(b) if !b.value); + } + } + false +} + +/// The `@Component`/`@Directive`/`@NgModule` decorator on `class` that sets +/// `jit: true`, if any. +/// +/// Checked in the same order the compilation dispatch below picks a decorator: +/// `@Component` → `@Directive` → `@Pipe` → `@NgModule` → `@Service` → +/// `@Injectable`. A `@Pipe` in between stops the search because the pipe +/// branch compiles the class and upstream's `PipeHandler` has no `jit` opt-out; +/// `@Service`/`@Injectable` don't either. +fn find_jit_forced_decorator<'a>( + class: &'a oxc_ast::ast::Class<'a>, + string_consts: &crate::directive::StringConsts<'_>, +) -> Option<(AngularDecoratorKind, &'a oxc_ast::ast::Decorator<'a>)> { + let decorators = &class.decorators; + let consts = Some(string_consts); + if let Some(decorator) = find_component_decorator(decorators, string_consts) { + return decorator_forces_jit(decorator) + .then_some((AngularDecoratorKind::Component, decorator)); + } + if let Some(decorator) = find_directive_decorator(decorators, consts) { + return decorator_forces_jit(decorator) + .then_some((AngularDecoratorKind::Directive, decorator)); + } + // Upstream's PipeHandler has no `jit` opt-out: a @Pipe always compiles, + // and its presence wins the dispatch below regardless of other decorators. + if find_pipe_decorator(decorators, consts).is_some() { + return None; + } + if let Some(decorator) = find_ng_module_decorator(decorators, consts) { + return decorator_forces_jit(decorator) + .then_some((AngularDecoratorKind::NgModule, decorator)); + } + None +} + /// Extract constructor parameter info for JIT ctorParameters generation. /// /// Like Angular's JIT transform (`downlevel_decorators_transform.ts`), a @@ -1350,6 +1413,19 @@ fn classify_initializer_api( /// an undefined identifier and throw `ReferenceError` at module-evaluation time. pub(crate) const JIT_ANGULAR_CORE_NS: &str = "i0"; +/// Identifiers referenced inside constructor parameters — types and +/// decorators. Used for jit-forced classes, whose parameters are re-emitted +/// as `ctorParameters`/`__param` entries and therefore keep their imports +/// live (TypeScript's import elision likewise sees the rewritten AST). +#[derive(Default)] +struct JitCtorParamIdentifiers<'a>(rustc_hash::FxHashSet<&'a str>); + +impl<'a> oxc_ast_visit::Visit<'a> for JitCtorParamIdentifiers<'a> { + fn visit_identifier_reference(&mut self, it: &oxc_ast::ast::IdentifierReference<'a>) { + self.0.insert(it.name.as_str()); + } +} + /// Every identifier a file has, like TypeScript's `SourceFile.identifiers`. #[derive(Default)] struct FileIdentifiers(rustc_hash::FxHashSet); @@ -2016,6 +2092,236 @@ fn strip_typescript(allocator: &Allocator, path: &str, code: &str) -> String { codegen_ret.code } +/// Collect the [`JitClassInfo`] for one class: all class-level decorator spans +/// and texts (the primary Angular decorator's text rewritten for resources, +/// the rest verbatim), constructor parameters, and member decorators. +/// +/// Shared by the whole-file JIT path and the `jit: true` opt-out in the AOT +/// path (`find_jit_forced_decorator`), which upstream registers in +/// `jitDeclarationRegistry` and downlevels through the same JIT transform. +#[allow(clippy::too_many_arguments)] +fn collect_jit_class_info<'a>( + allocator: &'a Allocator, + source: &str, + class: &oxc_ast::ast::Class<'a>, + class_name: String, + stmt_start: u32, + is_exported: bool, + is_default_export: bool, + angular_decorator: &oxc_ast::ast::Decorator<'a>, + decorator_kind: AngularDecoratorKind, + resource_counter: &mut u32, + resource_imports: &mut std::vec::Vec<(String, String)>, + string_consts: &crate::directive::StringConsts<'a>, + core_namespace: &str, +) -> JitClassInfo { + let mut all_class_decorator_spans: std::vec::Vec = std::vec::Vec::new(); + let mut all_class_decorator_texts: std::vec::Vec = std::vec::Vec::new(); + + for dec in &class.decorators { + all_class_decorator_spans.push(dec.span); + + // Check if this is the Angular decorator that needs special text transformation + if dec.span == angular_decorator.span { + let text = build_jit_decorator_text( + allocator, + source, + dec, + decorator_kind, + resource_counter, + resource_imports, + string_consts, + ); + all_class_decorator_texts.push(text); + } else { + // Non-Angular decorator: extract expression text from source (without @) + let expr_start = dec.expression.span().start; + let expr_end = dec.expression.span().end; + all_class_decorator_texts + .push(source[expr_start as usize..expr_end as usize].to_string()); + } + } + + // Extract constructor parameters for ctorParameters, and the other + // parameter decorators for the class's `__decorate`. + let (ctor_params, param_decorator_texts) = + extract_jit_ctor_params(source, class, string_consts); + + // Extract Angular and non-Angular member decorators + let (member_decorators, non_angular_member_decorators) = + extract_all_jit_member_decorators(source, class, string_consts, core_namespace); + + JitClassInfo { + class_name, + all_class_decorator_spans, + stmt_start, + class_start: class.span.start, + class_body_end: class.body.span.end, + is_exported, + is_default_export, + is_abstract: class.r#abstract, + ctor_params, + member_decorators, + all_class_decorator_texts, + param_decorator_texts, + non_angular_member_decorators, + } +} + +/// Generate the text edits that downlevel one class to JIT output: +/// decorator removal, `export class X` → `let X = class X` restructuring, +/// `ctorParameters`/`propDecorators` statics, member/class `__decorate` calls, +/// and the re-export. +/// +/// `extra_statics` (e.g. `static ɵfac/ɵprov` for a class that is also +/// `@Injectable`, which still compiles when `jit: true` opts the primary +/// decorator out) is inserted before `ctorParameters`, matching upstream where +/// the Ivy transform runs first and the JIT transform appends its statics. +/// `after_class_extra` (e.g. the injectable's `setClassMetadata`) is appended +/// after the export statement, matching Ivy's compile-statement placement. +fn jit_class_edits( + source: &str, + class: &oxc_ast::ast::Class<'_>, + jit_info: &JitClassInfo, + extra_statics: &str, + after_class_extra: &str, + edits: &mut std::vec::Vec, +) { + // Remove ALL class-level decorators (including @ and trailing whitespace) + for decorator_span in &jit_info.all_class_decorator_spans { + let mut end = decorator_span.end as usize; + let bytes = source.as_bytes(); + while end < bytes.len() { + let c = bytes[end]; + if c == b' ' || c == b'\t' || c == b'\n' || c == b'\r' { + end += 1; + } else { + break; + } + } + edits.push(Edit::delete(decorator_span.start, end as u32)); + } + + // Remove ALL member decorators and constructor param decorators + { + let mut decorator_spans: std::vec::Vec = std::vec::Vec::new(); + super::decorator::collect_all_constructor_decorator_spans(class, &mut decorator_spans); + super::decorator::collect_all_member_decorator_spans(class, &mut decorator_spans); + for span in &decorator_spans { + let mut end = span.end as usize; + let bytes = source.as_bytes(); + while end < bytes.len() { + let c = bytes[end]; + if c == b' ' || c == b'\t' || c == b'\n' || c == b'\r' { + end += 1; + } else { + break; + } + } + edits.push(Edit::delete(span.start, end as u32)); + } + } + + // Class restructuring: `export class X` → `let X = class X` + // For abstract classes, also strip the `abstract` keyword since class expressions can't be abstract. + let class_keyword_start = if jit_info.is_abstract { + let rest = &source[jit_info.class_start as usize..]; + let offset = rest.find("class").unwrap_or(0); + jit_info.class_start + offset as u32 + } else { + jit_info.class_start + }; + + if jit_info.is_exported || jit_info.is_default_export { + edits.push(Edit::replace( + jit_info.stmt_start, + class_keyword_start, + format!("let {} = ", jit_info.class_name), + )); + } else { + edits.push(Edit::replace( + jit_info.class_start, + class_keyword_start, + format!("let {} = ", jit_info.class_name), + )); + } + + // Add extra (Ivy) statics, then ctorParameters and propDecorators inside + // the class body (before closing `}`). + { + let mut class_statics = String::new(); + if !extra_statics.is_empty() { + class_statics.push_str(&format!("\n{};", extra_statics)); + } + if let Some(ctor_text) = build_ctor_parameters_text(&jit_info.ctor_params) { + class_statics.push_str(&format!("\n{};", ctor_text)); + } + if let Some(prop_text) = build_prop_decorators_text(&jit_info.member_decorators) { + class_statics.push_str(&format!("\n{};", prop_text)); + } + if !class_statics.is_empty() { + class_statics.push('\n'); + edits.push(Edit::insert(jit_info.class_body_end - 1, class_statics)); + } + } + + // After class body, add member __decorate calls, then class __decorate call, then export + let mut after_class = String::from(";\n"); + + // Emit __decorate() for non-Angular member decorators (before class __decorate). + // Match TypeScript's ordering: instance (prototype) members first, then static members. + // Within each group, preserve source declaration order. + for member_dec in jit_info + .non_angular_member_decorators + .iter() + .filter(|m| !m.is_static) + .chain(jit_info.non_angular_member_decorators.iter().filter(|m| m.is_static)) + { + let target = if member_dec.is_static { + jit_info.class_name.clone() + } else { + format!("{}.prototype", jit_info.class_name) + }; + // TypeScript uses `null` for methods/accessors (reads existing descriptor) + // and `void 0` for properties (no existing descriptor). + let desc = if member_dec.is_property { "void 0" } else { "null" }; + after_class.push_str(&format!( + "__decorate([{}], {}, \"{}\", {});\n", + member_dec.decorator_texts.join(", "), + target, + member_dec.member_name, + desc + )); + } + + // Emit class-level __decorate() with ALL class decorators + let all_decorator_text = jit_info + .all_class_decorator_texts + .iter() + .chain(&jit_info.param_decorator_texts) + .map(String::as_str) + .collect::>() + .join(",\n "); + after_class.push_str(&format!( + "{} = __decorate([\n {}\n], {});\n", + jit_info.class_name, all_decorator_text, jit_info.class_name + )); + + if jit_info.is_exported { + after_class.push_str(&format!("export {{ {} }};\n", jit_info.class_name)); + } else if jit_info.is_default_export { + after_class.push_str(&format!("export default {};\n", jit_info.class_name)); + } + + // Ivy compile statements (setClassMetadata for a co-located @Injectable) + // land after the class statement, i.e. after the downleveled export. + if !after_class_extra.is_empty() { + after_class.push_str(after_class_extra); + } + + edits.push(Edit::insert(jit_info.class_body_end, after_class)); +} + /// JIT mode produces output compatible with Angular's JIT runtime compiler: /// - Decorators are downleveled using `__decorate` from tslib /// - `templateUrl` is replaced with `angular:jit:template:file;` imports @@ -2110,58 +2416,21 @@ fn transform_angular_file_jit( continue; } - // Collect ALL class-level decorator spans and texts (in source order) - let mut all_class_decorator_spans: std::vec::Vec = std::vec::Vec::new(); - let mut all_class_decorator_texts: std::vec::Vec = std::vec::Vec::new(); - - for dec in &class.decorators { - all_class_decorator_spans.push(dec.span); - - // Check if this is the Angular decorator that needs special text transformation - if dec.span == angular_decorator.span { - let text = build_jit_decorator_text( - &allocator, - source, - dec, - decorator_kind, - &mut resource_counter, - &mut resource_imports, - &string_consts, - ); - all_class_decorator_texts.push(text); - } else { - // Non-Angular decorator: extract expression text from source (without @) - let expr_start = dec.expression.span().start; - let expr_end = dec.expression.span().end; - all_class_decorator_texts - .push(source[expr_start as usize..expr_end as usize].to_string()); - } - } - - // Extract constructor parameters for ctorParameters, and the other - // parameter decorators for the class's `__decorate`. - let (ctor_params, param_decorator_texts) = - extract_jit_ctor_params(source, class, &string_consts); - - // Extract Angular and non-Angular member decorators - let (member_decorators, non_angular_member_decorators) = - extract_all_jit_member_decorators(source, class, &string_consts, &core_namespace); - - jit_classes.push(JitClassInfo { + jit_classes.push(collect_jit_class_info( + allocator, + source, + class, class_name, - all_class_decorator_spans, stmt_start, - class_start: class.span.start, - class_body_end: class.body.span.end, is_exported, is_default_export, - is_abstract: class.r#abstract, - ctor_params, - member_decorators, - all_class_decorator_texts, - param_decorator_texts, - non_angular_member_decorators, - }); + angular_decorator, + decorator_kind, + &mut resource_counter, + &mut resource_imports, + &string_consts, + &core_namespace, + )); result.component_count += if matches!(decorator_kind, AngularDecoratorKind::Component) { 1 } else { 0 }; @@ -2254,129 +2523,7 @@ fn transform_angular_file_jit( continue; }; - // 4a. Remove ALL class-level decorators (including @ and trailing whitespace) - for decorator_span in &jit_info.all_class_decorator_spans { - let mut end = decorator_span.end as usize; - let bytes = source.as_bytes(); - while end < bytes.len() { - let c = bytes[end]; - if c == b' ' || c == b'\t' || c == b'\n' || c == b'\r' { - end += 1; - } else { - break; - } - } - edits.push(Edit::delete(decorator_span.start, end as u32)); - } - - // 4b. Remove ALL member decorators and constructor param decorators - { - let mut decorator_spans: std::vec::Vec = std::vec::Vec::new(); - super::decorator::collect_all_constructor_decorator_spans(class, &mut decorator_spans); - super::decorator::collect_all_member_decorator_spans(class, &mut decorator_spans); - for span in &decorator_spans { - let mut end = span.end as usize; - let bytes = source.as_bytes(); - while end < bytes.len() { - let c = bytes[end]; - if c == b' ' || c == b'\t' || c == b'\n' || c == b'\r' { - end += 1; - } else { - break; - } - } - edits.push(Edit::delete(span.start, end as u32)); - } - } - - // 4c. Class restructuring: `export class X` → `let X = class X` - // For abstract classes, also strip the `abstract` keyword since class expressions can't be abstract. - let class_keyword_start = if jit_info.is_abstract { - let rest = &source[jit_info.class_start as usize..]; - let offset = rest.find("class").unwrap_or(0); - jit_info.class_start + offset as u32 - } else { - jit_info.class_start - }; - - if jit_info.is_exported || jit_info.is_default_export { - edits.push(Edit::replace( - jit_info.stmt_start, - class_keyword_start, - format!("let {} = ", jit_info.class_name), - )); - } else { - edits.push(Edit::replace( - jit_info.class_start, - class_keyword_start, - format!("let {} = ", jit_info.class_name), - )); - } - - // 4d. Add ctorParameters and propDecorators inside class body (before closing `}`) - { - let mut class_statics = String::new(); - if let Some(ctor_text) = build_ctor_parameters_text(&jit_info.ctor_params) { - class_statics.push_str(&format!("\n{};", ctor_text)); - } - if let Some(prop_text) = build_prop_decorators_text(&jit_info.member_decorators) { - class_statics.push_str(&format!("\n{};", prop_text)); - } - if !class_statics.is_empty() { - class_statics.push('\n'); - edits.push(Edit::insert(jit_info.class_body_end - 1, class_statics)); - } - } - - // 4e. After class body, add member __decorate calls, then class __decorate call, then export - let mut after_class = String::from(";\n"); - - // Emit __decorate() for non-Angular member decorators (before class __decorate). - // Match TypeScript's ordering: instance (prototype) members first, then static members. - // Within each group, preserve source declaration order. - for member_dec in jit_info - .non_angular_member_decorators - .iter() - .filter(|m| !m.is_static) - .chain(jit_info.non_angular_member_decorators.iter().filter(|m| m.is_static)) - { - let target = if member_dec.is_static { - jit_info.class_name.clone() - } else { - format!("{}.prototype", jit_info.class_name) - }; - // TypeScript uses `null` for methods/accessors (reads existing descriptor) - // and `void 0` for properties (no existing descriptor). - let desc = if member_dec.is_property { "void 0" } else { "null" }; - after_class.push_str(&format!( - "__decorate([{}], {}, \"{}\", {});\n", - member_dec.decorator_texts.join(", "), - target, - member_dec.member_name, - desc - )); - } - - // Emit class-level __decorate() with ALL class decorators - let all_decorator_text = jit_info - .all_class_decorator_texts - .iter() - .chain(&jit_info.param_decorator_texts) - .map(String::as_str) - .collect::>() - .join(",\n "); - after_class.push_str(&format!( - "{} = __decorate([\n {}\n], {});\n", - jit_info.class_name, all_decorator_text, jit_info.class_name - )); - - if jit_info.is_exported { - after_class.push_str(&format!("export {{ {} }};\n", jit_info.class_name)); - } else if jit_info.is_default_export { - after_class.push_str(&format!("export default {};\n", jit_info.class_name)); - } - - edits.push(Edit::insert(jit_info.class_body_end, after_class)); + jit_class_edits(source, class, jit_info, "", "", &mut edits); } // Apply all edits @@ -2459,6 +2606,26 @@ pub fn transform_angular_file( // (class_name, effective_start, class_body_end) let mut class_positions: Vec<(String, u32, u32)> = Vec::new(); + // Classes that opted out of AOT via `jit: true` in their decorator + // metadata: downleveled like JIT-mode classes (no Ivy definitions) while + // the rest of the file still compiles AOT. Mirrors ngtsc's + // `jitDeclarationRegistry` + `angularJitApplicationTransform` wiring. + let mut jit_classes: std::vec::Vec = std::vec::Vec::new(); + let mut jit_edits: std::vec::Vec = std::vec::Vec::new(); + let mut jit_resource_counter: u32 = 0; + let mut jit_resource_imports: std::vec::Vec<(String, String)> = std::vec::Vec::new(); + // Lazily computed on the first jit-forced class; the name synthesized + // propDecorators use to reference `@angular/core` (`i0.Input`). + let mut jit_core_namespace: Option<(String, bool)> = None; + // Set when a jit-forced class also carries @Injectable: the emitted + // ɵfac/ɵprov reference `i0`, so the registry's `import * as i0` must be + // emitted even when no AOT-compiled class exists. + let mut jit_emitted_i0_statics = false; + // Import elision must keep symbols referenced by the downleveled output + // (ctorParameters types/decorators, __param entries) even though their + // source positions look elidable; accumulated here for the elision pass. + let mut import_elision_mut = import_elision; + // File-level namespace registry to collect all module imports let mut file_namespace_registry = NamespaceRegistry::new(allocator); @@ -2585,6 +2752,130 @@ pub fn transform_angular_file( } } + // `jit: true` in the decorator opts the class out of AOT + // compilation. ngtsc registers it in `jitDeclarationRegistry`, + // produces no analysis (no ɵcmp/ɵdir/ɵmod/ɵfac/setClassMetadata) + // and downlevels the decorators through the JIT transform + // (compiler.ts:848-870). Same here: the class is lowered like a + // JIT-mode class while the rest of the file still compiles AOT. + if class.id.is_some() + && let Some((jit_kind, jit_decorator)) = + find_jit_forced_decorator(class, &string_consts) + { + let core_namespace = jit_core_namespace + .get_or_insert_with(|| jit_angular_core_namespace(&parser_ret.program)) + .0 + .clone(); + let (is_exported, is_default_export) = match stmt { + Statement::ExportDeclaration(_) => (true, false), + Statement::ExportDefaultDeclaration(_) => (false, true), + _ => (false, false), + }; + let info = collect_jit_class_info( + allocator, + source, + class, + class.id.as_ref().map_or_else(String::new, |id| id.name.to_string()), + stmt_start, + is_exported, + is_default_export, + jit_decorator, + jit_kind, + &mut jit_resource_counter, + &mut jit_resource_imports, + &string_consts, + &core_namespace, + ); + + // A co-located @Injectable still compiles: upstream's + // InjectableHandler runs independently of the jitForced + // short-circuit, so the class keeps its ɵfac/ɵprov (and + // setClassMetadata) alongside the downleveled decorators. + let mut extra_statics = String::new(); + let mut after_class_extra = String::new(); + if let Some(mut injectable_metadata) = extract_injectable_metadata_in( + allocator, + class, + Some(source), + Some(&string_consts), + ) { + // Same namespace resolution the standalone-@Injectable + // branch applies to factory deps. + if let Some(ref mut deps) = injectable_metadata.deps { + resolve_factory_dep_namespaces( + &allocator, + deps, + &import_map, + &mut file_namespace_registry, + ); + } + if let Some(inj_def) = generate_injectable_definition_from_decorator( + &allocator, + &injectable_metadata, + options.compilation_mode, + ) { + jit_emitted_i0_statics = true; + let emitter = JsEmitter::new(); + extra_statics = format!( + "static ɵfac = {};\nstatic ɵprov = {};", + emitter.emit_expression(&inj_def.fac_definition), + emitter.emit_expression(&inj_def.prov_definition) + ); + let type_argument_count = + class.type_parameters.as_ref().map_or(0, |tp| tp.params.len() as u32); + result.dts_declarations.push(dts::generate_injectable_dts( + &injectable_metadata, + type_argument_count, + )); + if let Some(injectable_decorator) = + find_injectable_decorator(&class.decorators, Some(&string_consts)) + { + after_class_extra = build_set_class_metadata_decls( + &allocator, + class, + &info.class_name, + injectable_decorator, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); + } + } + } + + jit_class_edits( + source, + class, + &info, + &extra_statics, + &after_class_extra, + &mut jit_edits, + ); + jit_classes.push(info); + + // Identifiers used only in constructor parameters (types, + // param decorators) survive in the emitted ctorParameters / + // __param, so their imports must be kept — the same result + // TypeScript's elision produces on ngtsc's rewritten AST. + let mut param_idents = JitCtorParamIdentifiers::default(); + for element in &class.body.body { + if let oxc_ast::ast::ClassElement::MethodDefinition(method) = element + && method.kind == oxc_ast::ast::MethodDefinitionKind::Constructor + { + oxc_ast_visit::Visit::visit_formal_parameters( + &mut param_idents, + &method.value.params, + ); + } + } + for name in param_idents.0 { + import_elision_mut.preserve(&name); + } + continue; + } + // Compute implicit_standalone based on Angular version let implicit_standalone = options.implicit_standalone(); @@ -3547,13 +3838,62 @@ pub fn transform_angular_file( // All edits reference positions in the original source and are applied in one pass. // 5a. Import elision edits (collected first for namespace insert position check) - let elision_edits = import_elision_edits(source, &parser_ret.program, &import_elision); + let elision_edits = import_elision_edits(source, &parser_ret.program, &import_elision_mut); // 5b. Namespace import insertion // Must be computed before merging other edits, since we need to check if the // insert position falls inside an import elision edit span. - let namespace_imports = file_namespace_registry.generate_import_statements(); - let ns_edit = if !namespace_imports.is_empty() && !class_definitions.is_empty() { + // + // jit-forced classes additionally need tslib's `__decorate`/`__param`, the + // `@angular/core` namespace for synthesized signal-API propDecorators, and + // the `angular:jit:*` resource imports — same as `transform_angular_file_jit`. + // Helpers the file already imports are skipped so the transform stays + // idempotent. + let mut additional_imports = String::new(); + let mut jit_needs_core_ns = false; + if !jit_classes.is_empty() { + let needs_param = jit_classes.iter().any(|c| !c.param_decorator_texts.is_empty()); + let mut tslib_names: Vec<&str> = + if needs_param { vec!["__decorate", "__param"] } else { vec!["__decorate"] }; + tslib_names.retain(|name| { + !matches!(import_map.get(&Ident::from(*name)), + Some(info) if info.source_module.as_str() == "tslib") + }); + if !tslib_names.is_empty() { + additional_imports + .push_str(&format!("import {{ {} }} from \"tslib\";\n", tslib_names.join(", "))); + } + let (core_namespace, core_namespace_imported) = jit_core_namespace + .clone() + .unwrap_or_else(|| jit_angular_core_namespace(&parser_ret.program)); + jit_needs_core_ns = !core_namespace_imported + && jit_classes_need_angular_core_namespace(&jit_classes, &core_namespace); + // When synthesized references need `@angular/core` under a name other + // than `i0`, it must be imported here — `i0` itself is covered by the + // namespace registry below. + if jit_needs_core_ns && core_namespace != "i0" { + additional_imports + .push_str(&format!("import * as {core_namespace} from \"@angular/core\";\n")); + } + for (import_name, specifier) in &jit_resource_imports { + additional_imports + .push_str(&format!("import {} from \"{}\";\n", import_name, specifier)); + } + } + + // The registry's `import * as i0` (and any `i1…` aliases) is only emitted + // when something references it: compiled definitions, or ɵfac/ɵprov on a + // jit-forced class, or synthesized `i0.*` propDecorators. + let emit_ns_imports = !class_definitions.is_empty() + || jit_emitted_i0_statics + || (jit_needs_core_ns + && jit_core_namespace.map(|(name, _)| name == "i0").unwrap_or_default()); + if emit_ns_imports { + additional_imports.push_str(&file_namespace_registry.generate_import_statements()); + } + let ns_edit = if !additional_imports.is_empty() + && (!class_definitions.is_empty() || !jit_classes.is_empty()) + { let ns_insert_pos = find_last_import_end(&parser_ret.program.body); if let Some(insert_pos) = ns_insert_pos { let bytes = source.as_bytes(); @@ -3581,9 +3921,9 @@ pub fn transform_angular_file( actual_pos = edit.end as usize; } } - Some(Edit::insert(actual_pos as u32, namespace_imports).with_priority(10)) + Some(Edit::insert(actual_pos as u32, additional_imports).with_priority(10)) } else { - Some(Edit::insert(0, namespace_imports).with_priority(10)) + Some(Edit::insert(0, additional_imports).with_priority(10)) } } else { None @@ -3592,6 +3932,10 @@ pub fn transform_angular_file( // 5c. Merge all edits let mut edits: Vec = elision_edits; + // jit-forced class downleveling (decorator removal, `let X = class X` + // restructuring, __decorate calls) — same edits the JIT path emits. + edits.append(&mut jit_edits); + // Decorator removal edits for span in &decorator_spans_to_remove { let mut end = span.end as usize; diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88e797959..6c63bbbac 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -15485,3 +15485,282 @@ export class CounterService {} decl.members ); } + +// ============================================================================ +// Issue #515: `jit: true` in decorator metadata opts a class out of AOT +// ============================================================================ +// ngtsc's extractDirectiveMetadata / NgModuleDecoratorHandler return jitForced +// when the decorator's options object has `jit`, producing no analysis (no +// ɵcmp/ɵdir/ɵmod/ɵfac/setClassMetadata) and registering the class in +// jitDeclarationRegistry so the JIT transform downlevels its decorators. + +#[test] +fn test_jit_true_component_skips_aot() { + let allocator = Allocator::default(); + let source = r" +import { Component } from '@angular/core'; + +@Component({ jit: true, selector: 'c', template: '

{{x}}

' }) +export class C { x = 1 } +"; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + for forbidden in ["ɵcmp", "ɵfac", "ɵsetClassMetadata"] { + assert!( + !result.code.contains(forbidden), + "jit:true class must not emit {forbidden}. Got:\n{}", + result.code + ); + } + assert!( + result.code.contains("import { __decorate } from \"tslib\""), + "Should import __decorate. Got:\n{}", + result.code + ); + assert!( + result.code.contains("let C = class C {"), + "Class should be restructured as a class expression. Got:\n{}", + result.code + ); + assert!( + result.code + .contains("C = __decorate([\n Component({ jit: true, selector: 'c', template: '

{{x}}

' })\n], C);"), + "Should downlevel the decorator via __decorate. Got:\n{}", + result.code + ); + assert!(result.code.contains("export { C };"), "Should re-export. Got:\n{}", result.code); + // No Ivy fields in the .d.ts either. + assert!(result.dts_declarations.is_empty(), "jit:true must not emit d.ts Ivy fields"); +} + +#[test] +fn test_jit_true_directive_skips_aot() { + let allocator = Allocator::default(); + let source = r" +import { Directive } from '@angular/core'; + +@Directive({ jit: true, selector: '[d]' }) +export class D {} +"; + let result = transform_angular_file(&allocator, "d.directive.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + for forbidden in ["ɵdir", "ɵfac", "ɵsetClassMetadata"] { + assert!( + !result.code.contains(forbidden), + "jit:true directive must not emit {forbidden}. Got:\n{}", + result.code + ); + } + assert!( + result + .code + .contains("D = __decorate([\n Directive({ jit: true, selector: '[d]' })\n], D);"), + "Should downlevel via __decorate. Got:\n{}", + result.code + ); + assert!(result.dts_declarations.is_empty()); +} + +#[test] +fn test_jit_true_ng_module_skips_aot() { + let allocator = Allocator::default(); + let source = r" +import { NgModule } from '@angular/core'; + +@NgModule({ jit: true, declarations: [] }) +export class M {} +"; + let result = transform_angular_file(&allocator, "m.module.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + for forbidden in ["ɵmod", "ɵinj", "ɵfac", "ɵsetClassMetadata"] { + assert!( + !result.code.contains(forbidden), + "jit:true NgModule must not emit {forbidden}. Got:\n{}", + result.code + ); + } + assert!( + result + .code + .contains("M = __decorate([\n NgModule({ jit: true, declarations: [] })\n], M);"), + "Should downlevel via __decorate. Got:\n{}", + result.code + ); + assert!(result.dts_declarations.is_empty()); +} + +#[test] +fn test_jit_true_mixed_file() { + // A jit:true class and a normal component in the same file: only the + // jit:true class is skipped; the other still compiles AOT. + let allocator = Allocator::default(); + let source = r" +import { Component } from '@angular/core'; + +@Component({ jit: true, selector: 'jit-c', template: 'jit' }) +export class JitComp {} + +@Component({ selector: 'aot-c', template: 'aot' }) +export class AotComp {} +"; + let result = transform_angular_file(&allocator, "mix.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + assert_eq!(result.component_count, 1, "Only the AOT component counts"); + assert!( + result.code.contains("JitComp = __decorate("), + "JitComp should be downleveled. Got:\n{}", + result.code + ); + assert!( + result.code.contains("AotComp.ɵcmp") || result.code.contains("static ɵcmp"), + "AotComp should still compile. Got:\n{}", + result.code + ); + assert!( + !result.code.contains("JitComp.ɵfac"), + "JitComp must not get ɵfac. Got:\n{}", + result.code + ); +} + +#[test] +fn test_jit_true_member_and_ctor_metadata() { + // Member decorators become propDecorators, constructor parameters become + // ctorParameters, and their imports stay live (upstream: TypeScript's + // import elision sees the rewritten AST). + let allocator = Allocator::default(); + let source = r" +import { Component, Input, Inject, Optional } from '@angular/core'; +import { SomeService } from './svc'; + +@Component({ jit: true, selector: 'c', template: 'hi' }) +export class C { + @Input() x = 1; + constructor(private s: SomeService, @Optional() @Inject('TOK') private t?: string) {} +} +"; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + assert!( + result.code.contains("import { SomeService } from './svc';"), + "ctor type import must survive elision. Got:\n{}", + result.code + ); + assert!( + result.code.contains("import { Component, Input, Inject, Optional }"), + "Angular imports must survive. Got:\n{}", + result.code + ); + assert!(result.code.contains("static ctorParameters"), "Got:\n{}", result.code); + assert!(result.code.contains("{ type: SomeService }"), "Got:\n{}", result.code); + assert!( + result.code.contains("{ type: Optional }") && result.code.contains("type: Inject"), + "Param decorators belong in ctorParameters. Got:\n{}", + result.code + ); + assert!( + result.code.contains("static propDecorators") + && result.code.contains("x: [{ type: Input }]"), + "@Input belongs in propDecorators. Got:\n{}", + result.code + ); + assert!( + !result.code.contains("@Input"), + "Member decorator must be removed. Got:\n{}", + result.code + ); +} + +#[test] +fn test_jit_true_with_injectable() { + // Upstream's InjectableHandler runs independently of the jitForced + // short-circuit: a @Component({jit:true}) + @Injectable class keeps its + // ɵfac/ɵprov and setClassMetadata while its decorators are downleveled. + let allocator = Allocator::default(); + let source = r" +import { Component, Injectable } from '@angular/core'; + +@Component({ jit: true, selector: 'c', template: 'hi' }) +@Injectable() +export class C {} +"; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + assert!(result.code.contains("static ɵfac"), "ɵfac expected. Got:\n{}", result.code); + assert!(result.code.contains("static ɵprov"), "ɵprov expected. Got:\n{}", result.code); + assert!(!result.code.contains("ɵcmp"), "No ɵcmp. Got:\n{}", result.code); + assert!( + result.code.contains("Injectable()") && result.code.contains("C = __decorate("), + "Both decorators should be downleveled. Got:\n{}", + result.code + ); + assert!( + result.code.contains("import * as i0 from '@angular/core'"), + "ɵfac/ɵprov need i0. Got:\n{}", + result.code + ); +} + +#[test] +fn test_jit_false_still_compiles_aot() { + let allocator = Allocator::default(); + let source = r" +import { Component } from '@angular/core'; + +@Component({ jit: false, selector: 'c', template: 'hi' }) +export class C {} +"; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + assert_eq!(result.component_count, 1); + assert!(result.code.contains("static ɵcmp"), "Got:\n{}", result.code); +} + +#[test] +fn test_jit_true_signal_apis() { + // Signal initializer APIs synthesize propDecorators referencing the + // @angular/core namespace — the i0 import must be added for a jit:true + // class even though nothing else in the file needs it. + let allocator = Allocator::default(); + let source = r" +import { Component, input } from '@angular/core'; + +@Component({ jit: true, selector: 'c', template: 'hi' }) +export class C { + x = input.required(); +} +"; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + assert!( + result.code.contains("import * as i0 from '@angular/core'"), + "Synthesized i0.Input needs the namespace import. Got:\n{}", + result.code + ); + assert!(result.code.contains("type: i0.Input"), "Got:\n{}", result.code); +} + +#[test] +fn test_jit_true_no_stray_i0_import() { + // A jit:true class without signal APIs / @Injectable references no i0 — + // no @angular/core namespace import should be added. + let allocator = Allocator::default(); + let source = r" +import { Directive } from '@angular/core'; + +@Directive({ jit: true, selector: '[d]' }) +export class D {} +"; + let result = transform_angular_file(&allocator, "d.ts", source, None, None); + assert!( + !result.code.contains("import * as i0"), + "Unused i0 import must not be emitted. Got:\n{}", + result.code + ); +} From f811c9ef7cddd6cd5a448536ba4d09c584439bdd Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 22:01:10 +0800 Subject: [PATCH 11/14] fix(compiler): adapt jit-forced setClassMetadata call site to internal decorator scan --- .../src/component/transform.rs | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 566334d9e..c6df1377e 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -2853,21 +2853,20 @@ pub fn transform_angular_file( &injectable_metadata, type_argument_count, )); - if let Some(injectable_decorator) = - find_injectable_decorator(&class.decorators, Some(&string_consts)) - { - after_class_extra = build_set_class_metadata_decls( - &allocator, - class, - &info.class_name, - injectable_decorator, - options, - source, - &string_consts, - &import_map, - &mut file_namespace_registry, - ); - } + // `build_set_class_metadata_decls` lists every + // @angular/core class decorator itself, so the + // metadata covers the injectable alongside the + // jit-forced decorator. + after_class_extra = build_set_class_metadata_decls( + &allocator, + class, + &info.class_name, + options, + source, + &string_consts, + &import_map, + &mut file_namespace_registry, + ); } } From 09753143a238a36c980308cecfc46e34d521fbe5 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 22:10:28 +0800 Subject: [PATCH 12/14] fix(compiler): check static outputs without initializer; pass string consts in napi metadata - output_members used value? which skipped the static-member check for members without an initializer (bare @Output() static fields and all decorated methods). Gate the .required() check on Some(value) instead. - napi compileClassMetadata missed the new StringConsts arg on build_decorator_metadata_array; pass the already-collected consts. --- .../src/directive/decorator.rs | 16 ++++++++++------ .../tests/integration_test.rs | 4 ++++ napi/angular-compiler/src/lib.rs | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 176b15153..a015e1943 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -1179,12 +1179,16 @@ pub fn decorator_io_errors<'a>( return Some(error); } // ngtsc's `tryParseInitializerBasedOutput` rejects `output.required()` - // while parsing the member, before the checks below. - if let Some((_, true, call)) = initializer_api_call( - value?, - Some(consts), - &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API], - ) { + // while parsing the member, before the checks below. Members + // without an initializer (incl. every method) fall through to the + // static-member and @Output-on-signal checks. + if let Some(value) = value + && let Some((_, true, call)) = initializer_api_call( + value, + Some(consts), + &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API], + ) + { return Some(("Output does not support \".required()\".".to_string(), call.span)); } // Then `@Output` on an `output()` or a model, like ngtsc's diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index aa2f1ab25..2a969e0a9 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -16155,8 +16155,12 @@ fn test_static_input_on_component_and_pipe_is_diagnostic() { fn test_static_output_member_is_diagnostic() { // `model()` is covered by the input test: ngtsc's `parseInputFields` runs // first, so a static model reports the Input error, not the Output one. + // Members without an initializer — a bare declaration and a method — hit + // the same check (tryParseDecoratorOutput accepts any member kind). for member in [ "@Output() static y = new EventEmitter();", + "@Output() static y: EventEmitter;", + "@Output() static emit() {}", "static y = output();", "static y = outputFromObservable(of(0));", ] { diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index 704be4b01..fa9404b1d 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -2084,6 +2084,7 @@ pub fn compile_class_metadata_sync( None, None, None, + Some(&string_consts), ); // Build constructor parameters metadata From 90f2b0393fdaea7532150c2df56c91876b56b2d1 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 22:17:46 +0800 Subject: [PATCH 13/14] =?UTF-8?q?fix(compiler):=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20pipe=20diagnostics,=20query=20member=20kind,=20jit?= =?UTF-8?q?=20presence,=20ctorParameters=20null?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PipeDecoratorHandler never runs extractDirectiveMetadata upstream: drop @Pipe from angular_decorator_config so io/query checks and metadata fields don't apply to pipes. - Query decorator on a non-property member (method/ctor) reports 'Query decorator must go on a property-type member' before the static check; get/set accessors stay property-type. - accessor fields reflect as property members upstream: treat them in the view/content query extractors too. - jit forced on property presence (directive.has('jit')), not value. - ctorParameters emits bare null for params with no type/decorators. --- .../src/component/transform.rs | 15 +- .../src/directive/decorator.rs | 12 +- .../src/directive/property_decorators.rs | 286 ++++++++++-------- .../tests/integration_test.rs | 107 +++++-- ...core_param_decorators_ctor_parameters.snap | 2 +- ...tion_test__jit_union_type_ctor_params.snap | 5 +- 6 files changed, 259 insertions(+), 168 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index c6df1377e..bf7dddd31 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -1077,12 +1077,11 @@ fn find_angular_decorator<'a>( } /// Whether `decorator`'s options object opts out of AOT compilation via -/// `jit: true`. +/// `jit`. /// /// Mirrors ngtsc's `extractDirectiveMetadata` / `NgModuleDecoratorHandler.analyze` -/// (`shared.ts:176`, `ng_module/handler.ts:352`): presence of the `jit` -/// property forces JIT — the interface only allows `true`, so any value that -/// isn't statically `false` is treated as opt-out. +/// (`shared.ts:176`, `ng_module/handler.ts:352`): mere presence of the `jit` +/// property forces JIT — `directive.has('jit')` — regardless of the value. fn decorator_forces_jit(decorator: &oxc_ast::ast::Decorator<'_>) -> bool { let Expression::CallExpression(call) = &decorator.expression else { return false }; let Some(Argument::ObjectExpression(config)) = call.arguments.first() else { @@ -1099,7 +1098,7 @@ fn decorator_forces_jit(decorator: &oxc_ast::ast::Decorator<'_>) -> bool { _ => false, }; if is_jit { - return !matches!(&prop.value, Expression::BooleanLiteral(b) if !b.value); + return true; } } false @@ -1754,6 +1753,12 @@ fn build_ctor_parameters_text(params: &[JitCtorParam]) -> Option { let mut entries = std::vec::Vec::new(); for param in params { + // ngtsc emits bare `null` for a param with neither type nor + // decorators (downlevel_decorators_transform.ts). + if param.type_name.is_none() && param.decorators.is_empty() { + entries.push("null".to_string()); + continue; + } let mut parts = std::vec::Vec::new(); // type diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index a015e1943..e184ee8ee 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -1041,10 +1041,11 @@ fn upsert_input<'a>(inputs: &mut Vec<'a, R3InputMetadata<'a>>, input: R3InputMet upsert_meta(inputs, input, |i| i.class_property_name.as_str()); } -/// Angular's `@Component` / `@Directive` / `@Pipe` decorator on `class` -/// (imported from `@angular/core`, in the file `consts` was collected from), -/// its metadata object (if any) and its name. `@Pipe` is here because ngtsc -/// runs `extractDirectiveMetadata` — and its io/query checks — for pipes too. +/// Angular's `@Component` / `@Directive` decorator on `class` (imported from +/// `@angular/core`, in the file `consts` was collected from), its metadata +/// object (if any) and its name. `@Pipe` is excluded: upstream's +/// `PipeDecoratorHandler` never runs `extractDirectiveMetadata`, so io and +/// query checks don't apply to pipes. pub(crate) fn angular_decorator_config<'a>( class: &'a Class<'a>, consts: &StringConsts<'_>, @@ -1053,9 +1054,6 @@ pub(crate) fn angular_decorator_config<'a>( .map(|d| (d, "Component")) .or_else(|| { find_directive_decorator(&class.decorators, Some(consts)).map(|d| (d, "Directive")) - }) - .or_else(|| { - crate::pipe::find_pipe_decorator(&class.decorators, Some(consts)).map(|d| (d, "Pipe")) })?; let config = match &decorator.expression { Expression::CallExpression(call) => match call.arguments.first() { diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 01b35e6bd..70cfaf864 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -1202,77 +1202,78 @@ pub(crate) fn extract_view_queries_in<'a>( let mut view_children_queries = Vec::new_in(&allocator); for element in &class.body.body { - match element { + // Property-type members: fields and auto-accessors both reflect as + // `Property` upstream (`reflectClassMember`), so an `accessor` member + // is a query member too. + let prop_like = match element { ClassElement::PropertyDefinition(prop) => { - // Check for signal-based view queries first (viewChild(), viewChildren()) - if let Some(value) = &prop.value { - if let Some(property_name) = get_property_key_name(&prop.key) { - if let Some((query_type, metadata)) = try_parse_signal_query( - allocator, - value, - property_name, - source_text, - consts, - ) { - if query_type.is_view_query() { - signal_queries.push(metadata); - continue; - } + Some((&prop.decorators, &prop.key, prop.value.as_ref())) + } + ClassElement::AccessorProperty(prop) => { + Some((&prop.decorators, &prop.key, prop.value.as_ref())) + } + _ => None, + }; + if let Some((decorators, key, value)) = prop_like { + // Check for signal-based view queries first (viewChild(), viewChildren()) + if let Some(value) = value { + if let Some(property_name) = get_property_key_name(key) { + if let Some((query_type, metadata)) = + try_parse_signal_query(allocator, value, property_name, source_text, consts) + { + if query_type.is_view_query() { + signal_queries.push(metadata); + continue; } } } + } - // Check for decorator-based queries (@ViewChild, @ViewChildren) - if let Some(decorator) = - find_decorator_by_name(&prop.decorators, "ViewChild", consts) - { - if let Some(property_name) = get_property_key_name(&prop.key) { - let config = parse_query_config( - allocator, - decorator, - "ViewChild", - source_text, - consts, - ); - if let Some(predicate) = config.predicate { - view_child_queries.push(R3QueryMetadata { - property_name, - first: true, - predicate, - descendants: config.descendants, - emit_distinct_changes_only: config.emit_distinct_changes_only, - read: config.read, - is_static: config.is_static, - is_signal: false, - }); - } + // Check for decorator-based queries (@ViewChild, @ViewChildren) + if let Some(decorator) = find_decorator_by_name(decorators, "ViewChild", consts) { + if let Some(property_name) = get_property_key_name(key) { + let config = + parse_query_config(allocator, decorator, "ViewChild", source_text, consts); + if let Some(predicate) = config.predicate { + view_child_queries.push(R3QueryMetadata { + property_name, + first: true, + predicate, + descendants: config.descendants, + emit_distinct_changes_only: config.emit_distinct_changes_only, + read: config.read, + is_static: config.is_static, + is_signal: false, + }); } - } else if let Some(decorator) = - find_decorator_by_name(&prop.decorators, "ViewChildren", consts) - { - if let Some(property_name) = get_property_key_name(&prop.key) { - let config = parse_query_config( - allocator, - decorator, - "ViewChildren", - source_text, - consts, - ); - if let Some(predicate) = config.predicate { - view_children_queries.push(R3QueryMetadata { - property_name, - first: false, - predicate, - descendants: config.descendants, - emit_distinct_changes_only: config.emit_distinct_changes_only, - read: config.read, - is_static: config.is_static, - is_signal: false, - }); - } + } + } else if let Some(decorator) = + find_decorator_by_name(decorators, "ViewChildren", consts) + { + if let Some(property_name) = get_property_key_name(key) { + let config = parse_query_config( + allocator, + decorator, + "ViewChildren", + source_text, + consts, + ); + if let Some(predicate) = config.predicate { + view_children_queries.push(R3QueryMetadata { + property_name, + first: false, + predicate, + descendants: config.descendants, + emit_distinct_changes_only: config.emit_distinct_changes_only, + read: config.read, + is_static: config.is_static, + is_signal: false, + }); } } } + } + match element { ClassElement::MethodDefinition(method) if matches!(method.kind, MethodDefinitionKind::Set | MethodDefinitionKind::Get) => { @@ -1387,77 +1388,80 @@ pub(crate) fn extract_content_queries_in<'a>( let mut content_children_queries = Vec::new_in(&allocator); for element in &class.body.body { - match element { + let prop_like = match element { ClassElement::PropertyDefinition(prop) => { - // Check for signal-based content queries first (contentChild(), contentChildren()) - if let Some(value) = &prop.value { - if let Some(property_name) = get_property_key_name(&prop.key) { - if let Some((query_type, metadata)) = try_parse_signal_query( - allocator, - value, - property_name, - source_text, - consts, - ) { - if !query_type.is_view_query() { - signal_queries.push(metadata); - continue; - } + Some((&prop.decorators, &prop.key, prop.value.as_ref())) + } + ClassElement::AccessorProperty(prop) => { + Some((&prop.decorators, &prop.key, prop.value.as_ref())) + } + _ => None, + }; + if let Some((decorators, key, value)) = prop_like { + // Check for signal-based content queries first (contentChild(), contentChildren()) + if let Some(value) = value { + if let Some(property_name) = get_property_key_name(key) { + if let Some((query_type, metadata)) = + try_parse_signal_query(allocator, value, property_name, source_text, consts) + { + if !query_type.is_view_query() { + signal_queries.push(metadata); + continue; } } } + } - // Check for decorator-based queries (@ContentChild, @ContentChildren) - if let Some(decorator) = - find_decorator_by_name(&prop.decorators, "ContentChild", consts) - { - if let Some(property_name) = get_property_key_name(&prop.key) { - let config = parse_query_config( - allocator, - decorator, - "ContentChild", - source_text, - consts, - ); - if let Some(predicate) = config.predicate { - content_child_queries.push(R3QueryMetadata { - property_name, - first: true, - predicate, - descendants: config.descendants, - emit_distinct_changes_only: config.emit_distinct_changes_only, - read: config.read, - is_static: config.is_static, - is_signal: false, - }); - } + // Check for decorator-based queries (@ContentChild, @ContentChildren) + if let Some(decorator) = find_decorator_by_name(decorators, "ContentChild", consts) { + if let Some(property_name) = get_property_key_name(key) { + let config = parse_query_config( + allocator, + decorator, + "ContentChild", + source_text, + consts, + ); + if let Some(predicate) = config.predicate { + content_child_queries.push(R3QueryMetadata { + property_name, + first: true, + predicate, + descendants: config.descendants, + emit_distinct_changes_only: config.emit_distinct_changes_only, + read: config.read, + is_static: config.is_static, + is_signal: false, + }); } - } else if let Some(decorator) = - find_decorator_by_name(&prop.decorators, "ContentChildren", consts) - { - if let Some(property_name) = get_property_key_name(&prop.key) { - let config = parse_query_config( - &allocator, - decorator, - "ContentChildren", - source_text, - consts, - ); - if let Some(predicate) = config.predicate { - content_children_queries.push(R3QueryMetadata { - property_name, - first: false, - predicate, - descendants: config.descendants, - emit_distinct_changes_only: config.emit_distinct_changes_only, - read: config.read, - is_static: config.is_static, - is_signal: false, - }); - } + } + } else if let Some(decorator) = + find_decorator_by_name(decorators, "ContentChildren", consts) + { + if let Some(property_name) = get_property_key_name(key) { + let config = parse_query_config( + &allocator, + decorator, + "ContentChildren", + source_text, + consts, + ); + if let Some(predicate) = config.predicate { + content_children_queries.push(R3QueryMetadata { + property_name, + first: false, + predicate, + descendants: config.descendants, + emit_distinct_changes_only: config.emit_distinct_changes_only, + read: config.read, + is_static: config.is_static, + is_signal: false, + }); } } } + } + match element { ClassElement::MethodDefinition(method) if matches!(method.kind, MethodDefinitionKind::Set | MethodDefinitionKind::Get) => { @@ -1966,22 +1970,38 @@ pub(crate) fn member_query_error<'a>( consts: &super::StringConsts<'a>, ) -> Option<(String, Span)> { class.body.body.iter().find_map(|element| { - let (decorators, value, is_static, span) = match element { + // Upstream's `isPropertyTypeMember`: only getters, setters and + // properties (incl. auto-accessors) may hold a query decorator. + let (decorators, value, is_static, span, is_property_type) = match element { ClassElement::PropertyDefinition(prop) => { - (&prop.decorators, prop.value.as_ref(), prop.r#static, prop.span) - } - ClassElement::AccessorProperty(accessor) => { - (&accessor.decorators, accessor.value.as_ref(), accessor.r#static, accessor.span) - } - ClassElement::MethodDefinition(method) => { - (&method.decorators, None, method.r#static, method.span) + (&prop.decorators, prop.value.as_ref(), prop.r#static, prop.span, true) } + ClassElement::AccessorProperty(accessor) => ( + &accessor.decorators, + accessor.value.as_ref(), + accessor.r#static, + accessor.span, + true, + ), + ClassElement::MethodDefinition(method) => ( + &method.decorators, + None, + method.r#static, + method.span, + matches!(method.kind, MethodDefinitionKind::Get | MethodDefinitionKind::Set), + ), _ => return None, }; let decorator = QUERY_TYPES.iter().find_map(|name| { find_decorator_by_name(decorators, name, Some(consts)).zip(Some(*name)) }); - // A query decorator's own errors come first (`tryGetQueryFromFieldDecorator`). + // Its member-kind check (DECORATOR_UNEXPECTED) runs before the + // decorator metadata is parsed upstream (`isPropertyTypeMember` + // precedes `extractDecoratorQueryMetadata`). + if decorator.is_some() && !is_property_type { + return Some(("Query decorator must go on a property-type member".to_string(), span)); + } + // A query decorator's own errors come next (`tryGetQueryFromFieldDecorator`). if let Some((decorator, name)) = decorator && let Some(error) = member_query(allocator, decorator, name, span, source_text, consts).err() diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 2a969e0a9..b950fc97c 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -9352,11 +9352,12 @@ export class TestComponent { result.code ); - // The foreign @Inject is not Angular's: no ctorParameters decorators for - // param 3, and it's lowered as __param. + // The foreign @Inject is not Angular's: upstream emits `null` for a param + // with no type and no (Angular) decorators, and it's lowered as __param. assert!( - compact.contains("{type:undefined}]"), - "foreign-decorated param should have no decorators entry. Got:\n{}", + compact.contains("type:Injectable}]},\nnull]") + || compact.contains("type:Injectable}]},null]"), + "foreign-decorated param should be emitted as null. Got:\n{}", result.code ); assert!( @@ -16035,7 +16036,9 @@ export class C {} } #[test] -fn test_jit_false_still_compiles_aot() { +fn test_jit_false_still_forces_jit() { + // Upstream checks `directive.has('jit')` — presence, not value — so + // `jit: false` (type-invalid input) still opts out of AOT. let allocator = Allocator::default(); let source = r" import { Component } from '@angular/core'; @@ -16045,8 +16048,9 @@ export class C {} "; let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); - assert_eq!(result.component_count, 1); - assert!(result.code.contains("static ɵcmp"), "Got:\n{}", result.code); + assert_eq!(result.component_count, 0); + assert!(!result.code.contains("static ɵcmp"), "Got:\n{}", result.code); + assert!(result.code.contains("__decorate"), "Got:\n{}", result.code); } #[test] @@ -16134,19 +16138,32 @@ fn test_static_input_member_is_diagnostic() { } #[test] -fn test_static_input_on_component_and_pipe_is_diagnostic() { - // extractDirectiveMetadata runs for @Component and @Pipe too. - for decorator in ["@Component({ selector: 'c', template: '' })", "@Pipe({ name: 'p' })"] { - let source = format!( - "import {{ Component, Directive, Input, Pipe }} from '@angular/core';\n\ - {decorator}\n\ - export class C {{\n @Input() static x = 0;\n}}" - ); - let diagnostics = expect_diagnostics(&source); - let expected = "Input \"x\" is incorrectly declared as static member of \"C\"."; +fn test_static_input_on_component_is_diagnostic_and_pipe_is_silent() { + // extractDirectiveMetadata runs for @Component, but upstream's + // PipeDecoratorHandler never calls it: io/query checks don't apply to + // pipes, so a static input or a `queries:` field on a pipe is ignored. + let source = "import { Component, Input } from '@angular/core';\n\ + @Component({ selector: 'c', template: '' })\n\ + export class C {\n @Input() static x = 0;\n}"; + let diagnostics = expect_diagnostics(source); + let expected = "Input \"x\" is incorrectly declared as static member of \"C\"."; + assert!( + diagnostics.iter().any(|d| d.contains(expected)), + "@Component should report {expected:?}. Got: {diagnostics:?}" + ); + + for source in [ + "import { Input, Pipe } from '@angular/core';\n\ + @Pipe({ name: 'p' })\n\ + export class P {\n @Input() static x = 0;\n}", + "import { Pipe, ViewChild } from '@angular/core';\n\ + @Pipe({ name: 'p', queries: { q: new ViewChild(42) } })\n\ + export class P {}", + ] { + let diagnostics = expect_diagnostics(source); assert!( - diagnostics.iter().any(|d| d.contains(expected)), - "{decorator} should report {expected:?}. Got: {diagnostics:?}" + diagnostics.is_empty(), + "@Pipe members and metadata should not get io/query diagnostics. Got: {diagnostics:?}" ); } } @@ -16179,6 +16196,58 @@ fn test_static_output_member_is_diagnostic() { } } +#[test] +fn test_query_decorator_on_method_reports_property_type_member() { + // `isPropertyTypeMember` (shared.ts) runs before the static check, so a + // query decorator on a method — static or not — is DECORATOR_UNEXPECTED, + // while a query on an accessor counts as a property member upstream. + for member in [ + "@ViewChild('a') m() {}", + "@ViewChild('a') static m() {}", + "@ViewChild('a') constructor() {}", + ] { + let source = format!( + "import {{ Directive, ViewChild }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics + .iter() + .any(|d| d.contains("Query decorator must go on a property-type member")), + "`{member}` should report the property-type error. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_query_on_accessor_member_compiles() { + // A TS `accessor` field reflects upstream as a property member + // (`isPropertyTypeMember`): decorator and signal queries on it compile. + let allocator = Allocator::default(); + for (member, instr) in [ + ("@ViewChild('el') accessor z: any;", "ɵɵviewQuery"), + ("accessor z = viewChild('el');", "ɵɵviewQuerySignal"), + ("@ContentChild('el') accessor c: any;", "ɵɵcontentQuery"), + ("accessor c = contentChild('el');", "ɵɵcontentQuerySignal"), + ] { + let source = format!( + "import {{ Component, ViewChild, ContentChild, viewChild, contentChild }} \ + from '@angular/core';\n\ + @Component({{ selector: 'c', template: '' }})\n\ + export class C {{\n {member}\n}}" + ); + let result = transform_angular_file(&allocator, "test.ts", &source, None, None); + assert!(!result.has_errors(), "`{member}` should not error: {:?}", result.diagnostics); + assert!( + result.code.contains(instr), + "`{member}` should emit `{instr}(...)`. Got:\n{}", + result.code + ); + } +} + #[test] fn test_static_query_member_is_diagnostic() { for member in [ diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap index 0b8cdd651..2245a0e61 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_angular_core_param_decorators_ctor_parameters.snap @@ -24,7 +24,7 @@ let TestComponent = class TestComponent { type: undefined, decorators: [{ type: Injectable }] }, - { type: undefined } + null ]; }; TestComponent = __decorate([Component({ diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_union_type_ctor_params.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_union_type_ctor_params.snap index 7a34b853c..da5ace62c 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_union_type_ctor_params.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__jit_union_type_ctor_params.snap @@ -1,6 +1,5 @@ --- source: crates/oxc_angular_compiler/tests/integration_test.rs -assertion_line: 6406 expression: result.code --- import { Component } from "@angular/core"; @@ -11,8 +10,8 @@ import { __decorate } from "tslib"; let TestComponent = class TestComponent { constructor(svcA, svcB, svcC) {} static ctorParameters = () => [ - { type: undefined }, - { type: undefined }, + null, + null, { type: ServiceC } ]; }; From 97127c4c7c27de0230c6e70d2b15943b2e66b075 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 6 Oct 2026 22:45:40 +0800 Subject: [PATCH 14/14] test(compare): document kept setClassMetadata imports as known diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc emits type: Inject in setClassMetadata while TypeScript elision still drops the Inject import — our fix intentionally keeps it so the emitted code resolves. Extend KnownDifference with importDiffs entries and forgive documented import diffs in the fixture runner. --- .../e2e/compare/fixtures/known-differences.ts | 42 +++++++++++++++++++ .../e2e/compare/fixtures/runner.ts | 25 ++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts index fe9cfd192..867e65194 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts @@ -6,9 +6,13 @@ * `fields`, so a new difference elsewhere in the same fixture still fails. A listed fixture * that matches Angular fails too, so fixed entries get removed. */ +import type { ImportDiff } from '../src/compare.js' + interface KnownDifference { /** `Class.field` of every static field that differs, e.g. `MyComponent.ɵcmp`. */ fields: string[] + /** Every import difference, exactly as `compareImports` reports it. */ + importDiffs?: ImportDiff[] /** Why each difference exists. */ reasons: string[] } @@ -27,6 +31,8 @@ const SELECTOR_WHITESPACE = 'runs of whitespace inside shimmed selectors are collapsed where Angular keeps them' const INJECTABLE_FACTORY_WRAPPER = 'an @Injectable useFactory is wrapped in a function expression where Angular emits an arrow function' +const SET_CLASS_METADATA_IMPORT = + 'Oxc keeps the @Inject import because its setClassMetadata references it; ngtsc emits the same reference but TypeScript elision still drops the import (upstream emit bug)' export const KNOWN_DIFFERENCES: Record = { 'animations/animation-metadata-with-change-detection': { @@ -41,6 +47,42 @@ export const KNOWN_DIFFERENCES: Record = { fields: ['TestablePipe.ɵfac'], reasons: [FACTORY], }, + 'class-metadata/class-metadata-injectable': { + fields: [], + importDiffs: [ + { + type: 'different', + moduleSource: '@angular/core', + expected: ['Injectable', 'InjectionToken'], + actual: ['Inject', 'Injectable', 'InjectionToken'], + }, + ], + reasons: [SET_CLASS_METADATA_IMPORT], + }, + 'class-metadata/class-metadata-with-inject': { + fields: [], + importDiffs: [ + { + type: 'different', + moduleSource: '@angular/core', + expected: ['Component', 'InjectionToken'], + actual: ['Component', 'Inject', 'InjectionToken'], + }, + ], + reasons: [SET_CLASS_METADATA_IMPORT], + }, + 'full-file/component-with-services': { + fields: [], + importDiffs: [ + { + type: 'different', + moduleSource: '@angular/core', + expected: ['Component', 'InjectionToken', 'inject'], + actual: ['Component', 'Inject', 'InjectionToken', 'inject'], + }, + ], + reasons: [SET_CLASS_METADATA_IMPORT], + }, 'component-meta/change-detection-default': { fields: ['ChangeDetectionDefaultComponent.ɵcmp'], reasons: ['changeDetection is omitted where Angular emits it'], diff --git a/napi/angular-compiler/e2e/compare/fixtures/runner.ts b/napi/angular-compiler/e2e/compare/fixtures/runner.ts index f45dde006..6c0c3de86 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/runner.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/runner.ts @@ -140,8 +140,29 @@ async function testFixture(fixture: Fixture, verbose?: boolean): Promise `${diff.className}.${diff.fieldName}`) .filter((field) => !known.fields.includes(field)) - if (undocumented.length > 0) { - return { ...result, knownDifferences, undocumentedFields: [...new Set(undocumented)] } + const undocumentedImports = (result.importDiffs ?? []) + .filter( + (diff) => + !(known.importDiffs ?? []).some( + (k) => + k.type === diff.type && + k.moduleSource === diff.moduleSource && + JSON.stringify(k.expected ?? []) === JSON.stringify(diff.expected ?? []) && + JSON.stringify(k.actual ?? []) === JSON.stringify(diff.actual ?? []), + ), + ) + .map( + (diff) => + `import ${diff.moduleSource} expected { ${(diff.expected ?? []).join(', ')} } ` + + `got { ${(diff.actual ?? []).join(', ')} }`, + ) + const allUndocumented = [...undocumented, ...undocumentedImports] + if (allUndocumented.length > 0) { + return { + ...result, + knownDifferences, + undocumentedFields: [...new Set(allUndocumented)], + } } return { ...result, status: 'known-difference', knownDifferences } }