diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 9fd329465..fa75acae7 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -589,7 +589,16 @@ pub fn build_prop_decorators_metadata_in<'a>( }) .collect(); - if !angular_decorators.is_empty() { + // ngtsc lists any member that carries decorators — even when none + // of them are Angular's — as `prop: []` (`decoratedClassMemberTo- + // Metadata` filters to Angular decorators, but the member entry is + // unconditional on `member.decorators.length > 0`, metadata.ts:107). + // `member.decorators` is already filtered by the reflection host: + // `_reflectDecorator` drops a decorator whose callee isn't an + // identifier or `ns.Name` access (`isDecoratorIdentifier`), so an + // `@a.b.Foo()` or `@(a.Foo)()` member counts as undecorated and + // reaches the initializer-API synthesis below. + if decorators.iter().any(is_reflectable_decorator) { // Build decorators array from the real decorators present in source. let decorators_array = build_decorator_metadata_array( &allocator, @@ -1028,6 +1037,24 @@ fn extract_angular_decorators_from_param<'a, 'b>( .collect() } +/// Whether ngtsc's reflection host keeps `decorator` in `member.decorators`: +/// `_reflectDecorator` returns `null` unless the (possibly called) decorator +/// expression is an identifier or `ns.Name` property access +/// (`isDecoratorIdentifier`). `@decs['Foo']()` or `@a.b.Foo()` don't reflect. +fn is_reflectable_decorator(decorator: &Decorator<'_>) -> bool { + let expr = match &decorator.expression { + Expression::CallExpression(call) => &call.callee, + expr => expr, + }; + match expr { + Expression::Identifier(_) => true, + Expression::StaticMemberExpression(m) => { + matches!(&m.object, Expression::Identifier(_)) + } + _ => false, + } +} + /// Get the name of a decorator. fn get_decorator_name<'a>(decorator: &Decorator<'a>) -> Option<&'a str> { match &decorator.expression { diff --git a/crates/oxc_angular_compiler/src/output/oxc_converter.rs b/crates/oxc_angular_compiler/src/output/oxc_converter.rs index 6849c6785..fa72e7dd6 100644 --- a/crates/oxc_angular_compiler/src/output/oxc_converter.rs +++ b/crates/oxc_angular_compiler/src/output/oxc_converter.rs @@ -399,9 +399,12 @@ fn convert_call_expression_with_optional<'a>( for arg in &call.arguments { match arg { Argument::SpreadElement(spread) => { - // Handle spread arguments + // Keep the spread — `f(...P)` is not `f(P)` (issue #511). let expr = convert_oxc_expression(allocator, &spread.argument, source_text)?; - args.push(expr); + args.push(OutputExpression::SpreadElement(Box::new_in( + SpreadElementExpr { expr: Box::new_in(expr, &allocator), source_span: None }, + &allocator, + ))); } _ => { let expr = arg.to_expression(); @@ -437,8 +440,12 @@ fn convert_new_expression<'a>( for arg in &new_expr.arguments { match arg { Argument::SpreadElement(spread) => { + // Keep the spread — `new X(...P)` is not `new X(P)` (#511). let expr = convert_oxc_expression(allocator, &spread.argument, source_text)?; - args.push(expr); + args.push(OutputExpression::SpreadElement(Box::new_in( + SpreadElementExpr { expr: Box::new_in(expr, &allocator), source_span: None }, + &allocator, + ))); } _ => { let expr = arg.to_expression(); 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 336c212f0..bebc43527 100644 --- a/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs +++ b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs @@ -1327,7 +1327,9 @@ export class Cmp {{ /// `@angular/core`, under any alias or through a namespace import. Only those /// are compiled (the snapshot's `memberDecorator-*` probes), removed from the /// class and listed in `setClassMetadata`. Another module's `@Input` is left on -/// the class and its options aren't checked, as ngtsc 22.1.7 does. +/// the class and its options aren't checked, as ngtsc 22.1.7 does. Such +/// foreign-decorated members still get a `propDecorators` entry — with an +/// empty array, since none of their decorators are Angular's (issue #511). #[test] fn only_angular_member_decorators_are_compiled_and_removed() { let source = "import {Directive, Input as In} from '@angular/core'; @@ -1348,7 +1350,7 @@ export class Dir { assert!(code.contains("@Input({transform:5})c:any;"), "{}", result.code); assert!(code.contains("@HostListener('click')d(){}"), "{}", result.code); assert!(code.contains(r#"inputs:{a:"a"},outputs:{b:"b"}}"#), "{}", result.code); - assert!(code.contains("{a:[{type:In}],b:[{type:core.Output}]}"), "{}", result.code); + assert!(code.contains("{a:[{type:In}],b:[{type:core.Output}],c:[],d:[]}"), "{}", result.code); } /// A query predicate that isn't statically evaluable is emitted as written diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 4ebb87438..a7e8b3532 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -9430,7 +9430,10 @@ export class TestComponent { 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. + // @Component(). The member entry is unconditional on having decorators + // (metadata.ts:107 `member.decorators.length > 0`), so the foreign + // decorator yields `foreignMember: []` — filtered to no Angular + // decorators — same as the ctorParameters `decorators: []` below (#511). assert!( compact.contains("componentMember:[{type:Component}]"), "@Component() member should be in propDecorators. Got:\n{}", @@ -9442,8 +9445,8 @@ export class TestComponent { result.code ); assert!( - !compact.contains("foreignMember:["), - "foreign-decorated member should not be in propDecorators. Got:\n{}", + compact.contains("foreignMember:[]"), + "foreign-decorated member should get an empty propDecorators entry. Got:\n{}", result.code ); 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 index 8b3dccab4..0ba00e95e 100644 --- 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 @@ -2,7 +2,6 @@ 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'; @@ -42,5 +41,5 @@ static ɵcmp = /*@__PURE__*/ i0.ɵɵdefineComponent({type:TestComponent,selector [{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]}]})); + args:[TOKEN]}],foreignMember:[]})); })(); diff --git a/crates/oxc_angular_compiler/tests/spread_metadata_emit_test.rs b/crates/oxc_angular_compiler/tests/spread_metadata_emit_test.rs new file mode 100644 index 000000000..ac80d886e --- /dev/null +++ b/crates/oxc_angular_compiler/tests/spread_metadata_emit_test.rs @@ -0,0 +1,136 @@ +//! Spread arguments in decorator metadata must be preserved (issue #511): +//! `f(...P)` is not `f(P)`, and `new Box(...P)` is not `new Box(P)`. +//! Plus the propDecorators shape rule from the same issue: a member whose +//! decorators are all non-Angular still gets an entry (`x: []`). + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +fn compile(source: &str, mode: CompilationMode) -> String { + let allocator = Allocator::default(); + let options = TransformOptions { compilation_mode: mode, ..Default::default() }; + let result = transform_angular_file(&allocator, "test.ts", source, Some(&options), None); + assert!(!result.has_errors(), "should not have errors, got: {:?}", result.diagnostics); + // Whitespace-insensitive comparisons. + result.code.chars().filter(|c| !c.is_whitespace()).collect() +} + +const SPREAD_SOURCE: &str = "import { Component } from '@angular/core'; +const P: any[] = []; +function f(...a: any[]) { return a; } +class Box { constructor(...a: any[]) {} } +@Component({ selector: 'c', template: '', providers: f(...P) }) +export class C {} +@Component({ + selector: 'c2', + template: '', + providers: [{ provide: 'T', useValue: new Box(...P) }, ...P], +}) +export class C2 {} +"; + +#[test] +fn full_mode_call_spread_preserved() { + let code = compile(SPREAD_SOURCE, CompilationMode::Full); + assert!( + code.contains("ProvidersFeature(f(...P))"), + "providers call spread must survive, got:\n{code}" + ); + assert!( + code.contains(r#"providers:f(...P)"#), + "setClassMetadata call spread must survive, got:\n{code}" + ); +} + +#[test] +fn full_mode_new_spread_preserved() { + let code = compile(SPREAD_SOURCE, CompilationMode::Full); + assert!( + code.contains("useValue:newBox(...P)"), + "new-expression spread must survive, got:\n{code}" + ); + assert!(code.contains(",...P]"), "array spread in providers must survive, got:\n{code}"); +} + +#[test] +fn partial_mode_call_and_new_spread_preserved() { + let code = compile(SPREAD_SOURCE, CompilationMode::Partial); + assert!( + code.contains(r#"providers:f(...P)"#), + "ngDeclareComponent providers call spread must survive, got:\n{code}" + ); + assert!( + code.contains("useValue:newBox(...P)},...P]"), + "ngDeclareComponent new-expression + array spread must survive, got:\n{code}" + ); +} + +/// ngtsc lists a member in `propDecorators` whenever it carries decorators, +/// even when none of them are Angular's — `@Foo() x` emits `x: []` +/// (metadata.ts:107 + decoratedClassMemberToMetadata). +#[test] +fn member_with_only_non_angular_decorators_gets_empty_entry() { + let source = "import { Component, Input } from '@angular/core'; +function Foo(): PropertyDecorator { return () => {}; } +@Component({ selector: 'c', template: '' }) +export class C { + @Foo() x: any; + @Input() y: any; +} +"; + let code = compile(source, CompilationMode::Full); + assert!( + code.contains("x:[]") || code.contains("\"x\":[]"), + "non-Angular-decorated member must emit an empty propDecorators entry, got:\n{code}" + ); + assert!( + code.contains("y:[{type:Input}]") || code.contains("\"y\":[{type:Input}]"), + "Angular-decorated member must keep its entry, got:\n{code}" + ); +} + +/// A member with a non-Angular decorator AND a signal initializer still +/// emits `x: []` — upstream runs the decorated-member branch, not the +/// undecorated-metadata extractor. +#[test] +fn decorated_signal_member_emits_empty_entry() { + let source = "import { Component, Input, input } from '@angular/core'; +function Foo(): PropertyDecorator { return () => {}; } +@Component({ selector: 'c', template: '' }) +export class C { + @Foo() x = input(''); + @Input() y: any; +} +"; + let code = compile(source, CompilationMode::Full); + assert!( + code.contains("x:[]") || code.contains("\"x\":[]"), + "decorated signal member must emit `x: []`, got:\n{code}" + ); +} + +/// A decorator whose callee isn't an identifier or `ns.Name` access never +/// reaches `member.decorators` upstream — `_reflectDecorator` returns null +/// for `@a.b.Foo()` (the property-access object must be an identifier) or +/// `@decs['Foo']()` (`isDecoratorIdentifier`). The member counts as +/// undecorated, so its `input()` initializer gets the synthesized `Input` +/// entry, not `x: []`. +#[test] +fn unreflectable_decorator_leaves_signal_member_on_initializer_path() { + let source = "import { Component, input } from '@angular/core'; +const a: any = {}; +@Component({ selector: 'c', template: '' }) +export class C { + @a.b.Foo() x = input(''); +} +"; + let code = compile(source, CompilationMode::Full); + assert!( + code.contains("x:[{type:i0.Input"), + "unreflectable-decorated signal member must emit the synthesized Input entry, got:\n{code}" + ); + assert!( + !code.contains("x:[]"), + "unreflectable decorator must not produce an empty entry, got:\n{code}" + ); +}