fix(safe-nav): version-aware optional chaining for Angular v22+ (#317) (#330)

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) <noreply@anthropic.com>

Author: brandonroberts

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(...)(...)`).
     ///

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<AngularVersion>,
 
+    /// 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<bool>,
+
     // 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<AngularVersion>,
+    legacy_optional_chaining: Option<bool>,
 ) -> Option<HostBindingCompilationOutput<'a>> {
     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<crate::AngularVersion>,
+    legacy_optional_chaining: Option<bool>,
 ) -> Option<HostBindingCompilationResult<'a>> {
     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);

crates/oxc_angular_compiler/src/ir/enums.rs

@@ -264,6 +264,9 @@ pub enum ExpressionKind {
     ArrowFunction,
     /// Parenthesized expression.
     Parenthesized,
+    /// `$safeNavigationMigration(...)` wrapper marking its subtree for legacy
+    /// safe-read semantics (removed by `expandSafeReads`).
+    SafeNavigationMigration,
 }
 
 /// Flags for semantic variables.

crates/oxc_angular_compiler/src/ir/expression.rs

@@ -50,6 +50,12 @@ impl VisitorContextFlag {
     pub const IN_CHILD_OPERATION: Self = Self(0b0001);
     /// Inside an arrow function operation.
     pub const IN_ARROW_FUNCTION_OPERATION: Self = Self(0b0010);
+    /// Inside a `$safeNavigationMigration(...)` wrapper. Safe reads in this subtree
+    /// expand under legacy (`== null ? null`) semantics even when the compilation
+    /// targets native optional chaining.
+    ///
+    /// Mirrors Angular's `VisitorContextFlag.InSafeNavigationMigration` (`0b0100`).
+    pub const IN_SAFE_NAVIGATION_MIGRATION: Self = Self(0b0100);
 
     /// Check if a flag is set.
     pub fn contains(self, other: Self) -> bool {
@@ -153,6 +159,10 @@ pub enum IrExpression<'a> {
     /// Safe property read with resolved receiver (created during name resolution).
     /// Used when an expression like `item?.name` has the receiver resolved to a variable.
     ResolvedSafePropertyRead(Box<'a, ResolvedSafePropertyReadExpr<'a>>),
+    /// `$safeNavigationMigration(...)` wrapper marking its subtree for legacy
+    /// (`== null ? null`) safe-read semantics. Created by the
+    /// `removeSafeNavigationMigration` phase and removed by `expandSafeReads`.
+    SafeNavigationMigration(Box<'a, SafeNavigationMigrationExpr<'a>>),
     /// Derived literal array for pure function bodies.
     /// Contains IrExpression entries that can include PureFunctionParameter references.
     DerivedLiteralArray(Box<'a, DerivedLiteralArrayExpr<'a>>),
@@ -260,6 +270,7 @@ impl<'a> IrExpression<'a> {
             // ArrowFunction is an arrow function expression
             IrExpression::ArrowFunction(_) => ExpressionKind::ArrowFunction,
             IrExpression::Parenthesized(_) => ExpressionKind::Parenthesized,
+            IrExpression::SafeNavigationMigration(_) => ExpressionKind::SafeNavigationMigration,
         }
     }
 }
@@ -592,6 +603,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 +627,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 +637,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,
@@ -790,6 +804,15 @@ impl<'a> IrExpression<'a> {
                 },
                 allocator,
             )),
+            IrExpression::SafeNavigationMigration(e) => {
+                IrExpression::SafeNavigationMigration(Box::new_in(
+                    SafeNavigationMigrationExpr {
+                        expr: Box::new_in(e.expr.clone_in(allocator), allocator),
+                        source_span: e.source_span,
+                    },
+                    allocator,
+                ))
+            }
         }
     }
 }
@@ -923,6 +946,31 @@ 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<Span>,
+}
+
+/// `$safeNavigationMigration(expr)` wrapper.
+///
+/// A magic marker that opts `expr`'s safe reads into legacy `== null ? null`
+/// semantics even when the compilation targets native optional chaining. Created
+/// by the `removeSafeNavigationMigration` phase from an unqualified
+/// `$safeNavigationMigration(...)` call, and unwrapped in `expandSafeReads` after
+/// its subtree has been expanded under the
+/// [`VisitorContextFlag::IN_SAFE_NAVIGATION_MIGRATION`] flag.
+///
+/// Ported from Angular's `SafeNavigationMigrationExpr` in
+/// `template/pipeline/ir/src/expression.ts`.
+#[derive(Debug)]
+pub struct SafeNavigationMigrationExpr<'a> {
+    /// The wrapped expression.
+    pub expr: Box<'a, IrExpression<'a>>,
     /// Source span.
     pub source_span: Option<Span>,
 }
@@ -955,6 +1003,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<Span>,
 }
@@ -970,6 +1024,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<Span>,
 }
@@ -1664,6 +1724,13 @@ pub fn transform_expressions_in_expression<'a, F>(
         IrExpression::Parenthesized(e) => {
             transform_expressions_in_expression(&mut e.expr, transform, flags);
         }
+        IrExpression::SafeNavigationMigration(e) => {
+            // Mirror Angular's `SafeNavigationMigrationExpr.transformInternalExpressions`:
+            // the wrapped subtree is visited with the `InSafeNavigationMigration` flag so
+            // its safe reads expand under legacy `== null ? null` semantics.
+            let child_flags = flags.union(VisitorContextFlag::IN_SAFE_NAVIGATION_MIGRATION);
+            transform_expressions_in_expression(&mut e.expr, transform, child_flags);
+        }
         // These expressions have no internal expressions
         IrExpression::LexicalRead(_)
         | IrExpression::Reference(_)
@@ -1844,6 +1911,10 @@ pub fn visit_expressions_in_expression<'a, F>(
         IrExpression::Parenthesized(e) => {
             visit_expressions_in_expression(&e.expr, visitor, flags);
         }
+        IrExpression::SafeNavigationMigration(e) => {
+            let child_flags = flags.union(VisitorContextFlag::IN_SAFE_NAVIGATION_MIGRATION);
+            visit_expressions_in_expression(&e.expr, visitor, child_flags);
+        }
         // These expressions have no internal expressions
         IrExpression::LexicalRead(_)
         | IrExpression::Reference(_)
@@ -2813,6 +2884,9 @@ pub fn vars_used_by_ir_expression(expr: &IrExpression<'_>) -> u32 {
             vars_used_by_ir_expression(&b.left) + vars_used_by_ir_expression(&b.right)
         }
 
+        // The $safeNavigationMigration wrapper consumes no slots itself; recurse.
+        IrExpression::SafeNavigationMigration(m) => vars_used_by_ir_expression(&m.expr),
+
         // SafePropertyRead has a receiver expression
         IrExpression::SafePropertyRead(spr) => vars_used_by_ir_expression(&spr.receiver),