diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index 2a617d1ea..cf8f07d60 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -18,8 +18,9 @@ use super::metadata::{ }; use super::transform::ImportMap; use crate::directive::{ - HostKey, MemberHostBindings, StringConsts, evaluate_host_directives, extract_input_metadata_in, - extract_output_metadata_in, merge_by_class_property, merge_host_bindings, parse_decorator_io, + HostKey, HostValue, MemberHostBindings, StringConsts, evaluate_host_directives, + extract_input_metadata_in, extract_output_metadata_in, merge_by_class_property, + merge_host_bindings, parse_decorator_io, }; use crate::output::oxc_converter::convert_oxc_expression; use crate::util::is_metadata_property; @@ -170,7 +171,8 @@ pub fn extract_component_metadata<'a>( // an invalid one must not be applied here where it would // be dropped silently. if !prop.computed { - metadata.host = extract_host_metadata(allocator, &prop.value, consts); + metadata.host = + extract_host_metadata(allocator, &prop.value, source_text, consts); } } "imports" => { @@ -544,6 +546,7 @@ fn extract_change_detection(expr: &Expression<'_>) -> ChangeDetectionStrategy { fn extract_host_metadata<'a>( allocator: &'a Allocator, expr: &'a Expression<'a>, + source_text: Option<&'a str>, consts: &StringConsts<'a>, ) -> Option> { // An unusable host is reported by `decorator_io_errors`. @@ -552,12 +555,19 @@ fn extract_host_metadata<'a>( let mut host = HostMetadata::new(allocator); for (key, value) in entries { - match HostKey::of(key.as_str()) { + let kind = HostKey::of(key.as_str()); + if matches!(kind, HostKey::Attribute) { + host.attributes.push((key, value.to_output(allocator, source_text)?)); + continue; + } + // Only a plain attribute has an expression value. + let HostValue::String(value) = value else { continue }; + match kind { HostKey::Property => host.properties.push((key, value)), HostKey::Listener => host.listeners.push((key, value)), HostKey::Class => host.class_attr = Some(value), HostKey::Style => host.style_attr = Some(value), - HostKey::Attribute => host.attributes.push((key, value)), + HostKey::Attribute => {} } } Some(host) diff --git a/crates/oxc_angular_compiler/src/component/metadata.rs b/crates/oxc_angular_compiler/src/component/metadata.rs index b97bc38d8..5e1e4e842 100644 --- a/crates/oxc_angular_compiler/src/component/metadata.rs +++ b/crates/oxc_angular_compiler/src/component/metadata.rs @@ -795,8 +795,10 @@ pub struct HostMetadata<'a> { /// Host property bindings: `{ '[class.active]': 'isActive' }` pub properties: Vec<'a, (Ident<'a>, Ident<'a>)>, - /// Host attribute bindings: `{ 'role': 'button' }` - pub attributes: Vec<'a, (Ident<'a>, Ident<'a>)>, + /// Static host attributes: `{ 'role': 'button' }`, as the expressions + /// `hostAttrs` holds (a string literal, or the expression ngtsc emits for + /// a value it can't evaluate). + pub attributes: Vec<'a, (Ident<'a>, OutputExpression<'a>)>, /// Host event listeners: `{ '(click)': 'onClick()' }` pub listeners: Vec<'a, (Ident<'a>, Ident<'a>)>, diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 9bca18f5e..d0c303a00 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -6712,9 +6712,9 @@ fn convert_host_metadata_to_input<'a>( }); } - // Convert static attributes: "role" -> OutputExpression::Literal - // This matches TypeScript which uses `o.literal(value)` for host attributes. - // They are in the order `merge_host_bindings` put them in. + // Static attributes are already the expressions `hostAttrs` holds, in the + // order `merge_host_bindings` put them in. The static style and class + // become string literals, like `o.literal(value)` upstream. let literal = |value: &Ident<'a>| { crate::output::ast::OutputExpression::Literal(oxc_allocator::Box::new_in( crate::output::ast::LiteralExpr { @@ -6726,7 +6726,7 @@ fn convert_host_metadata_to_input<'a>( }; let mut attributes = crate::pipeline::ingest::HostAttributes::default(); for (key, value) in &host.attributes { - attributes.insert(*key, literal(value)); + attributes.insert(*key, value.clone_in(allocator)); } // Like Angular's `createHostBindingsFunction`, a non-empty static style, @@ -6848,10 +6848,15 @@ fn convert_host_metadata_input_to_host_metadata<'a>( .push((Ident::from_in(k.as_str(), &allocator), Ident::from_in(v.as_str(), &allocator))); } - let mut attributes: OxcVec<'a, (Ident<'a>, Ident<'a>)> = OxcVec::new_in(&allocator); + let mut attributes: OxcVec<'a, (Ident<'a>, OutputExpression<'a>)> = OxcVec::new_in(&allocator); for (k, v) in &input.attributes { - attributes - .push((Ident::from_in(k.as_str(), &allocator), Ident::from_in(v.as_str(), &allocator))); + attributes.push(( + Ident::from_in(k.as_str(), &allocator), + crate::output::ast::OutputAstBuilder::string( + allocator, + Ident::from_in(v.as_str(), &allocator), + ), + )); } let mut listeners: OxcVec<'a, (Ident<'a>, Ident<'a>)> = OxcVec::new_in(&allocator); diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 72502a9a0..ef842c984 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -23,7 +23,7 @@ use super::metadata::{ use super::property_decorators::try_unwrap_forward_ref; use crate::factory::R3DependencyMetadata; use crate::output::ast::{OutputAstBuilder, OutputExpression, ReadVarExpr}; -use crate::output::oxc_converter::convert_oxc_expression; +use crate::output::oxc_converter::{convert_oxc_expression, make_raw_source}; use crate::r3::CORE; use crate::util::is_metadata_property; use crate::util::lone_surrogate::has_lone_surrogate; @@ -182,7 +182,7 @@ pub fn extract_directive_metadata<'a>( // where it would be dropped silently. if !prop.computed { host_from_decorator = - extract_host_metadata(allocator, &prop.value, consts); + extract_host_metadata(allocator, &prop.value, source_text, consts); } } "hostDirectives" => { @@ -504,7 +504,7 @@ pub struct StringConsts<'a> { /// diagnostic pass (`decorator_io_errors`) and the metadata extraction /// both evaluate it, and evaluation is deterministic. host_values: std::cell::RefCell< - HashMap<(usize, String), Result, Ident<'a>)>, String>>, + HashMap<(usize, String), Result, HostValue<'a>)>, String>>, >, } @@ -1955,6 +1955,31 @@ impl HostKey { } } +/// A `host` metadata value: the string it evaluates to, or, for a plain +/// attribute whose value ngtsc can't evaluate (its `DynamicValue`), the +/// expression ngtsc emits in its place (`WrappedNodeExpr`), as it can be +/// written where the metadata is compiled. +#[derive(Debug, Clone, Copy)] +pub(crate) enum HostValue<'a> { + String(Ident<'a>), + Expression(&'a Expression<'a>), +} + +impl<'a> HostValue<'a> { + /// The attribute value in `hostAttrs`: a string literal or the expression. + pub(crate) fn to_output( + self, + allocator: &'a Allocator, + source_text: Option<&'a str>, + ) -> Option> { + match self { + Self::String(value) => Some(OutputAstBuilder::string(allocator, value)), + Self::Expression(expr) => convert_oxc_expression(allocator, expr, source_text) + .or_else(|| make_raw_source(allocator, source_text, expr.span())), + } + } +} + /// The entries of the `host` metadata `expr` evaluates to, in order, or the error ngtsc /// reports for it (on `expr`). /// @@ -1972,7 +1997,7 @@ pub(crate) fn evaluate_host_metadata<'a>( expr: &'a Expression<'a>, decorator_name: &str, consts: &StringConsts<'a>, -) -> Result, Ident<'a>)>, String> { +) -> Result, HostValue<'a>)>, String> { // This runs twice per decorated class — the diagnostic checks it and the // metadata extraction reads the entries — so each `host:` expression is // evaluated once and the result shared (`StringConsts::host_values`). @@ -1990,7 +2015,7 @@ fn evaluate_host_metadata_uncached<'a>( expr: &'a Expression<'a>, decorator_name: &str, consts: &StringConsts<'a>, -) -> Result, Ident<'a>)>, String> { +) -> Result, HostValue<'a>)>, String> { let subject = format!("@{decorator_name}.host"); let host = Evaluator::new(consts).evaluate(expr); let Value::Object(props) = &host else { @@ -2002,7 +2027,7 @@ fn evaluate_host_metadata_uncached<'a>( }; let alloc = |s: &str| Ident::from(allocator.alloc_str(s)); - let mut entries = std::vec::Vec::<(Ident<'a>, Ident<'a>)>::with_capacity(props.len()); + let mut entries = std::vec::Vec::<(Ident<'a>, HostValue<'a>)>::with_capacity(props.len()); // ngtsc reports unparseable values while building the host map, binding // kinds in `parseHostBindings` afterwards, and `verifyHostBindings` last: // the first error of an earlier step wins over every later one. @@ -2018,29 +2043,45 @@ fn evaluate_host_metadata_uncached<'a>( continue; } // The value the key ends up with: its last one. - let value = host.prop(&prop.key).map_or(&prop.value, |p| &p.value); - let value = match value { + let last = host.prop(&prop.key).unwrap_or(prop); + let value = match &last.value { Value::Enum { value, .. } => value, value => value, }; + // ngtsc's `DynamicValue`. One file at a time, so is a value computed + // from an import: ngtsc's local compilation mode can't read the other + // file either, and gets a `DynamicValue` for it. + let dynamic = + value.is_dynamic() || value.is_import() || matches!(value, Value::Function(_)); match value { - Value::String(s) => entries.push((alloc(&prop.key), alloc(s))), - // ngtsc's `DynamicValue`. `parseHostBindings` rejects it for everything but - // a plain attribute, whose expression it emits as it is written. - // Upstream throws these as bare `Error`s (a compiler crash, no - // node); here they surface as diagnostics on the host expression - // like the rest of `decorator_io_errors`. - _ if value.is_dynamic() || matches!(value, Value::Function(_)) => { + Value::String(s) => entries.push((alloc(&prop.key), HostValue::String(alloc(s)))), + // A plain attribute's dynamic value is emitted as it is written + // (`hostAttrs: ["role", make()]`). + _ if dynamic + && matches!(HostKey::of(&prop.key), HostKey::Attribute) + && let Some(origin) = last.origin => + { + entries.push((alloc(&prop.key), HostValue::Expression(origin))); + } + // `parseHostBindings` rejects a dynamic value for everything but a + // plain attribute. Upstream throws these as bare `Error`s (a + // compiler crash, no node); here they surface as diagnostics on + // the host expression like the rest of `decorator_io_errors`. + _ if dynamic && !value.is_import() => { if parse_error.is_none() { parse_error = Some(match HostKey::of(&prop.key) { HostKey::Property => "Property binding must be string".to_string(), HostKey::Listener => "Event binding must be string".to_string(), HostKey::Class => "Class binding must be string".to_string(), HostKey::Style => "Style binding must be string".to_string(), + // No expression to emit: it uses the parameters of + // a function the metadata calls, or is written in + // another file. HostKey::Attribute => format!( "Decorator host metadata must be a string -> string object, but the \ - value of '{}' could not be determined statically. OXC does not \ - support dynamic host attribute values.", + value of '{}' could not be determined statically, and OXC can't \ + emit the expression it is written as where the metadata is \ + compiled.", prop.key ), }); @@ -2110,6 +2151,7 @@ fn host_metadata_error<'a>( fn extract_host_metadata<'a>( allocator: &'a Allocator, expr: &'a Expression<'a>, + source_text: Option<&'a str>, consts: &StringConsts<'a>, ) -> Option> { // An unusable host is reported by `decorator_io_errors`. @@ -2117,14 +2159,19 @@ fn extract_host_metadata<'a>( let mut host = R3HostMetadata::new(allocator); for (key, value) in entries { - match HostKey::of(key.as_str()) { + let kind = HostKey::of(key.as_str()); + if matches!(kind, HostKey::Attribute) { + host.attributes.push((key, value.to_output(allocator, source_text)?)); + continue; + } + // Only a plain attribute has an expression value. + let HostValue::String(value) = value else { continue }; + match kind { HostKey::Property => host.properties.push((key, value)), HostKey::Listener => host.listeners.push((key, value)), HostKey::Class => host.class_attr = Some(value), HostKey::Style => host.style_attr = Some(value), - HostKey::Attribute => { - host.attributes.push((key, OutputAstBuilder::string(allocator, value))); - } + HostKey::Attribute => {} } } Some(host) diff --git a/crates/oxc_angular_compiler/src/directive/mod.rs b/crates/oxc_angular_compiler/src/directive/mod.rs index 7cdb5466e..bc76aba03 100644 --- a/crates/oxc_angular_compiler/src/directive/mod.rs +++ b/crates/oxc_angular_compiler/src/directive/mod.rs @@ -29,7 +29,7 @@ pub use compiler::{ pub(crate) use decorator::evaluate_host_directives; pub(crate) use decorator::find_directive_decorator; pub(crate) use decorator::{ - HostKey, angular_decorator_config, evaluate_host_metadata, evaluate_string_value, + HostKey, HostValue, angular_decorator_config, evaluate_host_metadata, evaluate_string_value, evaluate_style_urls, evaluate_styles, extract_string_value, styles_error, }; pub(crate) use decorator::{ diff --git a/crates/oxc_angular_compiler/src/partial/component.rs b/crates/oxc_angular_compiler/src/partial/component.rs index a703e069d..4ceeaeda2 100644 --- a/crates/oxc_angular_compiler/src/partial/component.rs +++ b/crates/oxc_angular_compiler/src/partial/component.rs @@ -558,9 +558,17 @@ fn compile_host_metadata<'a>( let mut entries: Vec<'a, LiteralMapEntry<'a>> = Vec::new_in(&allocator); if !host.attributes.is_empty() { + let mut attr_entries: Vec<'a, LiteralMapEntry<'a>> = Vec::new_in(&allocator); + for (key, value) in &host.attributes { + let quoted = is_unsafe_object_key(key.as_str()); + attr_entries.push(LiteralMapEntry::new(key.clone(), value.clone_in(allocator), quoted)); + } entries.push(LiteralMapEntry::new( Ident::from("attributes"), - ident_pairs_to_string_map(allocator, &host.attributes), + OutputExpression::LiteralMap(Box::new_in( + LiteralMapExpr { entries: attr_entries, source_span: None }, + &allocator, + )), false, )); } diff --git a/crates/oxc_angular_compiler/tests/cross_file_values_test.rs b/crates/oxc_angular_compiler/tests/cross_file_values_test.rs index f0ead7895..cc880811e 100644 --- a/crates/oxc_angular_compiler/tests/cross_file_values_test.rs +++ b/crates/oxc_angular_compiler/tests/cross_file_values_test.rs @@ -399,8 +399,9 @@ export class A { go() {} } } } - // An export that can't be read statically reports the entry, upstream - // would emit its expression for a plain attribute (a declared divergence). + // An export that can't be read statically reports the entry: upstream + // emits its expression for a plain attribute, which is written in the + // other file and can't be emitted in this one (a declared divergence). create_test_file( dir.path(), "app/bad.ts", @@ -444,3 +445,25 @@ export class X {} define_call(&result.code, "X") ); } + +/// A host attribute computed from an import ngtsc can't evaluate either is +/// emitted as written (ngtsc_spec.ts: "should accept dynamic host attribute +/// bindings"). +#[test] +fn dynamic_host_attribute_from_an_unreadable_import() { + let dir = TempDir::new().unwrap(); + create_test_file(dir.path(), "app/other.d.ts", "export declare const foo: any;"); + let result = transform( + &dir, + r#"import { Component } from '@angular/core'; +import { foo } from './other'; +const test = foo.bar(); +@Component({ selector: 'test', template: '', host: { 'test': test } }) +export class TestCmp {} +"#, + &resolve_options(), + ); + assert!(!result.has_errors(), "{}", error_messages(&result)); + let definition = define_call(&result.code, "TestCmp"); + assert!(definition.contains(r#"hostAttrs:["test",test]"#), "{definition}"); +} diff --git a/crates/oxc_angular_compiler/tests/host_metadata_evaluation_test.rs b/crates/oxc_angular_compiler/tests/host_metadata_evaluation_test.rs index 07bb4d59c..f0ad05821 100644 --- a/crates/oxc_angular_compiler/tests/host_metadata_evaluation_test.rs +++ b/crates/oxc_angular_compiler/tests/host_metadata_evaluation_test.rs @@ -373,28 +373,71 @@ fn dynamic_value_of_a_binding() { assert_error(make, "{ style: make() }", "Style binding must be string"); } -/// ngtsc emits a plain attribute's dynamic value as an expression in `hostAttrs` -/// (`["role", make()]`). That is not supported here, and is reported instead of dropped. +/// ngtsc emits a plain attribute's dynamic value as it is written, in `hostAttrs` +/// (`["role", make()]`). That includes a value computed from an import, which ngtsc's +/// local compilation mode can't read either (ngtsc_spec.ts: "should accept dynamic host +/// attribute bindings"). #[test] fn dynamic_value_of_a_plain_attribute() { - let message = |key: &str| { - format!( - "Decorator host metadata must be a string -> string object, but the value of \ - '{key}' could not be determined statically. OXC does not support dynamic \ - host attribute values." - ) - }; - assert_error("declare function make(): string;", "{ role: make() }", &message("role")); - assert_error("", "{ role: NOPE }", &message("role")); + let make = "declare function make(): string;"; + for decorator in ["Component", "Directive"] { + for (preamble, host, attrs) in [ + (make, "{ role: make() }", r#"hostAttrs:["role",make()]"#), + ("", "{ role: NOPE }", r#"hostAttrs:["role",NOPE]"#), + ( + &format!("{make} const S = {{ title: make() }};"), + "{ ...S, id: 'a' }", + r#"hostAttrs:["title",make(),"id","a"]"#, + ), + ( + "import { SHARED } from './other';", + "{ role: SHARED }", + r#"hostAttrs:["role",SHARED]"#, + ), + ( + "import { foo } from './other';\nconst test = foo.bar();", + "{ 'test': test }", + r#"hostAttrs:["test",test]"#, + ), + ] { + let definition = host_definition(decorator, preamble, host); + assert!(definition.contains(attrs), "for `host: {host}`: {definition}"); + } + } +} + +/// A partial declaration carries the expression in `host.attributes`. +#[test] +fn dynamic_value_of_a_plain_attribute_partial() { + for decorator in ["Component", "Directive"] { + let source = source(decorator, "declare function make(): string;", "{ role: make() }"); + let allocator = Allocator::default(); + let options = TransformOptions { + compilation_mode: oxc_angular_compiler::CompilationMode::Partial, + ..Default::default() + }; + let result = transform_angular_file(&allocator, "/x/a.ts", &source, Some(&options), None); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!(compact.contains("host:{attributes:{role:make()}}"), "{compact}"); + } +} + +/// A dynamic attribute value with no expression to emit where the metadata is +/// compiled: one that uses the parameters of a function the metadata calls. +#[test] +fn dynamic_value_of_a_plain_attribute_out_of_scope() { assert_error( - "declare function make(): string; const S = { title: make() };", - "{ ...S, id: 'a' }", - &message("title"), + "declare function make(): string;\nfunction h(v: string) { return { role: v + make() }; }", + "h('a')", + "Decorator host metadata must be a string -> string object, but the value of 'role' \ + could not be determined statically, and OXC can't emit the expression it is written \ + as where the metadata is compiled.", ); } /// ngtsc reads another file to evaluate an import. One file at a time, that is an error -/// that says so. +/// that says so, except for a plain attribute (see above). #[test] fn host_that_depends_on_an_import() { let imported = |subject: &str| { @@ -406,9 +449,7 @@ fn host_that_depends_on_an_import() { let preamble = "import { SHARED } from './other';"; for (decorator, subject) in [("Component", "@Component.host"), ("Directive", "@Directive.host")] { - for host in - ["SHARED", "{ ...SHARED, role: 'x' }", "{ role: SHARED }", "{ '(click)': SHARED }"] - { + for host in ["SHARED", "{ ...SHARED, role: 'x' }", "{ '(click)': SHARED }"] { let source = source(decorator, preamble, host); let (_, diagnostics) = compile(&source); let [diagnostic] = diagnostics.as_slice() else {