diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/chaining.rs b/crates/oxc_angular_compiler/src/pipeline/phases/chaining.rs index 4cf686557..cd4b8f09b 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/chaining.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/chaining.rs @@ -568,7 +568,7 @@ fn clone_expression<'a>( allocator, ), name: prop.name.clone(), - optional: false, + optional: prop.optional, source_span: prop.source_span, }, allocator, @@ -580,7 +580,7 @@ fn clone_expression<'a>( allocator, ), index: Box::new_in(clone_expression(allocator, &key.index, diagnostics), allocator), - optional: false, + optional: key.optional, source_span: key.source_span, }, allocator, @@ -721,7 +721,7 @@ fn clone_expression<'a>( ), args: clone_args(allocator, &invoke.args, diagnostics), pure: invoke.pure, - optional: false, + optional: invoke.optional, source_span: invoke.source_span, }, allocator, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/reify/ir_expression.rs b/crates/oxc_angular_compiler/src/pipeline/phases/reify/ir_expression.rs index 17e32fcb2..3eefc5a05 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/reify/ir_expression.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/reify/ir_expression.rs @@ -1100,6 +1100,10 @@ fn clone_output_expression<'a>( ReadPropExpr { receiver: Box::new_in(clone_output_expression(allocator, &rp.receiver), allocator), name: rp.name.clone(), + // Intentionally non-optional: `clone_output_expression` is only used to + // build the write-back assignment `target = value` for two-way bindings + // (see `create_assignment`), and an optional chain (`a?.b`) is not a legal + // assignment target — emitting `a?.b = $event` would be a JS SyntaxError. optional: false, source_span: rp.source_span, }, @@ -1109,6 +1113,8 @@ fn clone_output_expression<'a>( ReadKeyExpr { receiver: Box::new_in(clone_output_expression(allocator, &rk.receiver), allocator), index: Box::new_in(clone_output_expression(allocator, &rk.index), allocator), + // Non-optional for the same reason as the `ReadProp` arm above: this + // clone only ever forms a two-way write-back assignment target. optional: false, source_span: rk.source_span, }, diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 1578824ad..8858f6b68 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -2162,6 +2162,166 @@ fn test_safe_navigation_migration_forces_legacy_on_v22() { ); } +#[test] +fn test_safe_navigation_preserved_in_chained_property_bindings_v22() { + // CX-47617: when an element has several property bindings, the reify phase + // chains them into a single `ɵɵproperty(a)(b)` call. The chaining phase + // clones the arguments of every binding after the first, and that clone used + // to hard-code `optional: false`, silently dropping the `?.` guard on all but + // the first binding. Under Angular v22 (native optional chaining) this emitted + // e.g. `ctx.c.d` instead of `ctx.c?.d`, crashing at runtime when `c` is nullish. + let js = compile_template_to_js_with_version( + r#"
"#, + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.a?.b"), "first binding must keep `?.`, got:\n{js}"); + assert!(js.contains("ctx.c?.d"), "second (chained) binding must keep `?.`, got:\n{js}"); + assert!(js.contains("ctx.e?.f"), "third (chained) binding must keep `?.`, got:\n{js}"); +} + +#[test] +fn test_safe_navigation_preserved_in_chained_keyed_and_call_bindings_v22() { + // Same chaining-clone bug for SafeKeyedRead (`?.[k]`) and SafeCall (`?.()`) + // in later bindings of a chain. + let js = compile_template_to_js_with_version( + r#"
"#, + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.c?.[ctx.k]"), "chained keyed read must keep `?.[`, got:\n{js}"); + assert!(js.contains("ctx.fn?.()"), "chained safe call must keep `?.()`, got:\n{js}"); +} + +#[test] +fn test_safe_navigation_preserved_in_chained_attr_class_style_v22() { + // The chaining-clone bug affected every chainable binding instruction, not + // just ɵɵproperty. Verify attribute, class and style bindings keep `?.` on + // their chained (non-first) argument. + for (tpl, instr) in [ + (r#"
"#, "attribute"), + (r#"
"#, "class"), + (r#"
"#, "style"), + ] { + let js = compile_template_to_js_with_version( + tpl, + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.w?.x"), "{instr}: first binding must keep `?.`, got:\n{js}"); + assert!(js.contains("ctx.y?.z"), "{instr}: chained binding must keep `?.`, got:\n{js}"); + } +} + +#[test] +fn test_safe_navigation_preserved_when_nested_in_chained_binding_v22() { + // `?.` buried inside a larger expression (under `!`, in a binary, chained, + // or in a ternary branch) on a chained binding must survive the recursive + // clone. + let js = compile_template_to_js_with_version( + r#"
"#, + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("!ctx.c?.d"), "`?.` under `!` on chained binding must survive, got:\n{js}"); + assert!(js.contains("ctx.e?.f?.g"), "chained `?.` on chained binding must survive, got:\n{js}"); + assert!( + js.contains("ctx.i?.j"), + "`?.` in ternary branch on chained binding must survive, got:\n{js}" + ); + assert!( + js.contains("ctx.k?.l"), + "`?.` in ternary branch on chained binding must survive, got:\n{js}" + ); +} + +#[test] +fn test_safe_navigation_preserved_in_chained_host_property_bindings_v22() { + // CX-47617: host property bindings go through the same `chain_for_host` -> + // `clone_expression` path as template bindings. Multiple host `[prop]` + // bindings chain into `ɵɵhostProperty(a)(b)`, and the chained (non-first) + // ones must keep their `?.` guard under Angular v22. + use oxc_angular_compiler::{HostMetadataInput, compile_template_to_js_with_options}; + + let allocator = Allocator::default(); + let options = ComponentTransformOptions { + angular_version: Some(AngularVersion::new(22, 0, 0)), + host: Some(HostMetadataInput { + properties: vec![ + ("[title]".to_string(), "a?.b".to_string()), + ("[id]".to_string(), "c?.d".to_string()), + ("[lang]".to_string(), "e?.f".to_string()), + ], + attributes: vec![], + listeners: vec![], + class_attr: None, + style_attr: None, + }), + selector: Some("my-comp".to_string()), + ..Default::default() + }; + + let output = compile_template_to_js_with_options( + &allocator, + "

hi

", + "MyComponent", + "test.ts", + &options, + ) + .expect("compilation should succeed"); + let code = &output.code; + + assert!(code.contains("ctx.a?.b"), "first host binding must keep `?.`, got:\n{code}"); + assert!( + code.contains("ctx.c?.d"), + "second (chained) host binding must keep `?.`, got:\n{code}" + ); + assert!(code.contains("ctx.e?.f"), "third (chained) host binding must keep `?.`, got:\n{code}"); +} + +#[test] +fn test_safe_navigation_preserved_in_chained_bindings_legacy_v21() { + // Older Angular (< v22) uses the legacy `== null ? null` expansion instead of + // native `?.`. The guard there is a structural ternary, not the `optional` + // flag, so the chaining-clone bug never affected legacy output. This test + // pins that: every chained binding — not just the first — must keep its own + // null-guard ternary in v21, and the fix must not perturb legacy codegen. + let js = compile_template_to_js_with_version( + r#"
"#, + "TestComponent", + Some(AngularVersion::new(21, 0, 0)), + ); + assert!(!js.contains("?."), "v21 must not emit native optional chaining, got:\n{js}"); + for (recv, member) in [("ctx.a", "ctx.a.b"), ("ctx.c", "ctx.c.d"), ("ctx.e", "ctx.e.f")] { + assert!( + js.contains(&format!("({recv} == null)? null: {member}")), + "each chained binding must keep its legacy null guard; missing for {member}, got:\n{js}" + ); + } +} + +#[test] +fn test_two_way_binding_writeback_target_is_never_optional_v22() { + // Guard against "helpfully" preserving `?.` when cloning a two-way binding's + // write-back assignment target. `a?.b = $event` is a JS SyntaxError (an + // optional chain is not a legal assignment target), so the LHS of the + // `|| (target = value)` clause must stay a plain member access even when the + // bound expression uses safe navigation under Angular v22. + let js = compile_template_to_js_with_version( + r#""#, + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + // The twoWayBindingSet argument keeps its `?.` (it is only read, never assigned)... + assert!(js.contains("ɵɵtwoWayBindingSet(ctx.a?.b"), "read side should keep `?.`, got:\n{js}"); + // ...but the write-back assignment target must be a plain, assignable read. + assert!(js.contains("ctx.a.b = $event"), "write-back LHS must be non-optional, got:\n{js}"); + assert!( + !js.contains("a?.b = $event"), + "must not emit an illegal optional-chain LHS, got:\n{js}" + ); +} + #[test] fn test_safe_navigation_migration_ignores_qualified_call() { // Only the *unqualified* `$safeNavigationMigration(...)` helper is magic. A