From 1269b230eea240d45593687209c997e65e6d4081 Mon Sep 17 00:00:00 2001 From: Brandon Roberts Date: Mon, 1 Jun 2026 11:54:35 -0500 Subject: [PATCH] fix(safe-nav): version-aware optional chaining for Angular v22+ (#317) Angular v22 changed the safe-navigation operator (`?.`) in template expressions to yield `undefined` via native optional chaining, gated by the `legacyOptionalChaining` compiler option. OXC unconditionally emitted the legacy `== null ? null` ternary, so v22+ projects got the wrong runtime value for any `?.` expression. Changes: - Add `legacyOptionalChaining` to `TransformOptions` (NAPI + Rust) and thread it through ingest into the compilation jobs. The effective default is derived from `angularVersion`: legacy for < v22, modern (native `?.`) for >= v22, and legacy when the version is unknown (matches Angular's conservative fallback). - Add an `optional` flag to the resolved IR read/call nodes (`ResolvedPropertyRead`/`ResolvedKeyedRead`/`ResolvedCall`) and pass it through reify so it renders as native `?.` / `?.[]` / `?.()`. - Rewrite `expand_safe_reads` to branch per node: legacy builds the `SafeTernary` (`== null ? null`); modern rewrites each safe access into the equivalent optional resolved read (no temporaries needed). - Support the `$safeNavigationMigration(...)` escape hatch: a wrapped subtree is forced back to legacy null semantics even on a modern target, and the wrapper is stripped. Two deviations from the issue text, both to match the reference compiler (angular/angular@2896c93cc1): - The modern form is native optional chaining (`ctx.user?.name`), not the `== null ? undefined` ternary the issue described. Both yield `undefined` at runtime; native `?.` matches Angular's emitted output. - The magic function shipped in v22 is `$safeNavigationMigration(...)`, not `$null(...)` (the commit message named `$null` but the code renamed it). Partial/linker output keeps legacy semantics for now; threading the facade field through partial emit is deferred (issue required-work #4). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/component/metadata.rs | 14 ++ .../src/component/transform.rs | 43 +++- .../oxc_angular_compiler/src/ir/expression.rs | 21 ++ .../src/pipeline/compilation.rs | 39 ++++ .../src/pipeline/conversion.rs | 1 + .../src/pipeline/ingest.rs | 20 +- .../src/pipeline/phases/expand_safe_reads.rs | 212 +++++++++++++++++- .../src/pipeline/phases/generate_variables.rs | 1 + .../pipeline/phases/reify/ir_expression.rs | 6 +- .../src/pipeline/phases/resolve_names.rs | 6 + .../pipeline/phases/track_fn_optimization.rs | 2 + .../src/pipeline/phases/track_variables.rs | 1 + .../pipeline/phases/variable_optimization.rs | 3 + .../tests/integration_test.rs | 77 +++++++ napi/angular-compiler/index.d.ts | 11 + napi/angular-compiler/src/lib.rs | 11 + 16 files changed, 449 insertions(+), 19 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/metadata.rs b/crates/oxc_angular_compiler/src/component/metadata.rs index e1177c90c..fdfe3d590 100644 --- a/crates/oxc_angular_compiler/src/component/metadata.rs +++ b/crates/oxc_angular_compiler/src/component/metadata.rs @@ -72,6 +72,20 @@ impl AngularVersion { self.major >= 22 } + /// Check if this version uses modern optional-chaining semantics (v22.0.0+). + /// + /// Angular v22 changed the safe-navigation operator (`?.`) in template + /// expressions to yield `undefined` (native optional chaining) instead of the + /// legacy `null`. Earlier versions default to the legacy `== null ? null` + /// expansion. Users can opt back into legacy behavior with the + /// `legacyOptionalChaining` compiler option, or per-expression by wrapping it + /// in the `$safeNavigationMigration(...)` magic function. + /// + /// See `angular/angular@2896c93cc1`. + pub fn supports_modern_optional_chaining(&self) -> bool { + self.major >= 22 + } + /// Check if this version's runtime supports chained query instructions /// (`ɵɵviewQuery(p1)(p2)`, `ɵɵcontentQuerySignal(...)(...)`). /// diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 111f2ce31..b76b2fdaa 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -110,6 +110,17 @@ pub struct TransformOptions { /// When `None`, assumes latest Angular version (v19+ behavior). pub angular_version: Option, + /// Override for the `legacyOptionalChaining` Angular compiler option. + /// + /// Controls how the safe-navigation operator (`?.`) in template expressions is + /// emitted. When `Some(true)`, always uses the legacy `== null ? null` ternary; + /// when `Some(false)`, always emits native optional chaining (yielding + /// `undefined`). When `None`, the default is derived from `angular_version` + /// (legacy for < v22, modern for >= v22, legacy when the version is unknown). + /// + /// See `angular/angular@2896c93cc1`. + pub legacy_optional_chaining: Option, + // Component metadata overrides for template-only compilation. // These allow the build tool to pass component metadata when compiling // templates in isolation (e.g., for testing or compare tool). @@ -227,8 +238,9 @@ impl Default for TransformOptions { jit: false, hmr: false, advanced_optimizations: false, - i18n_use_external_ids: true, // Angular's JIT default - angular_version: None, // None means assume latest (v19+ behavior) + i18n_use_external_ids: true, // Angular's JIT default + angular_version: None, // None means assume latest (v19+ behavior) + legacy_optional_chaining: None, // None: derive default from angular_version // Metadata overrides default to None (use extracted/default values) selector: None, standalone: None, @@ -3764,6 +3776,7 @@ fn compile_component_full<'a>( pool_starting_index, // Pass Angular version for feature-gated instruction selection angular_version: options.angular_version, + legacy_optional_chaining: options.legacy_optional_chaining, }; let mut job = ingest_component_with_options( @@ -3827,6 +3840,7 @@ fn compile_component_full<'a>( metadata, template_pool_index, options.angular_version, + options.legacy_optional_chaining, ); // Extract the result and update pool index if host bindings were compiled @@ -4217,6 +4231,7 @@ pub fn compile_template_to_js_with_options<'a>( all_deferrable_deps_fn: None, pool_starting_index: 0, // Standalone template compilation starts from 0 angular_version: options.angular_version, + legacy_optional_chaining: options.legacy_optional_chaining, }; // Stage 3-5: Ingest and compile @@ -4271,6 +4286,7 @@ pub fn compile_template_to_js_with_options<'a>( options.selector.as_deref(), host_pool_starting_index, options.angular_version, + options.legacy_optional_chaining, ) { // Add host binding pool declarations (pure functions, etc.) for decl in host_result.declarations { @@ -4390,6 +4406,7 @@ pub fn compile_template_for_hmr<'a>( all_deferrable_deps_fn: None, pool_starting_index: 0, // HMR template compilation starts from 0 angular_version: options.angular_version, + legacy_optional_chaining: options.legacy_optional_chaining, }; // Stage 3-5: Ingest and compile @@ -4535,6 +4552,7 @@ fn compile_component_host_bindings<'a>( metadata: &ComponentMetadata<'a>, pool_starting_index: u32, angular_version: Option, + legacy_optional_chaining: Option, ) -> Option> { let host = metadata.host.as_ref()?; @@ -4558,8 +4576,13 @@ fn compile_component_host_bindings<'a>( // Ingest and compile the host bindings with the pool starting index // This ensures constant names continue from where template compilation left off - let mut job = - ingest_host_binding_with_version(allocator, input, pool_starting_index, angular_version); + let mut job = ingest_host_binding_with_version( + allocator, + input, + pool_starting_index, + angular_version, + legacy_optional_chaining, + ); let result = compile_host_bindings(&mut job); // Get the next pool index after host binding compilation @@ -4883,6 +4906,7 @@ fn compile_host_bindings_from_input<'a>( selector: Option<&str>, pool_starting_index: u32, angular_version: Option, + legacy_optional_chaining: Option, ) -> Option> { use oxc_allocator::FromIn; @@ -4908,8 +4932,13 @@ fn compile_host_bindings_from_input<'a>( // Convert to HostBindingInput and compile let input = convert_host_metadata_to_input(allocator, &host, component_name_atom, component_selector); - let mut job = - ingest_host_binding_with_version(allocator, input, pool_starting_index, angular_version); + let mut job = ingest_host_binding_with_version( + allocator, + input, + pool_starting_index, + angular_version, + legacy_optional_chaining, + ); let result = compile_host_bindings(&mut job); Some(result) @@ -4945,6 +4974,7 @@ pub fn compile_host_bindings_for_linker( selector, pool_starting_index, None, // Linker always targets latest Angular version + None, // legacyOptionalChaining: derive from (absent) version )?; let emitter = JsEmitter::new(); @@ -5064,6 +5094,7 @@ pub fn compile_template_for_linker<'a>( all_deferrable_deps_fn: None, pool_starting_index: 0, angular_version: None, + legacy_optional_chaining: None, }; let component_name_atom = Ident::from_in(component_name, allocator); diff --git a/crates/oxc_angular_compiler/src/ir/expression.rs b/crates/oxc_angular_compiler/src/ir/expression.rs index e5fc56e16..926060a21 100644 --- a/crates/oxc_angular_compiler/src/ir/expression.rs +++ b/crates/oxc_angular_compiler/src/ir/expression.rs @@ -592,6 +592,7 @@ impl<'a> IrExpression<'a> { ResolvedPropertyReadExpr { receiver: Box::new_in(e.receiver.clone_in(allocator), allocator), name: e.name.clone(), + optional: e.optional, source_span: e.source_span, }, allocator, @@ -615,6 +616,7 @@ impl<'a> IrExpression<'a> { ResolvedCallExpr { receiver: Box::new_in(e.receiver.clone_in(allocator), allocator), args, + optional: e.optional, source_span: e.source_span, }, allocator, @@ -624,6 +626,7 @@ impl<'a> IrExpression<'a> { ResolvedKeyedReadExpr { receiver: Box::new_in(e.receiver.clone_in(allocator), allocator), key: Box::new_in(e.key.clone_in(allocator), allocator), + optional: e.optional, source_span: e.source_span, }, allocator, @@ -923,6 +926,12 @@ pub struct ResolvedPropertyReadExpr<'a> { pub receiver: Box<'a, IrExpression<'a>>, /// Property name to read. pub name: Ident<'a>, + /// Whether to read via native optional chaining (`receiver?.name`). + /// + /// Set to `true` when a safe property read (`a?.b`) is expanded under modern + /// (Angular v22+) optional-chaining semantics, where `?.` yields `undefined`. + /// Legacy reads and plain (non-safe) reads leave this `false`. + pub optional: bool, /// Source span. pub source_span: Option, } @@ -955,6 +964,12 @@ pub struct ResolvedCallExpr<'a> { pub receiver: Box<'a, IrExpression<'a>>, /// The call arguments (resolved or original). pub args: Vec<'a, IrExpression<'a>>, + /// Whether to invoke via native optional chaining (`receiver?.()`). + /// + /// Set to `true` when a safe call (`a?.()`) is expanded under modern + /// (Angular v22+) optional-chaining semantics. Legacy and plain calls + /// leave this `false`. + pub optional: bool, /// Source span. pub source_span: Option, } @@ -970,6 +985,12 @@ pub struct ResolvedKeyedReadExpr<'a> { pub receiver: Box<'a, IrExpression<'a>>, /// The key expression (original, e.g., a number or expression). pub key: Box<'a, IrExpression<'a>>, + /// Whether to read via native optional chaining (`receiver?.[key]`). + /// + /// Set to `true` when a safe keyed read (`a?.[k]`) is expanded under modern + /// (Angular v22+) optional-chaining semantics. Legacy and plain keyed reads + /// leave this `false`. + pub optional: bool, /// Source span. pub source_span: Option, } diff --git a/crates/oxc_angular_compiler/src/pipeline/compilation.rs b/crates/oxc_angular_compiler/src/pipeline/compilation.rs index 9978ca4ec..12e0c625e 100644 --- a/crates/oxc_angular_compiler/src/pipeline/compilation.rs +++ b/crates/oxc_angular_compiler/src/pipeline/compilation.rs @@ -191,6 +191,14 @@ pub struct ComponentCompilationJob<'a> { /// `ɵɵconditionalCreate`/`ɵɵconditionalBranchCreate` for `@if`/`@switch` blocks. /// When `None`, assumes latest Angular version (v20+ behavior). pub angular_version: Option, + /// Explicit override for the `legacyOptionalChaining` compiler option. + /// + /// When `Some(true)`, safe navigation (`?.`) always expands to the legacy + /// `== null ? null` ternary. When `Some(false)`, it always emits native + /// optional chaining (`?.`, yielding `undefined`). When `None`, the default + /// is derived from `angular_version` (legacy for < v22, modern for >= v22, + /// legacy when the version is unknown). See `legacy_optional_chaining()`. + pub legacy_optional_chaining: Option, /// Diagnostics collected during compilation. pub diagnostics: std::vec::Vec, } @@ -241,6 +249,7 @@ impl<'a> ComponentCompilationJob<'a> { all_deferrable_deps_fn: None, content_selectors: None, angular_version: None, + legacy_optional_chaining: None, diagnostics: std::vec::Vec::new(), } } @@ -279,6 +288,21 @@ impl<'a> ComponentCompilationJob<'a> { self.angular_version.map_or(true, |v: AngularVersion| v.supports_dom_property()) } + /// Resolve whether safe navigation (`?.`) should use legacy null semantics. + /// + /// Returns `true` when `?.` must expand to the legacy `== null ? null` ternary, + /// `false` when it should emit native optional chaining (yielding `undefined`). + /// + /// An explicit `legacy_optional_chaining` override wins. Otherwise the default + /// follows the Angular version: legacy for < v22, modern for >= v22, and legacy + /// when the version is unknown (the safe, conservative fallback Angular itself + /// uses for partial-compiled libraries targeting older runtimes). + pub fn legacy_optional_chaining(&self) -> bool { + self.legacy_optional_chaining.unwrap_or_else(|| { + self.angular_version.map_or(true, |v| !v.supports_modern_optional_chaining()) + }) + } + /// Allocates a new cross-reference ID. pub fn allocate_xref_id(&mut self) -> XrefId { let id = XrefId::new(self.next_xref_id); @@ -621,6 +645,11 @@ pub struct HostBindingCompilationJob<'a> { pub diagnostics: std::vec::Vec, /// Angular version for version-gated instruction emission. pub angular_version: Option, + /// Explicit override for the `legacyOptionalChaining` compiler option. + /// + /// See [`ComponentCompilationJob::legacy_optional_chaining`] for the resolution + /// rules; `None` derives the default from `angular_version`. + pub legacy_optional_chaining: Option, } impl<'a> HostBindingCompilationJob<'a> { @@ -667,6 +696,7 @@ impl<'a> HostBindingCompilationJob<'a> { fn_suffix: Ident::from("HostBindings"), diagnostics: std::vec::Vec::new(), angular_version: None, + legacy_optional_chaining: None, } } @@ -685,6 +715,15 @@ impl<'a> HostBindingCompilationJob<'a> { self.angular_version.map_or(true, |v| v.supports_dom_property()) } + /// Resolve whether safe navigation (`?.`) should use legacy null semantics. + /// + /// See [`ComponentCompilationJob::legacy_optional_chaining`] for the rules. + pub fn legacy_optional_chaining(&self) -> bool { + self.legacy_optional_chaining.unwrap_or_else(|| { + self.angular_version.map_or(true, |v| !v.supports_modern_optional_chaining()) + }) + } + /// Allocates a new cross-reference ID. pub fn allocate_xref_id(&mut self) -> XrefId { let id = XrefId::new(self.next_xref_id); diff --git a/crates/oxc_angular_compiler/src/pipeline/conversion.rs b/crates/oxc_angular_compiler/src/pipeline/conversion.rs index 0ddd0240e..2a99c8ad0 100644 --- a/crates/oxc_angular_compiler/src/pipeline/conversion.rs +++ b/crates/oxc_angular_compiler/src/pipeline/conversion.rs @@ -345,6 +345,7 @@ pub fn convert_ast<'a>( allocator, ), name: pr.name.clone(), + optional: false, source_span: convert_source_span(pr.source_span), }, allocator, diff --git a/crates/oxc_angular_compiler/src/pipeline/ingest.rs b/crates/oxc_angular_compiler/src/pipeline/ingest.rs index efae25b02..17ffc12c6 100644 --- a/crates/oxc_angular_compiler/src/pipeline/ingest.rs +++ b/crates/oxc_angular_compiler/src/pipeline/ingest.rs @@ -122,6 +122,13 @@ pub struct IngestOptions<'a> { /// `ɵɵconditionalCreate`/`ɵɵconditionalBranchCreate` for `@if`/`@switch` blocks. /// When `None`, assumes latest Angular version (v20+ behavior). pub angular_version: Option, + + /// Explicit override for the `legacyOptionalChaining` compiler option. + /// + /// When `None`, the safe-navigation default is derived from `angular_version` + /// (legacy `null` for < v22, native optional chaining for >= v22, legacy when + /// the version is unknown). + pub legacy_optional_chaining: Option, } impl Default for IngestOptions<'_> { @@ -137,6 +144,7 @@ impl Default for IngestOptions<'_> { all_deferrable_deps_fn: None, pool_starting_index: 0, angular_version: None, + legacy_optional_chaining: None, } } } @@ -432,6 +440,7 @@ fn convert_ast_to_ir<'a>( allocator, ), name: prop.name, + optional: false, source_span: Some(prop.source_span.to_span()), }, allocator, @@ -447,6 +456,7 @@ fn convert_ast_to_ir<'a>( ResolvedPropertyReadExpr { receiver, name: prop.name, + optional: false, source_span: Some(prop.source_span.to_span()), }, allocator, @@ -467,6 +477,7 @@ fn convert_ast_to_ir<'a>( ResolvedKeyedReadExpr { receiver, key, + optional: false, source_span: Some(keyed.source_span.to_span()), }, allocator, @@ -490,6 +501,7 @@ fn convert_ast_to_ir<'a>( ResolvedCallExpr { receiver, args, + optional: false, source_span: Some(call.source_span.to_span()), }, allocator, @@ -793,6 +805,7 @@ pub fn ingest_component_with_options<'a>( // Set Angular version for feature-gated instruction selection job.angular_version = options.angular_version; + job.legacy_optional_chaining = options.legacy_optional_chaining; let root_xref = job.root.xref; @@ -3819,6 +3832,7 @@ fn host_convert_ast_to_ir<'a>( ResolvedPropertyReadExpr { receiver, name: prop.name, + optional: false, source_span: Some(prop.source_span.to_span()), }, allocator, @@ -3838,6 +3852,7 @@ fn host_convert_ast_to_ir<'a>( ResolvedKeyedReadExpr { receiver, key, + optional: false, source_span: Some(keyed.source_span.to_span()), }, allocator, @@ -3860,6 +3875,7 @@ fn host_convert_ast_to_ir<'a>( ResolvedCallExpr { receiver, args, + optional: false, source_span: Some(call.source_span.to_span()), }, allocator, @@ -3994,7 +4010,7 @@ pub fn ingest_host_binding<'a>( input: HostBindingInput<'a>, pool_starting_index: u32, ) -> HostBindingCompilationJob<'a> { - ingest_host_binding_with_version(allocator, input, pool_starting_index, None) + ingest_host_binding_with_version(allocator, input, pool_starting_index, None, None) } /// Ingest host bindings into a `HostBindingCompilationJob` with a specific Angular version. @@ -4003,6 +4019,7 @@ pub fn ingest_host_binding_with_version<'a>( input: HostBindingInput<'a>, pool_starting_index: u32, angular_version: Option, + legacy_optional_chaining: Option, ) -> HostBindingCompilationJob<'a> { let mut job = HostBindingCompilationJob::with_pool_starting_index( allocator, @@ -4011,6 +4028,7 @@ pub fn ingest_host_binding_with_version<'a>( pool_starting_index, ); job.angular_version = angular_version; + job.legacy_optional_chaining = legacy_optional_chaining; // Ingest host properties for property in input.properties { diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/expand_safe_reads.rs b/crates/oxc_angular_compiler/src/pipeline/phases/expand_safe_reads.rs index 18d4f653c..2b211041d 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/expand_safe_reads.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/expand_safe_reads.rs @@ -5,9 +5,23 @@ //! finds all unresolved safe read expressions and converts them into the appropriate output AST //! reads, guarded by null checks. //! +//! ## Optional-chaining semantics +//! +//! Since Angular v22, safe reads default to **native optional chaining** (`a?.b`), +//! which yields `undefined`. Earlier versions (and projects that opt in via the +//! `legacyOptionalChaining` compiler option) use the **legacy** `== null ? null` +//! expansion, which yields `null`. The choice per compilation is resolved by +//! [`ComponentCompilationJob::legacy_optional_chaining`]. +//! +//! A single expression can opt back into legacy semantics on a modern target by +//! wrapping it in the `$safeNavigationMigration(...)` magic function — this phase +//! detects that wrapper, forces the legacy expansion on the wrapped subtree, and +//! strips the wrapper. Mirrors Angular's `removeSafeNavigationMigration` + +//! `expandSafeReads` phases (`angular/angular@2896c93cc1`). +//! //! ## Algorithm //! -//! This phase performs two transformations: +//! **Legacy** mode performs two transformations: //! //! 1. **Safe Transform**: Converts safe access expressions to `SafeTernaryExpr` //! - `a?.b` → `SafeTernaryExpr { guard: a, expr: a.b }` @@ -17,6 +31,12 @@ //! 2. **Ternary Transform**: Converts `SafeTernaryExpr` to `ConditionalExpr` //! - `SafeTernaryExpr { guard, expr }` → `(guard == null ? null : expr)` //! +//! **Modern** mode instead rewrites each safe access into the equivalent resolved +//! read flagged as optional, which reifies to native `?.`: +//! - `a?.b` → `ResolvedPropertyRead { receiver: a, name: b, optional: true }` +//! - `a?.[k]` → `ResolvedKeyedRead { receiver: a, key: k, optional: true }` +//! - `a?.()` → `ResolvedCall { receiver: a, optional: true }` +//! //! ## Temporary Variables //! //! When the guard expression has side effects (e.g., function calls), a temporary @@ -54,7 +74,8 @@ use oxc_str::Ident; use crate::ir::expression::{ AssignTemporaryExpr, IrExpression, ReadTemporaryExpr, ResolvedCallExpr, ResolvedKeyedReadExpr, ResolvedPropertyReadExpr, SafeTernaryExpr, VisitorContextFlag, - transform_expressions_in_create_op, transform_expressions_in_update_op, + transform_expressions_in_create_op, transform_expressions_in_expression, + transform_expressions_in_update_op, }; use crate::ir::ops::XrefId; use crate::pipeline::compilation::{ComponentCompilationJob, HostBindingCompilationJob}; @@ -95,6 +116,10 @@ impl<'a> SafeTransformContext<'a> { pub fn expand_safe_reads(job: &mut ComponentCompilationJob<'_>) { let allocator = job.allocator; + // Resolve whether `?.` uses legacy (`== null ? null`) or modern (native `?.`) + // semantics for this compilation. Read before borrowing views mutably. + let legacy = job.legacy_optional_chaining(); + // Get the current xref counter value - we'll track allocations ourselves // and sync back at the end let starting_xref = job.allocate_xref_id().0; @@ -104,12 +129,35 @@ pub fn expand_safe_reads(job: &mut ComponentCompilationJob<'_>) { for view in job.all_views_mut() { let ctx = SafeTransformContext { allocator, next_xref: xref_counter.clone() }; - // Transform safe access expressions to SafeTernary + // Pass 1: resolve `$safeNavigationMigration(...)` markers. This forces the + // legacy expansion on the wrapped subtree and strips the wrapper. Runs in + // both modes (the wrapper must always be removed so it never reifies as a + // real call). + for op in view.create.iter_mut() { + transform_expressions_in_create_op( + op, + &|expr, _flags| { + resolve_safe_navigation_migration(expr, &ctx); + }, + VisitorContextFlag::NONE, + ); + } + for op in view.update.iter_mut() { + transform_expressions_in_update_op( + op, + &|expr, _flags| { + resolve_safe_navigation_migration(expr, &ctx); + }, + VisitorContextFlag::NONE, + ); + } + + // Pass 2: expand the remaining safe access expressions per the resolved mode. for op in view.create.iter_mut() { transform_expressions_in_create_op( op, &|expr, _flags| { - safe_transform(expr, &ctx); + expand_safe_access(expr, &ctx, legacy); }, VisitorContextFlag::NONE, ); @@ -118,7 +166,7 @@ pub fn expand_safe_reads(job: &mut ComponentCompilationJob<'_>) { transform_expressions_in_update_op( op, &|expr, _flags| { - safe_transform(expr, &ctx); + expand_safe_access(expr, &ctx, legacy); }, VisitorContextFlag::NONE, ); @@ -126,6 +174,121 @@ pub fn expand_safe_reads(job: &mut ComponentCompilationJob<'_>) { } } +/// Dispatch a single node to the legacy or modern safe-read expansion. +fn expand_safe_access<'a>( + expr: &mut IrExpression<'a>, + ctx: &SafeTransformContext<'a>, + legacy: bool, +) { + if legacy { + safe_transform(expr, ctx); + } else { + safe_transform_modern(expr, ctx); + } +} + +/// Rewrites `$safeNavigationMigration(arg)` into `arg`, expanding any safe reads +/// inside `arg` with legacy (`== null ? null`) semantics. +/// +/// After name resolution the wrapper appears as a `ResolvedCall` whose receiver is +/// a `ResolvedPropertyRead` of the magic name `$safeNavigationMigration` on the +/// component context. The wrapped argument's receivers are already resolved at this +/// point, so it is safe to run the legacy expansion on the detached subtree here. +/// +/// Mirrors Angular's `removeSafeNavigationMigration` phase combined with the +/// `InSafeNavigationMigration` flag handling in `expandSafeReads`. +fn resolve_safe_navigation_migration<'a>( + expr: &mut IrExpression<'a>, + ctx: &SafeTransformContext<'a>, +) { + let allocator = ctx.allocator; + + let is_marker = match expr { + IrExpression::ResolvedCall(call) => { + call.args.len() == 1 + && matches!( + call.receiver.as_ref(), + IrExpression::ResolvedPropertyRead(p) + if p.name.as_str() == "$safeNavigationMigration" + ) + } + _ => false, + }; + if !is_marker { + return; + } + + let IrExpression::ResolvedCall(call) = expr else { return }; + let mut arg = std::mem::replace(&mut call.args[0], make_placeholder(allocator)); + + // Force legacy null semantics on the wrapped subtree. + transform_expressions_in_expression( + &mut arg, + &|e: &mut IrExpression<'a>, _flags| { + safe_transform(e, ctx); + }, + VisitorContextFlag::NONE, + ); + + *expr = arg; +} + +/// Modern (Angular v22+) safe-read expansion: rewrite each safe access into the +/// equivalent resolved read flagged as optional, which reifies to native `?.`. +/// +/// Unlike the legacy expansion this needs no temporaries or ternary restructuring: +/// each safe node becomes optional independently, and the post-order visitor has +/// already converted any nested safe receivers. +fn safe_transform_modern<'a>(expr: &mut IrExpression<'a>, ctx: &SafeTransformContext<'a>) { + let allocator = ctx.allocator; + + match expr { + IrExpression::SafePropertyRead(p) => { + let receiver = std::mem::replace(p.receiver.as_mut(), make_placeholder(allocator)); + let name = p.name.clone(); + let source_span = p.source_span; + *expr = IrExpression::ResolvedPropertyRead(ArenaBox::new_in( + ResolvedPropertyReadExpr { + receiver: ArenaBox::new_in(receiver, allocator), + name, + optional: true, + source_span, + }, + allocator, + )); + } + IrExpression::SafeKeyedRead(k) => { + let receiver = std::mem::replace(k.receiver.as_mut(), make_placeholder(allocator)); + let key = std::mem::replace(k.index.as_mut(), make_placeholder(allocator)); + let source_span = k.source_span; + *expr = IrExpression::ResolvedKeyedRead(ArenaBox::new_in( + ResolvedKeyedReadExpr { + receiver: ArenaBox::new_in(receiver, allocator), + key: ArenaBox::new_in(key, allocator), + optional: true, + source_span, + }, + allocator, + )); + } + IrExpression::SafeInvokeFunction(c) => { + let receiver = std::mem::replace(c.receiver.as_mut(), make_placeholder(allocator)); + let args = std::mem::replace(&mut c.args, ArenaVec::new_in(allocator)); + let source_span = c.source_span; + *expr = IrExpression::ResolvedCall(ArenaBox::new_in( + ResolvedCallExpr { + receiver: ArenaBox::new_in(receiver, allocator), + args, + optional: true, + source_span, + }, + allocator, + )); + } + _ => {} + } +} + /// Checks if an IR expression requires a temporary variable to avoid re-evaluation. /// /// Returns true for expressions with side effects (function calls, pipe bindings), @@ -402,6 +565,9 @@ fn create_access_expr<'a>( ResolvedPropertyReadExpr { receiver: ArenaBox::new_in(receiver, allocator), name, + // Legacy expansion produces a plain read inside the `== null ? null` + // ternary, not native optional chaining. + optional: false, source_span, }, allocator, @@ -412,13 +578,19 @@ fn create_access_expr<'a>( ResolvedKeyedReadExpr { receiver: ArenaBox::new_in(receiver, allocator), key: ArenaBox::new_in(key, allocator), + optional: false, source_span, }, allocator, )) } AccessInfo::Call { args, source_span } => IrExpression::ResolvedCall(ArenaBox::new_in( - ResolvedCallExpr { receiver: ArenaBox::new_in(receiver, allocator), args, source_span }, + ResolvedCallExpr { + receiver: ArenaBox::new_in(receiver, allocator), + args, + optional: false, + source_span, + }, allocator, )), } @@ -559,17 +731,39 @@ fn safe_transform<'a>(expr: &mut IrExpression<'a>, ctx: &SafeTransformContext<'a pub fn expand_safe_reads_for_host(job: &mut HostBindingCompilationJob<'_>) { let allocator = job.allocator; + let legacy = job.legacy_optional_chaining(); + // Get the current xref counter value let starting_xref = job.allocate_xref_id().0; let xref_counter = RefCell::new(starting_xref); let ctx = SafeTransformContext { allocator, next_xref: xref_counter }; - // Transform safe access expressions to SafeTernary + // Pass 1: resolve `$safeNavigationMigration(...)` markers. + for op in job.root.create.iter_mut() { + transform_expressions_in_create_op( + op, + &|expr, _flags| { + resolve_safe_navigation_migration(expr, &ctx); + }, + VisitorContextFlag::NONE, + ); + } + for op in job.root.update.iter_mut() { + transform_expressions_in_update_op( + op, + &|expr, _flags| { + resolve_safe_navigation_migration(expr, &ctx); + }, + VisitorContextFlag::NONE, + ); + } + + // Pass 2: expand the remaining safe access expressions per the resolved mode. for op in job.root.create.iter_mut() { transform_expressions_in_create_op( op, &|expr, _flags| { - safe_transform(expr, &ctx); + expand_safe_access(expr, &ctx, legacy); }, VisitorContextFlag::NONE, ); @@ -578,7 +772,7 @@ pub fn expand_safe_reads_for_host(job: &mut HostBindingCompilationJob<'_>) { transform_expressions_in_update_op( op, &|expr, _flags| { - safe_transform(expr, &ctx); + expand_safe_access(expr, &ctx, legacy); }, VisitorContextFlag::NONE, ); diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/generate_variables.rs b/crates/oxc_angular_compiler/src/pipeline/phases/generate_variables.rs index c68147544..28743ed01 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/generate_variables.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/generate_variables.rs @@ -717,6 +717,7 @@ fn create_context_read_variable<'a>( ResolvedPropertyReadExpr { receiver: Box::new_in(context_expr, allocator), name: context_value, + optional: false, source_span: None, }, 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 9b4d26ba0..17e32fcb2 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 @@ -493,7 +493,7 @@ pub fn convert_ir_expression<'a>( ReadPropExpr { receiver: Box::new_in(receiver, allocator), name: resolved.name.clone(), - optional: false, + optional: resolved.optional, source_span: resolved.source_span, }, allocator, @@ -571,7 +571,7 @@ pub fn convert_ir_expression<'a>( fn_expr: Box::new_in(receiver, allocator), args, pure: false, - optional: false, + optional: resolved.optional, source_span: resolved.source_span, }, allocator, @@ -589,7 +589,7 @@ pub fn convert_ir_expression<'a>( ReadKeyExpr { receiver: Box::new_in(receiver, allocator), index: Box::new_in(index, allocator), - optional: false, + optional: resolved.optional, source_span: resolved.source_span, }, allocator, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs b/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs index 84f618d1a..394927a40 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs @@ -428,6 +428,7 @@ fn resolve_expression<'a>( allocator, ), name: lexical.name.clone(), + optional: false, source_span: lexical.source_span, }, allocator, @@ -519,6 +520,7 @@ fn resolve_expression<'a>( allocator, ), name: name.clone(), + optional: false, source_span, }, allocator, @@ -937,6 +939,7 @@ fn resolve_angular_expression<'a>( allocator, ), name: name.clone(), + optional: false, source_span, }, allocator, @@ -952,6 +955,7 @@ fn resolve_angular_expression<'a>( ResolvedPropertyReadExpr { receiver: Box::new_in(resolved_receiver, allocator), name: prop_read.name.clone(), + optional: false, source_span: Some(prop_read.source_span.to_span()), }, allocator, @@ -1009,6 +1013,7 @@ fn resolve_angular_expression<'a>( ResolvedCallExpr { receiver: Box::new_in(receiver, allocator), args: resolved_args, + optional: false, source_span: Some(call.source_span.to_span()), }, allocator, @@ -1044,6 +1049,7 @@ fn resolve_angular_expression<'a>( ResolvedKeyedReadExpr { receiver: Box::new_in(receiver, allocator), key: Box::new_in(key, allocator), + optional: false, source_span: Some(keyed.source_span.to_span()), }, allocator, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs b/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs index f1bff218d..740d6ad8f 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs @@ -318,6 +318,7 @@ mod tests { ResolvedPropertyReadExpr { receiver: oxc_allocator::Box::new_in(ctx, alloc), name: Ident::from(method_name), + optional: false, source_span: None, }, alloc, @@ -335,6 +336,7 @@ mod tests { ResolvedCallExpr { receiver: oxc_allocator::Box::new_in(prop_read, alloc), args, + optional: false, source_span: None, }, alloc, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/track_variables.rs b/crates/oxc_angular_compiler/src/pipeline/phases/track_variables.rs index e9e24e440..6c0ce8382 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/track_variables.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/track_variables.rs @@ -219,6 +219,7 @@ fn transform_angular_expression_for_track<'a>( ResolvedPropertyReadExpr { receiver: Box::new_in(transformed_receiver, allocator), name: prop_read.name.clone(), + optional: false, source_span: None, }, allocator, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs b/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs index 21dcf89b6..6a8ac3328 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs @@ -3501,6 +3501,7 @@ where allocator, ), name: rpr.name.clone(), + optional: rpr.optional, source_span: rpr.source_span, }, allocator, @@ -3537,6 +3538,7 @@ where allocator, ), args, + optional: rc.optional, source_span: rc.source_span, }, allocator, @@ -3554,6 +3556,7 @@ where transform_expression(&rkr.key, allocator, transform), allocator, ), + optional: rkr.optional, source_span: rkr.source_span, }, allocator, diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 7047820e7..74609b91e 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -2038,6 +2038,83 @@ fn test_pipe_in_binary_with_safe_property_read() { insta::assert_snapshot!("pipe_in_binary_with_safe_property_read", js); } +// ---------------------------------------------------------------------------- +// legacyOptionalChaining (Angular v22+): native `?.` vs legacy `== null ? null` +// See issue #317 and angular/angular@2896c93cc1. +// ---------------------------------------------------------------------------- + +#[test] +fn test_safe_navigation_modern_interpolation_v22() { + // Angular v22+ emits native optional chaining, which yields `undefined`. + let js = compile_template_to_js_with_version( + r"
{{user?.name}}
", + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.user?.name"), "expected native optional chaining, got:\n{js}"); + assert!(!js.contains("== null"), "modern mode must not emit the legacy null ternary:\n{js}"); +} + +#[test] +fn test_safe_navigation_modern_chain_v22() { + let js = compile_template_to_js_with_version( + r"
{{user?.address?.city}}
", + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.user?.address?.city"), "expected chained native `?.`, got:\n{js}"); +} + +#[test] +fn test_safe_navigation_modern_mixed_chain_v22() { + // Only the safe steps become optional; the plain `.b` stays a normal read. + let js = compile_template_to_js_with_version( + r"
{{a?.b.c?.d}}
", + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.a?.b.c?.d"), "expected mixed optional/plain chain, got:\n{js}"); +} + +#[test] +fn test_safe_call_modern_v22() { + let js = compile_template_to_js_with_version( + r"
{{getData?.()}}
", + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("ctx.getData?.()"), "expected native optional call, got:\n{js}"); +} + +#[test] +fn test_safe_navigation_legacy_on_v21() { + // Pre-v22 keeps the legacy `== null ? null` expansion. + let js = compile_template_to_js_with_version( + r"
{{user?.name}}
", + "TestComponent", + Some(AngularVersion::new(21, 0, 0)), + ); + assert!(js.contains("== null"), "v21 must use the legacy null ternary, got:\n{js}"); + assert!(!js.contains("ctx.user?.name"), "v21 must not emit native optional chaining:\n{js}"); +} + +#[test] +fn test_safe_navigation_migration_forces_legacy_on_v22() { + // The `$safeNavigationMigration(...)` magic function opts a subtree back into + // legacy null semantics even on a modern (v22) target, and is stripped from + // the output. + let js = compile_template_to_js_with_version( + r"
{{ $safeNavigationMigration(user?.name) }}
", + "TestComponent", + Some(AngularVersion::new(22, 0, 0)), + ); + assert!(js.contains("== null"), "wrapped subtree must use the legacy null ternary, got:\n{js}"); + assert!( + !js.contains("$safeNavigationMigration"), + "the migration wrapper must be stripped from the output:\n{js}" + ); +} + // ============================================================================ // Event Modifier Tests // ============================================================================ diff --git a/napi/angular-compiler/index.d.ts b/napi/angular-compiler/index.d.ts index 5e13671d3..97a18ff1f 100644 --- a/napi/angular-compiler/index.d.ts +++ b/napi/angular-compiler/index.d.ts @@ -768,6 +768,17 @@ export interface TransformOptions { * When not set, assumes latest Angular version (v19+ behavior). */ angularVersion?: AngularVersion + /** + * Override for the `legacyOptionalChaining` Angular compiler option + * (`angularCompilerOptions.legacyOptionalChaining` in `tsconfig.json`). + * + * Controls the safe-navigation operator (`?.`) in template expressions. + * When `true`, always emits the legacy `== null ? null` form; when `false`, + * emits native optional chaining (yielding `undefined`). When unset, the + * default is derived from `angularVersion` (legacy for < v22, modern for + * >= v22, legacy when the version is unknown). + */ + legacyOptionalChaining?: boolean /** The CSS selector that identifies this component in a template. */ selector?: string /** diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index eb06ffa8c..e658a9e2c 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -138,6 +138,16 @@ pub struct TransformOptions { /// When not set, assumes latest Angular version (v19+ behavior). pub angular_version: Option, + /// Override for the `legacyOptionalChaining` Angular compiler option + /// (`angularCompilerOptions.legacyOptionalChaining` in `tsconfig.json`). + /// + /// Controls the safe-navigation operator (`?.`) in template expressions. + /// When `true`, always emits the legacy `== null ? null` form; when `false`, + /// emits native optional chaining (yielding `undefined`). When unset, the + /// default is derived from `angularVersion` (legacy for < v22, modern for + /// >= v22, legacy when the version is unknown). + pub legacy_optional_chaining: Option, + // Component metadata fields for full compilation testing // These mirror Angular's @Component decorator options. /// The CSS selector that identifies this component in a template. @@ -229,6 +239,7 @@ impl From for RustTransformOptions { advanced_optimizations: options.advanced_optimizations.unwrap_or(false), i18n_use_external_ids: options.i18n_use_external_ids.unwrap_or(true), angular_version: options.angular_version.map(Into::into), + legacy_optional_chaining: options.legacy_optional_chaining, // Component metadata overrides selector: options.selector, standalone: options.standalone,