refactor(migrations): improve safety of queries migration when dealing with union types (#58190)

When dealing with union types of queries, we should be conservative in
the default mode and skip migration. That is because `viewChild()`
always includes `undefined`, but we cannot trivially emulate the
behavior where it was initially set to `null` (as an alternative to
`undefined`). Those queries can still be migrated via best effort
mode.

PR Close #58190

packages/core/schematics/migrations/signal-migration/src/passes/problematic_patterns/incompatibility.ts

@@ -25,10 +25,11 @@ export enum FieldIncompatibilityReason {
   SignalInput__RequiredButNoGoodExplicitTypeExtractable = 8,
   SignalInput__QuestionMarkButNoGoodExplicitTypeExtractable = 9,
   SignalQueries__QueryListProblematicFieldAccessed = 10,
-  WriteAssignment = 11,
-  Accessor = 12,
-  OutsideOfMigrationScope = 13,
-  SkippedViaConfigFilter = 14,
+  SignalQueries__IncompatibleMultiUnionType = 11,
+  WriteAssignment = 12,
+  Accessor = 13,
+  OutsideOfMigrationScope = 14,
+  SkippedViaConfigFilter = 15,
 }
 
 /** Field reasons that cannot be ignored. */

packages/core/schematics/migrations/signal-migration/src/passes/problematic_patterns/incompatibility_human.ts

@@ -75,6 +75,11 @@ export function getMessageForFieldIncompatibility(
         short: `There are references to this query that cannot be migrated automatically.`,
         extra: "For example, it's not possible to migrate `.changes` or `.dirty` trivially.",
       };
+    case FieldIncompatibilityReason.SignalQueries__IncompatibleMultiUnionType:
+      return {
+        short: `Query type is too complex to automatically migrate.`,
+        extra: "The new query API doesn't allow us to migrate safely without breaking your app.",
+      };
     case FieldIncompatibilityReason.SkippedViaConfigFilter:
       return {
         short: `This ${fieldName.single} is not part of the current migration scope.`,

packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts

@@ -1303,6 +1303,45 @@ describe('signal queries migration', () => {
     ).not.toBeRejected();
   });
 
+  it('should properly deal with union types of single queries', async () => {
+    const {fs} = await runTsurgeMigration(new SignalQueriesMigration({}), [
+      {
+        name: absoluteFrom('/app.component.ts'),
+        isProgramRootFile: true,
+        contents: dedent`
+          import {ViewChild, ElementRef, Component} from '@angular/core';
+
+          @Component({
+            template: '',
+          })
+          class MyComp {
+            @ViewChild(MyService) bla: MyService|undefined = undefined;
+            @ViewChild(MyService) bla2?: MyService;
+            @ViewChild(MyService) bla3: MyService|null = null;
+            @ViewChild(MyService) bla4: MyService|SomethingUnrelated = null!;
+            @ViewChild(MyService) bla5!: MyService|SomethingUnrelated;
+          }
+        `,
+      },
+    ]);
+
+    const actual = fs.readFile(absoluteFrom('/app.component.ts'));
+    expect(actual).toMatchWithDiff(`
+      import {ViewChild, ElementRef, Component, viewChild} from '@angular/core';
+
+      @Component({
+        template: '',
+      })
+      class MyComp {
+        readonly bla = viewChild(MyService);
+        readonly bla2 = viewChild(MyService);
+        @ViewChild(MyService) bla3: MyService|null = null;
+        @ViewChild(MyService) bla4: MyService|SomethingUnrelated = null!;
+        @ViewChild(MyService) bla5!: MyService|SomethingUnrelated;
+      }
+    `);
+  });
+
   describe('--best-effort-mode', () => {
     it('should be possible to forcibly migrate even with a detected `.changes` access', async () => {
       const {fs} = await runTsurgeMigration(new SignalQueriesMigration({bestEffortMode: true}), [

packages/core/schematics/migrations/signal-queries-migration/migration.ts

@@ -123,11 +123,12 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
     const findQueryDefinitionsVisitor = (node: ts.Node) => {
       const extractedQuery = extractSourceQueryDefinition(node, reflector, evaluator, info);
       if (extractedQuery !== null) {
+        const queryNode = extractedQuery.node;
         const descriptor = {
           key: extractedQuery.id,
-          node: extractedQuery.node,
+          node: queryNode,
         };
-        const containingFile = projectFile(descriptor.node.getSourceFile(), info);
+        const containingFile = projectFile(queryNode.getSourceFile(), info);
 
         // If we have a config filter function, use it here for later
         // perf-boosted reference lookups. Useful in non-batch mode.
@@ -135,7 +136,7 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
           this.config.shouldMigrateQuery === undefined ||
           this.config.shouldMigrateQuery(descriptor, containingFile)
         ) {
-          classesWithFilteredQueries.add(extractedQuery.node.parent);
+          classesWithFilteredQueries.add(queryNode.parent);
           filteredQueriesForCompilationUnit.set(extractedQuery.id, {
             fieldName: extractedQuery.queryInfo.propertyName,
           });
@@ -146,13 +147,30 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
           isMulti: extractedQuery.queryInfo.first === false,
         };
 
-        if (ts.isAccessor(extractedQuery.node)) {
+        if (ts.isAccessor(queryNode)) {
           markFieldIncompatibleInMetadata(
             res.potentialProblematicQueries,
             extractedQuery.id,
             FieldIncompatibilityReason.Accessor,
           );
         }
+
+        // Detect queries with union types that are uncommon to be
+        // automatically migrate-able. E.g. `refs: ElementRef|null`,
+        // or `ElementRef|SomeOtherType`.
+        if (
+          queryNode.type !== undefined &&
+          ts.isUnionTypeNode(queryNode.type) &&
+          // Either too large union, or doesn't match `T|undefined`.
+          (queryNode.type.types.length > 2 ||
+            !queryNode.type.types.some((t) => t.kind === ts.SyntaxKind.UndefinedKeyword))
+        ) {
+          markFieldIncompatibleInMetadata(
+            res.potentialProblematicQueries,
+            extractedQuery.id,
+            FieldIncompatibilityReason.SignalQueries__IncompatibleMultiUnionType,
+          );
+        }
       }
     };