diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 120427316..a19eb239f 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -19,7 +19,9 @@ use crate::output::ast::{ ArrowFunctionBody, ArrowFunctionExpr, LiteralArrayExpr, LiteralExpr, LiteralMapEntry, LiteralMapExpr, LiteralValue, OutputExpression, ReadPropExpr, ReadVarExpr, }; -use crate::output::oxc_converter::convert_oxc_expression; +use crate::output::oxc_converter::{ + convert_oxc_expression, convert_plain_properties, is_plain_property, +}; /// Build the decorators metadata array expression. /// @@ -101,7 +103,20 @@ pub fn build_decorator_metadata_array<'a>( let mut args = AllocVec::new_in(&allocator); for (arg_idx, arg) in call.arguments.iter().enumerate() { let expr = arg.to_expression(); - if let Some(mut converted) = convert_oxc_expression(allocator, expr, source_text) { + 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), + }; + 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. @@ -154,6 +169,18 @@ pub fn build_decorator_metadata_array<'a>( )) } +/// Whether a `@Component` metadata object has a resource field that ngtsc +/// inlines: `templateUrl`, `styleUrl`, `styleUrls` or `styles`. +fn has_resource_property(obj: &oxc_ast::ast::ObjectExpression<'_>) -> bool { + obj.properties.iter().any(|prop| { + let ObjectPropertyKind::ObjectProperty(prop) = prop else { return false }; + is_plain_property(prop) + && get_property_key_name(&prop.key).is_some_and(|name| { + matches!(name.as_str(), "templateUrl" | "styleUrl" | "styleUrls" | "styles") + }) + }) +} + /// Rewrite the `@Component` config map so external resource references are /// inlined into the `setClassMetadata` args. /// diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index 3b1f93a9a..f5ae9803e 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -22,6 +22,7 @@ use crate::directive::{ extract_output_metadata, }; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; /// Extract component metadata from a class with decorators. /// @@ -86,7 +87,9 @@ pub fn extract_component_metadata<'a>( // Parse each property in the config object for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let key_name = get_property_key_name(&prop.key, consts)?; match key_name.as_str() { @@ -509,7 +512,9 @@ fn extract_host_metadata<'a>( }; for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key, consts) else { continue; }; @@ -611,7 +616,9 @@ fn extract_single_host_directive<'a>( let mut is_forward_reference = false; for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key, consts) else { continue; }; diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index b45fda650..e6b09537c 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -20,6 +20,7 @@ use super::metadata::{ use crate::factory::R3DependencyMetadata; use crate::output::ast::{OutputAstBuilder, OutputExpression, ReadVarExpr}; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; /// Find the @Directive decorator in a list of decorators. pub(crate) fn find_directive_decorator<'a>( @@ -121,7 +122,9 @@ pub fn extract_directive_metadata<'a>( // Parse each property in the config object (if present) if let Some(config_obj) = config_obj { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key, consts) else { continue; }; @@ -624,7 +627,9 @@ fn extract_host_metadata<'a>( let mut host = R3HostMetadata::new(allocator); for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key, consts) else { continue; }; @@ -714,7 +719,9 @@ fn extract_single_host_directive<'a>( let mut is_forward_reference = false; for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key, consts) else { continue; }; diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 77b094287..9efc8f7aa 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -21,6 +21,7 @@ use oxc_str::Ident; use super::metadata::{QueryPredicate, R3InputMetadata, R3QueryMetadata}; use crate::output::ast::OutputExpression; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; // ============================================================================ // Helper Functions @@ -169,7 +170,9 @@ fn parse_input_config<'a>( let mut config = InputConfig::default(); for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; @@ -280,7 +283,9 @@ pub(crate) fn try_parse_signal_model<'a>( if let Some(options_arg) = call_expr.arguments.get(options_arg_index) { if let Argument::ObjectExpression(obj) = options_arg { for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; @@ -357,7 +362,9 @@ pub(crate) fn try_parse_signal_output<'a>( if let Some(Argument::ObjectExpression(obj)) = call_expr.arguments.get(options_idx) { for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; @@ -465,7 +472,9 @@ pub(crate) fn try_parse_signal_input<'a>( if let Some(options_arg) = call_expr.arguments.get(options_arg_index) { if let Argument::ObjectExpression(obj) = options_arg { for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; @@ -816,7 +825,9 @@ fn parse_query_config<'a>( if let Some(second_arg) = call.arguments.get(1) { if let Argument::ObjectExpression(obj) = second_arg { for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; @@ -982,7 +993,9 @@ fn try_parse_signal_query<'a>( if let Some(second_arg) = call_expr.arguments.get(1) { if let Argument::ObjectExpression(obj) = second_arg { for prop in &obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; diff --git a/crates/oxc_angular_compiler/src/injectable/decorator.rs b/crates/oxc_angular_compiler/src/injectable/decorator.rs index 0ab8f4528..105f47d6d 100644 --- a/crates/oxc_angular_compiler/src/injectable/decorator.rs +++ b/crates/oxc_angular_compiler/src/injectable/decorator.rs @@ -14,6 +14,7 @@ use oxc_str::Ident; use crate::factory::R3DependencyMetadata; use crate::output::ast::{OutputExpression, ReadVarExpr}; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; /// Extracted injectable metadata from a `@Injectable` decorator. #[derive(Debug)] @@ -316,7 +317,9 @@ fn extract_provided_in<'a>( source_text: Option<&'a str>, ) -> Option> { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "providedIn" { return parse_provided_in_value(allocator, &prop.value, source_text); @@ -355,7 +358,9 @@ fn extract_use_class<'a>( source_text: Option<&'a str>, ) -> Option> { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "useClass" { let (class_expr, is_forward_ref) = @@ -375,7 +380,9 @@ fn extract_use_factory<'a>( source_text: Option<&'a str>, ) -> Option> { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "useFactory" { let factory = convert_oxc_expression(allocator, &prop.value, source_text)?; @@ -394,7 +401,9 @@ fn extract_use_value<'a>( source_text: Option<&'a str>, ) -> Option> { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "useValue" { return convert_oxc_expression(allocator, &prop.value, source_text); @@ -411,7 +420,9 @@ fn extract_use_existing<'a>( source_text: Option<&'a str>, ) -> Option> { for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "useExisting" { let (existing, is_forward_ref) = @@ -459,7 +470,9 @@ fn extract_deps_from_config<'a>( let mut deps = Vec::new_in(&allocator); for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { if let Some(key_name) = get_property_key_name(&prop.key) { if key_name.as_str() == "deps" { if let Expression::ArrayExpression(arr) = &prop.value { diff --git a/crates/oxc_angular_compiler/src/ng_module/decorator.rs b/crates/oxc_angular_compiler/src/ng_module/decorator.rs index 0586413ff..289a5fd6a 100644 --- a/crates/oxc_angular_compiler/src/ng_module/decorator.rs +++ b/crates/oxc_angular_compiler/src/ng_module/decorator.rs @@ -14,6 +14,7 @@ use oxc_str::Ident; use crate::factory::R3DependencyMetadata; use crate::output::ast::{OutputExpression, ReadVarExpr}; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; /// Extracted NgModule metadata from a `@NgModule` decorator. /// @@ -213,7 +214,9 @@ pub fn extract_ng_module_metadata<'a>( // Parse each property in the config object for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; diff --git a/crates/oxc_angular_compiler/src/output/emitter.rs b/crates/oxc_angular_compiler/src/output/emitter.rs index 545731069..8dfa788b3 100644 --- a/crates/oxc_angular_compiler/src/output/emitter.rs +++ b/crates/oxc_angular_compiler/src/output/emitter.rs @@ -776,7 +776,12 @@ impl JsEmitter { OutputExpression::Parenthesized(p) => p.expr.as_ref(), other => other, }; - let is_object_literal = matches!(inner, OutputExpression::LiteralMap(_)); + // Objects the converter keeps as written arrive as raw source. + let is_object_literal = match inner { + OutputExpression::LiteralMap(_) => true, + OutputExpression::RawSource(raw) => raw.source.starts_with('{'), + _ => false, + }; if is_object_literal { ctx.print("("); } diff --git a/crates/oxc_angular_compiler/src/output/oxc_converter.rs b/crates/oxc_angular_compiler/src/output/oxc_converter.rs index 6960c757a..59f661562 100644 --- a/crates/oxc_angular_compiler/src/output/oxc_converter.rs +++ b/crates/oxc_angular_compiler/src/output/oxc_converter.rs @@ -14,7 +14,7 @@ use oxc_allocator::{Allocator, Box, Vec as OxcVec}; use oxc_ast::ast::{ Argument, ArrayExpressionElement, BindingPattern, Expression, ObjectPropertyKind, PropertyKey, - UnaryOperator as OxcUnaryOperator, + PropertyKind, UnaryOperator as OxcUnaryOperator, }; use oxc_span::Span; use oxc_str::Ident; @@ -300,25 +300,57 @@ fn convert_object_expression<'a>( allocator: &'a Allocator, obj: &oxc_ast::ast::ObjectExpression<'a>, source_text: Option<&'a str>, +) -> Option> { + // A `LiteralMap` entry is a plain `key: value` pair, so methods (`a() {}`), + // accessors (`get a() {}`), computed keys (`[a]: 1`) and other key kinds + // cannot be represented. Keep such objects as written rather than emitting + // an altered object. + if obj + .properties + .iter() + .any(|prop| matches!(prop, ObjectPropertyKind::ObjectProperty(p) if !is_plain_property(p))) + { + return make_raw_source(allocator, source_text, obj.span); + } + convert_plain_properties(allocator, obj, source_text) +} + +/// Whether a property is a `key: value` pair with an identifier, string or number key. +pub fn is_plain_property(p: &oxc_ast::ast::ObjectProperty<'_>) -> bool { + !p.method + && !p.computed + && p.kind == PropertyKind::Init + && matches!( + p.key, + PropertyKey::StaticIdentifier(_) + | PropertyKey::StringLiteral(_) + | PropertyKey::NumericLiteral(_) + ) +} + +/// Convert an object expression, keeping only its plain `key: value` pairs and +/// spreads. ngtsc rebuilds a component's metadata the same way when it inlines +/// resources (`transformDecoratorResources`). +pub fn convert_plain_properties<'a>( + allocator: &'a Allocator, + obj: &oxc_ast::ast::ObjectExpression<'a>, + source_text: Option<&'a str>, ) -> Option> { let mut entries = OxcVec::with_capacity_in(obj.properties.len(), &allocator); for prop in &obj.properties { match prop { ObjectPropertyKind::ObjectProperty(p) => { - // Get the property key + if !is_plain_property(p) { + continue; + } let (key, quoted) = match &p.key { PropertyKey::StaticIdentifier(id) => (id.name.clone().into(), false), PropertyKey::StringLiteral(lit) => (lit.value.clone().into(), true), PropertyKey::NumericLiteral(lit) => { (Ident::from(allocator.alloc_str(&lit.value.to_string())), true) } - PropertyKey::PrivateIdentifier(_) => return None, // Private fields not supported - _ => { - // Computed property key - try to convert it - // For now, skip computed properties - continue; - } + _ => continue, }; // Convert the value @@ -760,6 +792,10 @@ fn strip_expression_types(expr_source: &str) -> String { return inner.to_string(); } } + // Codegen drops parentheses it doesn't need, e.g. "0, { a() {} };" + if let Some(inner) = code.strip_prefix("0, ").and_then(|rest| rest.strip_suffix(';')) { + return inner.to_string(); + } // Fallback: return original expr_source.to_string() @@ -1151,4 +1187,20 @@ mod tests { assert!(!result.contains(": number"), "Should strip type annotation. Got: {result}"); assert!(result.contains("=> x + 1"), "Should preserve expression. Got: {result}"); } + + #[test] + fn test_object_with_method_falls_back_to_raw_source_without_types() { + let allocator = Allocator::default(); + let source = "{ attach(x: string): string { return x; }, [key]: 1 }"; + let expr = Parser::new(&allocator, source, SourceType::ts()) + .parse_expression() + .expect("Failed to parse expression"); + let result = convert_oxc_expression(&allocator, &expr, Some(source)); + let Some(OutputExpression::RawSource(raw)) = result else { + panic!("Expected RawSource expression, got {result:?}"); + }; + let raw = raw.source.as_str(); + assert!(!raw.contains(": string"), "Should strip type annotations. Got: {raw}"); + assert!(raw.contains("attach(x)") && raw.contains("[key]: 1"), "Got: {raw}"); + } } diff --git a/crates/oxc_angular_compiler/src/pipe/decorator.rs b/crates/oxc_angular_compiler/src/pipe/decorator.rs index 9bbebba97..eac0e8715 100644 --- a/crates/oxc_angular_compiler/src/pipe/decorator.rs +++ b/crates/oxc_angular_compiler/src/pipe/decorator.rs @@ -15,6 +15,7 @@ use super::metadata::R3PipeMetadata; use crate::factory::R3DependencyMetadata; use crate::output::ast::{OutputExpression, ReadVarExpr}; use crate::output::oxc_converter::convert_oxc_expression; +use crate::util::is_metadata_property; /// Extracted pipe metadata from a `@Pipe` decorator. /// @@ -141,7 +142,9 @@ pub fn extract_pipe_metadata<'a>( // Parse each property in the config object for prop in &config_obj.properties { - if let ObjectPropertyKind::ObjectProperty(prop) = prop { + if let ObjectPropertyKind::ObjectProperty(prop) = prop + && is_metadata_property(prop) + { let Some(key_name) = get_property_key_name(&prop.key) else { continue; }; diff --git a/crates/oxc_angular_compiler/src/service/decorator.rs b/crates/oxc_angular_compiler/src/service/decorator.rs index 3b708bea2..c5a8840a7 100644 --- a/crates/oxc_angular_compiler/src/service/decorator.rs +++ b/crates/oxc_angular_compiler/src/service/decorator.rs @@ -19,6 +19,7 @@ use oxc_str::Ident; use super::metadata::R3ServiceMetadata; use crate::output::ast::{OutputExpression, ReadVarExpr}; +use crate::util::is_metadata_property; /// Extracted metadata from a `@Service` decorator. #[derive(Debug)] @@ -84,6 +85,9 @@ pub fn extract_service_metadata<'a>( for prop in &config_obj.properties { let ObjectPropertyKind::ObjectProperty(prop) = prop else { continue }; + if !is_metadata_property(prop) { + continue; + } let Some(key) = get_property_key_name(&prop.key) else { continue }; match key.as_str() { diff --git a/crates/oxc_angular_compiler/src/util/mod.rs b/crates/oxc_angular_compiler/src/util/mod.rs index 1ed0d18c4..4a58bd698 100644 --- a/crates/oxc_angular_compiler/src/util/mod.rs +++ b/crates/oxc_angular_compiler/src/util/mod.rs @@ -8,3 +8,9 @@ mod type_extract; pub use deferred_time::*; pub use parse_util::*; pub use type_extract::*; + +/// Whether a property of a decorator's options object is read as metadata. +/// Like ngtsc's `reflectObjectLiteral`, methods and accessors are not. +pub fn is_metadata_property(prop: &oxc_ast::ast::ObjectProperty<'_>) -> bool { + !prop.method && prop.kind == oxc_ast::ast::PropertyKind::Init +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 3edea3221..cb656d259 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -11234,6 +11234,185 @@ export class MyComponent {} ); } +/// Object literals in decorator metadata must keep methods, accessors and +/// computed keys. The converter used to re-emit `attach() {}` as the invalid +/// `attach:() {}`, drop `get`/`set`/`async`/`*`, and silently drop computed keys. +#[test] +fn test_object_methods_and_computed_keys_in_decorator_metadata_preserved() { + let members = [ + ("attach() {}", "attach() {}"), + ("get ready() { return 1; }", "get ready() {"), + ("set ready(v) {}", "set ready(v) {"), + ("async load() {}", "async load() {"), + ("*items() {}", "*items() {"), + ("[key]() {}", "[key]() {"), + ("[key]: 1", "[key]: 1"), + ]; + let fields = [ + ("Component", "providers"), + ("Component", "viewProviders"), + ("Directive", "providers"), + ("NgModule", "providers"), + ]; + for (member, expected) in members { + for (decorator, field) in fields { + let extra = if decorator == "Component" { "template: '
'," } else { "" }; + let source = format!( + "import {{ {decorator} }} from '@angular/core'; +class Token {{}} +const key = 'k'; +@{decorator}({{ selector: 'app-repro', {extra} {field}: [{{ provide: Token, useValue: {{ {member} }} }}] }}) +export class Repro {{}} +" + ); + let code = compile_to_valid_js(&source); + // Once in the definition, once in setClassMetadata. + assert_eq!( + code.matches(expected).count(), + 2, + "`{member}` lost in @{decorator} {field}. Got:\n{code}" + ); + } + } +} + +/// Compiles `source`, asserting there are no errors and the output parses as JS. +fn compile_to_valid_js(source: &str) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(!result.has_errors(), "Unexpected errors {:?} for:\n{source}", result.diagnostics); + let parsed = + oxc_parser::Parser::new(&allocator, &result.code, oxc_span::SourceType::mjs()).parse(); + assert!( + parsed.diagnostics.is_empty(), + "Output is not valid JS ({:?}). Got:\n{}", + parsed.diagnostics, + result.code + ); + result.code +} + +/// Keys that aren't identifiers, strings or numbers (here a BigInt, which only a +/// `@ts-ignore` lets through) keep the object as written, as ngc does, rather +/// than failing the conversion and losing the whole field. +#[test] +fn test_object_with_bigint_key_kept_as_written() { + let code = compile_to_valid_js( + "import { Component } from '@angular/core'; +class Token {} +@Component({ selector: 'a', template: '', providers: [{ provide: Token, useValue: { 1n: 5, b: 2 } }] }) +export class C {} +", + ); + assert_eq!(code.matches("{ 1n: 5, b: 2 }").count(), 2, "Got:\n{code}"); + assert!(code.contains("selector:\"a\""), "setClassMetadata keeps its args. Got:\n{code}"); +} + +/// When a component's resources are inlined, ngc rebuilds its metadata object +/// for `setClassMetadata` from the plain properties, dropping methods, accessors +/// and computed keys. Without resources it keeps the object as written. +#[test] +fn test_component_metadata_with_accessor_and_resources_matches_ngc() { + let source = "import { Component } from '@angular/core'; +const key = 'k'; +@Component({ selector: 'b', templateUrl: './x.html', get foo() { return 1; }, [key]: 2 }) +export class B {} +@Component({ selector: 'c', template: '', get foo() { return 1; } }) +export class C {} +"; + let allocator = Allocator::default(); + let mut templates = std::collections::HashMap::new(); + templates.insert("./x.html".to_string(), "
hi
".to_string()); + let resources = ResolvedResources { templates, styles: std::collections::HashMap::new() }; + let result = transform_angular_file(&allocator, "test.ts", source, None, Some(&resources)); + assert!(!result.has_errors(), "Unexpected errors: {:?}", result.diagnostics); + let code = &result.code; + + assert!( + code.contains("args:[{selector:\"b\",template:\"
hi
\"}]"), + "Resources are inlined into a rebuilt object. Got:\n{code}" + ); + assert!(!code.contains("templateUrl"), "Got:\n{code}"); + assert_eq!(code.matches("get foo()").count(), 1, "Only C keeps its accessor. Got:\n{code}"); +} + +/// An arrow whose body is an object literal must keep its parentheses, even when +/// the object is kept as written because it has methods or computed keys. +#[test] +fn test_arrow_returning_object_with_methods_keeps_parentheses() { + for body in ["({ attach() { return 1; } })", "({ [key]: 1 })"] { + let component = compile_to_valid_js(&format!( + "import {{ Component }} from '@angular/core'; +class Token {{}} +const key = 'k'; +@Component({{ selector: 'a', template: '', providers: [{{ provide: Token, useFactory: () => {body} }}] }}) +export class C {{}} +" + )); + assert_eq!(component.matches(body).count(), 2, "Got:\n{component}"); + + let injectable = compile_to_valid_js(&format!( + "import {{ Injectable }} from '@angular/core'; +const key = 'k'; +@Injectable({{ providedIn: 'root', useFactory: () => {body} }}) +export class S {{}} +" + )); + assert_eq!(injectable.matches(body).count(), 2, "Got:\n{injectable}"); + } +} + +/// Like ngtsc's `reflectObjectLiteral`, methods and accessors on a decorator's +/// own options object are not metadata. `setClassMetadata` keeps them as written. +/// Expected output checked against ngc 22. +#[test] +fn test_methods_and_accessors_on_decorator_options_are_ignored() { + let code = compile_to_valid_js( + "import { Component, ContentChild, Directive, Injectable, Input, NgModule, Pipe, ViewChild, input, model, output, viewChild } from '@angular/core'; +class Token {} +@Injectable({ providedIn: 'root', useFactory() { return 1; } }) +export class S1 {} +@Injectable({ providedIn: 'root', get useValue() { return 1; } }) +export class S2 {} +@Directive({ selector: '[d]', get providers() { return []; } }) +export class D {} +@NgModule({ get providers() { return []; } }) +export class M {} +@Pipe({ name: 'p', get pure() { return false; } }) +export class P {} +@Component({ selector: 'c', template: '', get viewProviders() { return []; } }) +export class C { + @Input({ alias: 'value', transform(v: string) { return v; } }) value = ''; + @ViewChild('x', { static: true, get read() { return Token; } }) x; + @ContentChild('y', { get descendants() { return false; } }) y; +} +@Component({ selector: 's', template: '
' }) +export class Signals { + a = input(0, { get alias() { return 'aliasA'; } }); + m = model(0, { get alias() { return 'aliasM'; } }); + o = output({ get alias() { return 'aliasO'; } }); + q = viewChild('z', { get read() { return Token; } }); +} +", + ); + let definitions = code.split("ɵsetClassMetadata").next().unwrap(); + for ignored in ["useFactory", "useValue", "ɵɵProvidersFeature", "providers:", "transform"] { + assert!(!definitions.contains(ignored), "`{ignored}` should be ignored. Got:\n{code}"); + } + for expected in [ + "factory:S1.ɵfac", + "factory:S2.ɵfac", + "pure:true", + "i0.ɵɵviewQuery(_c1,7);", + "i0.ɵɵcontentQuery(dirIndex,_c0,5);", + "inputs:{a:[1,\"a\"],m:[1,\"m\"]}", + "outputs:{m:\"mChange\",o:\"o\"}", + ] { + assert!(code.contains(expected), "Expected `{expected}`. Got:\n{code}"); + } + assert!(code.contains("get providers() {"), "setClassMetadata keeps members. Got:\n{code}"); +} + // ============================================================================= // Regression: @Inject(TOKEN) on pipe constructor parameters // ============================================================================= diff --git a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts index 967d5ba80..fe9cfd192 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts @@ -25,6 +25,8 @@ const INCREMENTAL_HYDRATION = 'ɵɵenableIncrementalHydrationRuntime is not emitted for a `hydrate` trigger' 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' export const KNOWN_DIFFERENCES: Record = { 'animations/animation-metadata-with-change-detection': { @@ -193,6 +195,26 @@ export const KNOWN_DIFFERENCES: Record = { fields: ['MyPurePipe.ɵfac'], reasons: [FACTORY], }, + 'providers/object-members-injectable-use-factory': { + fields: ['ObjectMembersService.ɵfac', 'ObjectMembersService.ɵprov'], + reasons: [FACTORY, INJECTABLE_FACTORY_WRAPPER], + }, + 'providers/object-members-injectable-use-value': { + fields: ['ObjectMembersValueService.ɵfac'], + reasons: [FACTORY], + }, + 'providers/object-members-ng-module': { + fields: ['ObjectMembersModule.ɵfac'], + reasons: [FACTORY], + }, + 'providers/object-members-options-directive-pipe-module': { + fields: ['OptionsPipe.ɵfac', 'OptionsModule.ɵfac'], + reasons: [FACTORY], + }, + 'providers/object-members-options-injectable': { + fields: ['OptionsMethodService.ɵfac', 'OptionsGetterService.ɵfac'], + reasons: [FACTORY], + }, 'providers/providers-with-change-detection': { fields: ['ProvidersWithChangeDetectionComponent.ɵcmp'], reasons: [CHANGE_DETECTION_V22], diff --git a/napi/angular-compiler/e2e/compare/fixtures/providers/object-members.fixture.ts b/napi/angular-compiler/e2e/compare/fixtures/providers/object-members.fixture.ts new file mode 100644 index 000000000..a07111bd9 --- /dev/null +++ b/napi/angular-compiler/e2e/compare/fixtures/providers/object-members.fixture.ts @@ -0,0 +1,352 @@ +/** + * Object literal members in decorator metadata that are not plain `key: value` + * pairs: methods, accessors, generators, computed keys and so on. + * + * Angular passes these expressions through unchanged, both in the definition + * and in `setClassMetadata`, so the output must keep every member as written. + */ +import type { Fixture } from '../types.js' + +function providerFixture(name: string, description: string, value: string): Fixture { + return { + name: `object-members-${name}`, + category: 'providers', + description, + className: 'ObjectMembersComponent', + type: 'full-transform', + sourceCode: ` +import { Component, InjectionToken } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); +const key = 'computedKey'; +class Base { base() { return 'base'; } } + +@Component({ + selector: 'app-object-members', + template: '
', + providers: [{ provide: TOKEN, useValue: ${value} }], +}) +export class ObjectMembersComponent {} + `.trim(), + expectedFeatures: ['ɵɵProvidersFeature', 'ɵsetClassMetadata'], + } +} + +export const fixtures: Fixture[] = [ + // ========================================================================== + // Member kinds + // ========================================================================== + + providerFixture('method', 'Method shorthand', `{ attach() { return 1; } }`), + providerFixture('getter', 'Getter', `{ get ready() { return true; } }`), + providerFixture('setter', 'Setter', `{ set ready(value: boolean) {} }`), + providerFixture( + 'accessor-pair', + 'Getter and setter for the same key', + `{ _v: 1, get v() { return this._v; }, set v(value: number) { this._v = value; } }`, + ), + providerFixture('async-method', 'Async method', `{ async load() { return 1; } }`), + providerFixture('generator', 'Generator method', `{ *items() { yield 1; } }`), + providerFixture('async-generator', 'Async generator method', `{ async *stream() { yield 1; } }`), + providerFixture('computed-method', 'Computed-key method', `{ [key]() { return 1; } }`), + providerFixture('computed-property', 'Computed-key property', `{ [key]: 1 }`), + providerFixture( + 'computed-string-literal', + 'Computed key that is a string literal', + `{ ['literal']: 1 }`, + ), + providerFixture( + 'symbol-method', + 'Well-known symbol method', + `{ *[Symbol.iterator]() { yield 1; } }`, + ), + providerFixture( + 'string-and-numeric-method-keys', + 'Methods with string and numeric keys', + `{ 'with-dash'() { return 1; }, 42() { return 2; } }`, + ), + providerFixture( + 'this-and-super', + 'Methods using this and super', + `Object.setPrototypeOf({ count: 1, read() { return this.count + super.toString().length; } }, new Base())`, + ), + + // ========================================================================== + // Signatures and bodies + // ========================================================================== + + providerFixture( + 'typed-method', + 'Parameter, return and generic type annotations are stripped', + `{ map(value: T, fallback?: T, ...rest: T[]): T { return value ?? fallback ?? rest[0]; } }`, + ), + providerFixture( + 'default-and-destructured-params', + 'Default and destructured parameters', + `{ configure({ a, b }: { a: number; b: number }, [c]: number[] = [3], d = a + b) { return a + b + c + d; } }`, + ), + providerFixture( + 'body-with-template-literal-and-comments', + 'Method body with a template literal and comments', + `{ + describe(name: string) { + // explain + return \`hello \${name}\`; /* trailing */ + }, + }`, + ), + + // ========================================================================== + // Mixed and nested objects + // ========================================================================== + + providerFixture( + 'mixed-members', + 'Spread, shorthand, plain and method members together', + `{ ...{ spread: 1 }, key, plain: 'x', method() { return key; } }`, + ), + providerFixture( + 'nested-in-plain-object', + 'Method object nested inside a plain object and array', + `{ plain: 1, list: [{ nested() { return 2; } }] }`, + ), + providerFixture( + 'call-argument', + 'Method object passed to a call', + `Object.freeze({ attach() { return 1; } })`, + ), + + // ========================================================================== + // Other decorator fields and decorators + // ========================================================================== + + { + name: 'object-members-use-factory', + category: 'providers', + description: 'Factory returning an object with methods', + className: 'ObjectMembersFactoryComponent', + type: 'full-transform', + sourceCode: ` +import { Component, InjectionToken } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); + +@Component({ + selector: 'app-object-members-factory', + template: '
', + providers: [{ provide: TOKEN, useFactory: () => ({ attach() { return 1; }, get ready() { return true; } }) }], +}) +export class ObjectMembersFactoryComponent {} + `.trim(), + expectedFeatures: ['ɵɵProvidersFeature', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-view-providers', + category: 'providers', + description: 'viewProviders with methods and computed keys', + className: 'ObjectMembersViewProvidersComponent', + type: 'full-transform', + sourceCode: ` +import { Component, InjectionToken } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); +const key = 'computedKey'; + +@Component({ + selector: 'app-object-members-view-providers', + template: '
', + viewProviders: [{ provide: TOKEN, useValue: { attach() { return 1; }, [key]: 2 } }], +}) +export class ObjectMembersViewProvidersComponent {} + `.trim(), + expectedFeatures: ['ɵɵProvidersFeature', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-directive', + category: 'providers', + description: 'Directive providers with methods', + className: 'ObjectMembersDirective', + type: 'full-transform', + sourceCode: ` +import { Directive, InjectionToken } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); + +@Directive({ + selector: '[appObjectMembers]', + providers: [{ provide: TOKEN, useValue: { attach() { return 1; }, get ready() { return true; } } }], +}) +export class ObjectMembersDirective {} + `.trim(), + expectedFeatures: ['ɵɵdefineDirective', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-ng-module', + category: 'providers', + description: 'NgModule providers with methods', + className: 'ObjectMembersModule', + type: 'full-transform', + sourceCode: ` +import { NgModule, InjectionToken } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); +const key = 'computedKey'; + +@NgModule({ + providers: [{ provide: TOKEN, useValue: { async load() { return 1; }, [key]() { return 2; } } }], +}) +export class ObjectMembersModule {} + `.trim(), + expectedFeatures: ['ɵɵdefineInjector', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-injectable-use-factory', + category: 'providers', + description: 'Injectable useFactory returning an object with methods', + className: 'ObjectMembersService', + type: 'full-transform', + sourceCode: ` +import { Injectable } from '@angular/core'; + +@Injectable({ + providedIn: 'root', + useFactory: () => ({ attach() { return 1; }, get ready() { return true; } }), +}) +export class ObjectMembersService {} + `.trim(), + expectedFeatures: ['ɵɵdefineInjectable', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-injectable-use-value', + category: 'providers', + description: 'Injectable useValue with methods', + className: 'ObjectMembersValueService', + type: 'full-transform', + sourceCode: ` +import { Injectable } from '@angular/core'; + +@Injectable({ + providedIn: 'root', + useValue: { attach() { return 1; } }, +}) +export class ObjectMembersValueService {} + `.trim(), + expectedFeatures: ['ɵɵdefineInjectable', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-host-computed-key', + category: 'providers', + description: 'host metadata with a computed string key', + className: 'ObjectMembersHostComponent', + type: 'full-transform', + sourceCode: ` +import { Component } from '@angular/core'; + +@Component({ + selector: 'app-object-members-host', + template: '
', + host: { ['(click)']: 'onClick()', class: 'plain' }, +}) +export class ObjectMembersHostComponent { + onClick() {} +} + `.trim(), + expectedFeatures: ['hostBindings', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-animations', + category: 'providers', + description: 'animations metadata containing an object with a method', + className: 'ObjectMembersAnimationsComponent', + type: 'full-transform', + sourceCode: ` +import { Component } from '@angular/core'; + +@Component({ + selector: 'app-object-members-animations', + template: '
', + animations: [{ type: 7, name: 'fade', definitions: [], options: { params: { get delay() { return 1; } } } }], +}) +export class ObjectMembersAnimationsComponent {} + `.trim(), + expectedFeatures: ['ɵɵdefineComponent', 'ɵsetClassMetadata'], + }, + + // ========================================================================== + // Methods and accessors on a decorator's own options object are ignored, + // like ngtsc's reflectObjectLiteral. setClassMetadata keeps them. + // (`@Input({ transform() {} })` is a compile error in ngc, NG1010.) + // ========================================================================== + + { + name: 'object-members-options-injectable', + category: 'providers', + description: '@Injectable options with a useFactory method and a useValue getter', + className: 'OptionsMethodService', + type: 'full-transform', + sourceCode: ` +import { Injectable } from '@angular/core'; + +@Injectable({ providedIn: 'root', useFactory() { return 1; } }) +export class OptionsMethodService {} + +@Injectable({ providedIn: 'root', get useValue() { return 1; } }) +export class OptionsGetterService {} + `.trim(), + expectedFeatures: ['ɵɵdefineInjectable', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-options-directive-pipe-module', + category: 'providers', + description: 'Accessors on @Directive, @Pipe and @NgModule options', + className: 'OptionsDirective', + type: 'full-transform', + sourceCode: ` +import { Directive, NgModule, Pipe } from '@angular/core'; + +@Directive({ selector: '[appOptions]', get providers() { return []; } }) +export class OptionsDirective {} + +@Pipe({ name: 'options', get pure() { return false; } }) +export class OptionsPipe { + transform(value: unknown) { return value; } +} + +@NgModule({ get providers() { return []; } }) +export class OptionsModule {} + `.trim(), + expectedFeatures: ['ɵɵdefineDirective', 'ɵɵdefinePipe', 'ɵsetClassMetadata'], + }, + + { + name: 'object-members-options-component-queries', + category: 'providers', + description: 'Accessors on @Component and query options', + className: 'OptionsComponent', + type: 'full-transform', + sourceCode: ` +import { Component, ContentChild, InjectionToken, ViewChild } from '@angular/core'; + +const TOKEN = new InjectionToken('TOKEN'); + +@Component({ + selector: 'app-options', + template: '
', + get viewProviders() { return []; }, +}) +export class OptionsComponent { + @ViewChild('x', { static: true, get read() { return TOKEN; } }) x: unknown; + @ContentChild('y', { get descendants() { return false; } }) y: unknown; +} + `.trim(), + expectedFeatures: ['ɵɵdefineComponent', 'viewQuery', 'ɵsetClassMetadata'], + }, +]