From f579e1a1d3c27e7c4d6f25195d399e01a07dff99 Mon Sep 17 00:00:00 2001
From: Matthieu Riegler <kyro38@gmail.com>
Date: Thu, 28 May 2026 09:56:59 +0200
Subject: [PATCH] fix(migrations): Make the safe optional chaining idempotent

Our unit tests were missleading, the migration wasn't idempotent and `$safeNavigationMigration` were added multiple times on consecutive runs.
---
 .../safe-optional-chaining/migration.ts       | 17 ++++++++++
 .../safe-optional-chaining.spec.ts            | 31 ++++++++++++++-----
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/packages/core/schematics/migrations/safe-optional-chaining/migration.ts b/packages/core/schematics/migrations/safe-optional-chaining/migration.ts
index b116a5e1132c..dc890809d4fc 100644
--- a/packages/core/schematics/migrations/safe-optional-chaining/migration.ts
+++ b/packages/core/schematics/migrations/safe-optional-chaining/migration.ts
@@ -68,6 +68,8 @@ export interface MigrationConfig {
   shouldMigrate?: (containingFile: ProjectFile) => boolean;
 }
 
+const SAFE_NAVIGATION_MIGRATION_FN = '$safeNavigationMigration';
+
 /**
  * This migration wraps optional chaining expressions in Angular templates with a call to the
  * `$safeNavigationMigration()` magic function. This function doesn't exist at runtime, but is
@@ -371,6 +373,11 @@ class ExpressionMigrator extends RecursiveAstVisitor {
   // ---------------------------------------------------------------------------
 
   override visitCall(ast: Call, nullSensitive: boolean): any {
+    if (isSafeNavigationMigrationCall(ast)) {
+      this.visit(ast.receiver, false);
+      return;
+    }
+
     if (nullSensitive && this.hasSafeReceiver(ast.receiver)) {
       this.addReplacement(ast);
     }
@@ -503,6 +510,16 @@ function isNullishLiteralAST(ast: AST): boolean {
   );
 }
 
+function isSafeNavigationMigrationCall(ast: AST): boolean {
+  const innerAst = ast instanceof ASTWithSource ? ast.ast : ast;
+
+  return (
+    innerAst instanceof Call &&
+    innerAst.receiver instanceof PropertyRead &&
+    innerAst.receiver.name === SAFE_NAVIGATION_MIGRATION_FN
+  );
+}
+
 /** Returns true if the AST node is a non-null, non-undefined primitive literal. */
 function isNonNullishLiteralAST(ast: AST): boolean {
   const innerAst = ast instanceof ASTWithSource ? ast.ast : ast;
diff --git a/packages/core/schematics/migrations/safe-optional-chaining/safe-optional-chaining.spec.ts b/packages/core/schematics/migrations/safe-optional-chaining/safe-optional-chaining.spec.ts
index 31d098e32251..7dd446f50b46 100644
--- a/packages/core/schematics/migrations/safe-optional-chaining/safe-optional-chaining.spec.ts
+++ b/packages/core/schematics/migrations/safe-optional-chaining/safe-optional-chaining.spec.ts
@@ -515,27 +515,27 @@ describe('SafeOptionalChainingMigration', () => {
       {{ foo?.bar | json }}
       <div [id]="user?.id"></div>
       <div *ngIf="foo?.bar === null"></div>
+      {{ compute(my?.utility?.service(my?.optional?.argument)) }}
     `;
 
     // First pass: migrate fresh code
-    const firstPass = await migrateInlineTemplate(input);
+    const firstPass = await migrateInlineTemplateContent(input);
 
     // Verify all expressions are wrapped correctly on first pass
     expect(firstPass).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
     expect(firstPass).toContain('{{ $safeNavigationMigration(foo?.bar) | json }}');
     expect(firstPass).toContain('<div [id]="$safeNavigationMigration(user?.id)"></div>');
     expect(firstPass).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) === null"></div>');
+    expect(firstPass).toContain(
+      '{{ compute($safeNavigationMigration(my?.utility?.service($safeNavigationMigration(my?.optional?.argument)))) }}',
+    );
 
     // Second pass: run migration again on already-migrated code
-    const secondPass = await migrateInlineTemplate(firstPass);
+    const secondPass = await migrateInlineTemplateContent(firstPass);
 
     // Verify no double-wrapping occurred
     expect(secondPass).not.toContain('$safeNavigationMigration($safeNavigationMigration');
-    // The already-wrapped expressions should remain unchanged
-    expect(secondPass).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
-    expect(secondPass).toContain('{{ $safeNavigationMigration(foo?.bar) | json }}');
-    expect(secondPass).toContain('<div [id]="$safeNavigationMigration(user?.id)"></div>');
-    expect(secondPass).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) === null"></div>');
+    expect(normalizeTemplateContent(secondPass)).toBe(normalizeTemplateContent(firstPass));
   });
 });
 
@@ -552,13 +552,28 @@ async function migrateInlineTemplate(template: string): Promise<string> {
                ${template}
               \`
             })
-            export class AppComponent { foo: any; compute(a: any) {} }
+            export class AppComponent { foo: any; compute(a: any) {}; my:any; }
             `,
     },
   ]);
   return fs.readFile(absoluteFrom('/app.component.ts'));
 }
 
+async function migrateInlineTemplateContent(template: string): Promise<string> {
+  const contents = await migrateInlineTemplate(template);
+  const match = contents.match(/template:\s*`([\s\S]*?)`\s*}\)\s*export class/);
+
+  if (match === null) {
+    throw new Error('Failed to extract migrated inline template content.');
+  }
+
+  return match[1];
+}
+
+function normalizeTemplateContent(template: string): string {
+  return template.replace(/^\s+|\s+$/g, '');
+}
+
 async function migrateExternalTemplate(template: string): Promise<string> {
   const {fs} = await runTsurgeMigration(new SafeOptionalChainingMigration(), [
     {