Skip to content

Commit df18eb1

Browse files
Brooooooklynclaude
andcommitted
fix(directive): read member @Input/@output arguments through the evaluator, like ngtsc
A same-file value passed to a member decorator lost its alias with no error: `const NAME = 'y'` with `@Input(NAME)` compiled to `x: "x"` (ngtsc: `x: [0, "y", "x"]`), `@Input(OPTS)` with `const OPTS = {alias: 'y', required: true}` dropped the alias and `required`, and `@Output(NAME)` compiled to `e: "e"` (ngtsc: `e: "y"`). Only literals were read. Like ngtsc's `tryParseInputFieldMapping` / `tryParseDecoratorOutput`, the argument is now evaluated (checked against @angular/compiler-cli 22.1.7): - `@Input`: a string (a const, `let`, template literal, `'a' + 'b'`, `as const`, a ternary, a same-file function call, ...) is the alias; an object (a const, a spread, `{alias: NAME}`) gives `alias`, `required` and `transform`. This holds for the `ɵdir`/`ɵcmp` inputs, the `.d.ts` and NAPI's `extractComponentMetadataSync`, which now also gets the transform ngtsc emits. - `@Output`: a string is the alias. - ngtsc's errors: "@input can have at most one argument, got N argument(s)" (named as written: `@In` for `import {Input as In}`), "@input decorator argument must resolve to a string or an object literal" (numbers, booleans, `undefined`, arrays, enum members, `declare`d values; `null` is allowed), and the same two for `@Output` ("must resolve to a string", which also rejects `null` and objects). - An imported `@Output(NAME)` (`ns.NAME`, `` `${NAME}` ``, a local copy) gets the "imported from another module" error, like `@Input(OPTS)`. The public `extract_*` functions, which don't get the file's imports, still read literals only. Left as before: ngtsc's "Input/Output 'y' is bound to both ..." error, and `@Output()` on a setter (ngtsc compiles it; oxc doesn't read outputs from methods). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 4b26d7f commit df18eb1

7 files changed

Lines changed: 1592 additions & 84 deletions

File tree

‎crates/oxc_angular_compiler/src/component/decorator.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use super::transform::ImportMap;
2020
use crate::directive::{
2121
StringConsts, extract_host_bindings_in, extract_host_listeners_in, extract_input_metadata_in,
2222
extract_output_metadata_in, merge_by_class_property, parse_decorator_io,
23-
resolve_member_transforms,
2423
};
2524
use crate::output::oxc_converter::convert_oxc_expression;
2625

@@ -270,7 +269,6 @@ pub fn extract_component_metadata<'a>(
270269
extract_input_metadata_in(allocator, class, source_text, Some(consts)),
271270
|i| i.class_property_name.as_str(),
272271
);
273-
resolve_member_transforms(allocator, class, source_text, consts, &mut metadata.inputs);
274272
metadata.outputs = merge_by_class_property(
275273
io.outputs,
276274
extract_output_metadata_in(allocator, class, Some(consts)),

‎crates/oxc_angular_compiler/src/directive/decorator.rs‎

Lines changed: 99 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,6 @@ pub fn extract_directive_metadata<'a>(
228228
let fields = std::mem::replace(&mut metadata.outputs, Vec::new_in(&allocator));
229229
metadata.outputs = merge_by_class_property(io.outputs, fields, |o| o.0.as_str());
230230
}
231-
resolve_member_transforms(allocator, class, source_text, consts, &mut metadata.inputs);
232231

233232
// Merge host metadata from decorator into the existing host metadata
234233
if let Some(decorator_host) = host_from_decorator {
@@ -952,37 +951,6 @@ pub(crate) fn transform_expression<'a>(
952951
}
953952
}
954953

955-
/// Give `@Input({ transform })` members the transform expression ngtsc emits
956-
/// (see [`transform_expression`]), in place of the one written.
957-
pub(crate) fn resolve_member_transforms<'a>(
958-
allocator: &'a Allocator,
959-
class: &'a Class<'a>,
960-
source_text: Option<&'a str>,
961-
consts: &StringConsts<'a>,
962-
inputs: &mut [R3InputMetadata<'a>],
963-
) {
964-
let evaluator = Evaluator::new(consts);
965-
for element in &class.body.body {
966-
let (key, decorators) = match element {
967-
ClassElement::PropertyDefinition(p) => (&p.key, &p.decorators),
968-
ClassElement::AccessorProperty(p) => (&p.key, &p.decorators),
969-
ClassElement::MethodDefinition(m) => (&m.key, &m.decorators),
970-
_ => continue,
971-
};
972-
let Some(name) = key.static_name() else { continue };
973-
let options = super::property_decorators::input_decorator_options(decorators, consts);
974-
let Some(options) = options else { continue };
975-
let options = evaluator.evaluate(options);
976-
let Some(transform) = options.prop("transform") else { continue };
977-
let Some(input) = inputs.iter_mut().find(|i| i.class_property_name == name.as_ref()) else {
978-
continue;
979-
};
980-
if let Some(expr) = transform_expression(allocator, transform, source_text, consts) {
981-
input.transform_function = Some(expr);
982-
}
983-
}
984-
}
985-
986954
/// ngtsc's `{...fromMeta, ...fromFields}` keyed by class property name: a member
987955
/// declaration replaces the metadata entry in place, new members are appended.
988956
/// The result is ordered like the keys of a JavaScript object: integer-like
@@ -1083,33 +1051,13 @@ pub fn decorator_io_errors<'a>(
10831051
_ => return None,
10841052
};
10851053
let name = key.static_name()?;
1086-
// `@Input({ transform })`
1087-
let options = super::property_decorators::input_decorator_options(decorators, consts);
1088-
if let Some(options) = options {
1089-
let span = options.span();
1090-
let options = evaluator.evaluate(options);
1091-
// ngtsc reads imported options (or an imported alias or
1092-
// `required`) from their file; oxc can't, and compiling the
1093-
// input without them would be a different binding.
1094-
let imported = std::iter::once(&options)
1095-
.chain(
1096-
["alias", "required"].iter().filter_map(|k| Some(&options.prop(k)?.value)),
1097-
)
1098-
.find(|value| value.is_import());
1099-
if let Some(value) = imported {
1100-
return Some((value_error("@Input", String::new, value), span));
1101-
}
1102-
if let Some(transform) = options.prop("transform") {
1103-
let error =
1104-
transform_error(transform, None, &name, class, consts.scope(), span)
1105-
.or_else(|| {
1106-
is_out_of_scope(transform, consts)
1107-
.then(|| (scoped_transform_error("@Input", &name), span))
1108-
});
1109-
if error.is_some() {
1110-
return error;
1111-
}
1112-
}
1054+
// `@Input(...)`, as ngtsc's `tryParseInputFieldMapping` reads it.
1055+
let decorator =
1056+
super::property_decorators::member_decorator(decorators, "Input", consts);
1057+
let error =
1058+
decorator.and_then(|d| input_decorator_error(d, &name, class, consts, &evaluator));
1059+
if error.is_some() {
1060+
return error;
11131061
}
11141062
// A signal input only collides with a metadata entry of the same name.
11151063
let value = value.filter(|_| meta_inputs.contains(&name.as_ref()))?;
@@ -1124,6 +1072,19 @@ pub fn decorator_io_errors<'a>(
11241072
};
11251073
let output_members = || {
11261074
class.body.body.iter().find_map(|element| {
1075+
// `@Output(...)`, as ngtsc's `tryParseDecoratorOutput` reads it, on
1076+
// the members an output is compiled from.
1077+
let decorators = match element {
1078+
ClassElement::PropertyDefinition(p) => Some(&p.decorators),
1079+
ClassElement::AccessorProperty(p) => Some(&p.decorators),
1080+
_ => None,
1081+
};
1082+
let decorator = decorators.and_then(|decorators| {
1083+
super::property_decorators::member_decorator(decorators, "Output", consts)
1084+
});
1085+
if let Some(error) = decorator.and_then(|d| output_decorator_error(d, &evaluator)) {
1086+
return Some(error);
1087+
}
11271088
let ClassElement::PropertyDefinition(prop) = element else { return None };
11281089
let (value, name) = (prop.value.as_ref()?, prop.key.static_name()?);
11291090
if !meta_outputs.contains(&name.as_ref()) {
@@ -1171,6 +1132,85 @@ pub fn decorator_io_errors<'a>(
11711132
.collect()
11721133
}
11731134

1135+
/// ngtsc's error for the `@Input(...)` decorator of the member `name`
1136+
/// (`tryParseInputFieldMapping`): more than one argument, an argument that
1137+
/// isn't `null`, a string or an object, or a `transform` it can't use. Options
1138+
/// from another module are reported as such (see [`value_error`]).
1139+
fn input_decorator_error<'a>(
1140+
decorator: &'a Decorator<'a>,
1141+
name: &str,
1142+
class: &'a Class<'a>,
1143+
consts: &StringConsts<'a>,
1144+
evaluator: &Evaluator<'_, 'a>,
1145+
) -> Option<(String, Span)> {
1146+
// ngtsc names it as it's written: `@In` for `import { Input as In }`.
1147+
let subject = format!("@{}", super::property_decorators::decorator_written_name(decorator));
1148+
if let Some(error) = decorator_arity_error(decorator, &subject) {
1149+
return Some(error);
1150+
}
1151+
let options = super::property_decorators::decorator_argument(decorator)?;
1152+
let span = options.span();
1153+
let options = evaluator.evaluate(options);
1154+
// ngtsc reads imported options (or an imported alias or `required`) from
1155+
// their file; oxc can't, and compiling the input without them would be a
1156+
// different binding.
1157+
let imported = std::iter::once(&options)
1158+
.chain(["alias", "required"].iter().filter_map(|k| Some(&options.prop(k)?.value)))
1159+
.find(|value| value.is_import());
1160+
if let Some(value) = imported {
1161+
return Some((value_error(&subject, String::new, value), span));
1162+
}
1163+
if !matches!(options, Value::Null | Value::String(_) | Value::Object(_)) {
1164+
let message = format!(
1165+
"{subject} decorator argument must resolve to a string or an object literal{}",
1166+
options.wrong_type_suffix()
1167+
);
1168+
return Some((message, decorator.span));
1169+
}
1170+
let transform = options.prop("transform")?;
1171+
transform_error(transform, None, name, class, consts.scope(), span).or_else(|| {
1172+
is_out_of_scope(transform, consts).then(|| (scoped_transform_error("@Input", name), span))
1173+
})
1174+
}
1175+
1176+
/// ngtsc's error for a member decorator called with more than one argument,
1177+
/// named `subject` (`@Input`, or `@In` for `import { Input as In }`).
1178+
fn decorator_arity_error(decorator: &Decorator<'_>, subject: &str) -> Option<(String, Span)> {
1179+
let Expression::CallExpression(call) = &decorator.expression else { return None };
1180+
let count = call.arguments.len();
1181+
(count > 1).then(|| {
1182+
let message = format!("{subject} can have at most one argument, got {count} argument(s)");
1183+
(message, decorator.span)
1184+
})
1185+
}
1186+
1187+
/// ngtsc's error for an `@Output(...)` decorator (`tryParseDecoratorOutput`):
1188+
/// more than one argument, or an argument that isn't a string. An argument
1189+
/// from another module is reported as such (see [`value_error`]).
1190+
fn output_decorator_error<'a>(
1191+
decorator: &'a Decorator<'a>,
1192+
evaluator: &Evaluator<'_, 'a>,
1193+
) -> Option<(String, Span)> {
1194+
// ngtsc names it `@Output` whatever it's imported as.
1195+
if let Some(error) = decorator_arity_error(decorator, "@Output") {
1196+
return Some(error);
1197+
}
1198+
let argument = super::property_decorators::decorator_argument(decorator)?;
1199+
match evaluator.evaluate(argument) {
1200+
Value::String(_) => None,
1201+
value if value.is_import() => {
1202+
Some((value_error("@Output", String::new, &value), argument.span()))
1203+
}
1204+
value => {
1205+
let message = format!(
1206+
"@Output decorator argument must resolve to a string{}",
1207+
value.wrong_type_suffix()
1208+
);
1209+
Some((message, decorator.span))
1210+
}
1211+
}
1212+
}
1213+
11741214
/// An initializer API: its function name and the module exporting it.
11751215
pub(crate) type InitializerApi = (&'static str, &'static str);
11761216
pub(crate) const INPUT_API: InitializerApi = ("input", "@angular/core");

‎crates/oxc_angular_compiler/src/directive/mod.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ pub use decorator::{
3838
pub(crate) use decorator::{
3939
angular_decorator_config, extract_string_value, resolve_template_literal,
4040
};
41-
pub(crate) use decorator::{
42-
merge_by_class_property, parse_decorator_io, resolve_member_transforms,
43-
};
41+
pub(crate) use decorator::{merge_by_class_property, parse_decorator_io};
4442
pub use definition::{DirectiveDefinitions, generate_directive_definitions};
4543
pub(crate) use dts_type::quote as ts_string_literal;
4644
pub use evaluator::input_transform_types;

0 commit comments

Comments
 (0)