From 6b7827eed0edb5b3078801ea3d5e14138b7808aa Mon Sep 17 00:00:00 2001 From: Ashley Hunter Date: Thu, 24 Sep 2026 21:05:21 +0100 Subject: [PATCH] fix: keep object methods, accessors and computed keys in decorator metadata Object literals in decorator metadata lost any member that is not a plain `key: value` pair. `{ attach() {} }` compiled to the invalid `{attach:() {}}`, `get`/`set`/`async`/`*` were dropped, and computed keys disappeared, with no error. Such objects are now kept as written, with TypeScript types stripped, as ngtsc does. Type stripping also failed for objects because codegen drops the redundant parentheses the unwrap relied on, and an arrow returning such an object now keeps the parentheses around its body. When a component's resources are inlined, its metadata object in `setClassMetadata` is rebuilt from the plain properties only, matching ngtsc's `transformDecoratorResources`. Methods and accessors on a decorator's own options object, such as `@Injectable({ useFactory() {} })` or `@ViewChild('x', { get read() {} })`, were read as `key: value` pairs and emitted as invalid code. Like ngtsc's `reflectObjectLiteral`, they are now ignored. --- .../src/class_metadata/builders.rs | 31 +- .../src/component/decorator.rs | 13 +- .../src/directive/decorator.rs | 13 +- .../src/directive/property_decorators.rs | 25 +- .../src/injectable/decorator.rs | 25 +- .../src/ng_module/decorator.rs | 5 +- .../src/output/emitter.rs | 7 +- .../src/output/oxc_converter.rs | 68 +++- .../src/pipe/decorator.rs | 5 +- .../src/service/decorator.rs | 4 + crates/oxc_angular_compiler/src/util/mod.rs | 6 + .../tests/integration_test.rs | 179 +++++++++ .../e2e/compare/fixtures/known-differences.ts | 22 ++ .../providers/object-members.fixture.ts | 352 ++++++++++++++++++ 14 files changed, 724 insertions(+), 31 deletions(-) create mode 100644 napi/angular-compiler/e2e/compare/fixtures/providers/object-members.fixture.ts 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'], + }, +]