diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 1656baabc..9fd329465 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: [...] } @@ -369,9 +404,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>( @@ -428,6 +467,7 @@ pub fn build_ctor_params_metadata_in<'a>( None, None, None, + None, ); map_entries.push(LiteralMapEntry::new( Ident::from("decorators"), @@ -534,12 +574,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(); @@ -553,6 +598,7 @@ pub fn build_prop_decorators_metadata_in<'a>( None, None, None, + None, ); prop_entries.push(LiteralMapEntry::new(prop_name, decorators_array, quoted)); continue; @@ -962,8 +1008,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 +1021,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/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/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 7c91bcf07..3127288ff 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 { @@ -694,6 +713,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 @@ -726,7 +758,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 +1065,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 +1138,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 +1666,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..bf7dddd31 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( @@ -1062,15 +1076,79 @@ fn find_angular_decorator<'a>( None } +/// Whether `decorator`'s options object opts out of AOT compilation via +/// `jit`. +/// +/// Mirrors ngtsc's `extractDirectiveMetadata` / `NgModuleDecoratorHandler.analyze` +/// (`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 { + 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 true; + } + } + 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 /// 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 +1179,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}, {})", @@ -1350,6 +1431,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); @@ -1659,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 @@ -2016,6 +2116,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 +2440,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 +2547,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 @@ -2445,7 +2616,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) @@ -2459,6 +2637,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 +2783,129 @@ 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, + )); + // `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, + ); + } + } + + 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(); @@ -2778,9 +3099,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 +3140,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 +3415,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 +3525,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 +3649,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 +3753,6 @@ pub fn transform_angular_file( &allocator, class, &class_name, - service_decorator, options, source, &string_consts, @@ -3511,22 +3830,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(), @@ -3547,13 +3860,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 +3943,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 +3954,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; @@ -5598,6 +5964,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(); diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 4ebbf81b3..e184ee8ee 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -1043,7 +1043,9 @@ fn upsert_input<'a>(inputs: &mut Vec<'a, R3InputMetadata<'a>>, input: R3InputMet /// 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. +/// 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<'_>, @@ -1091,12 +1093,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 +1129,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 +1158,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 +1176,19 @@ 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. 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 // `parseOutputFields`. if let (Some(decorator), Some(value)) = (decorator, value) { @@ -1163,6 +1204,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/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/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 3034eedd3..70cfaf864 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; // ============================================================================ @@ -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<'_>>, @@ -980,7 +981,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 +1103,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) } }; @@ -1196,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) => { @@ -1381,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) => { @@ -1642,7 +1652,10 @@ 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) + // 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.r#static) @@ -1650,6 +1663,9 @@ pub(crate) fn extract_host_listeners_in<'a>( ClassElement::PropertyDefinition(prop) => { (&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, }; @@ -1941,9 +1957,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>, @@ -1951,19 +1970,82 @@ 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) + // 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, 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, 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)) + }); + // 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() + { + 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 }) } @@ -1989,8 +2071,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/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/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/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/decorator_metadata_ngtsc_test.rs b/crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs index 52999089b..336c212f0 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, @@ -1277,3 +1350,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())] + ); +} 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", diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 55d898b4b..b950fc97c 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}" ); } } @@ -6186,6 +6213,40 @@ export class D { ); } +/// @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`). @@ -6193,8 +6254,8 @@ export class D { 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', @@ -6203,10 +6264,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); } "; @@ -9246,6 +9307,153 @@ 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: upstream emits `null` for a param + // with no type and no (Angular) decorators, and it's lowered as __param. + assert!( + compact.contains("type:Injectable}]},\nnull]") + || compact.contains("type:Injectable}]},null]"), + "foreign-decorated param should be emitted as null. 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 // ========================================================================= @@ -12307,6 +12515,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 // ============================================================================= @@ -15534,3 +15813,563 @@ 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_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'; + +@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, 0); + assert!(!result.code.contains("static ɵcmp"), "Got:\n{}", result.code); + assert!(result.code.contains("__decorate"), "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 + ); +} + +// ============================================================================ +// 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_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.is_empty(), + "@Pipe members and metadata should not get io/query diagnostics. 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. + // 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));", + ] { + 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_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 [ + "@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 + ); +} 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}"); +} 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..2245a0e61 --- /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 }] + }, + null + ]; +}; +TestComponent = __decorate([Component({ + selector: "app-test", + template: "" +}), __param(3, ForeignInject(TOKEN))], TestComponent); +export { TestComponent }; 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 } ]; }; 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 } } 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