refactor(migrations): automatically skip inputs outside of project (angular#57883)

devversion · pkozlowski-opensource · commit d8aba1d90119 · 2024-09-20T12:19:11.000+02:00
Currently we support filtering files outside of the project, or source files via `shouldMigrateInput` option. This works well, but we can smartly skip inputs in batch migrations if we never saw a source declaration. This is an improvement in G3 where we cannot simply limit the migration to a given directory, because we may include build targets from various places. E.g. via reverse dependency tracking— so this fixes the issue naturally. Notably, an explicit filter would improve reference lookups because we wouldn't consider the input when determining potential references. That is because we would know beforehand that those inputs in the `.d.ts` cannot be migrated inputs— and therefore references with names of the input would never be verified via expensive type checking. This is fine for G3 though, and there is no way around this. This is a slow performance overhead; mostly releveant for VSCode integration. PR Close angular#57883
diff --git a/packages/core/schematics/migrations/signal-migration/src/batch/extract.ts b/packages/core/schematics/migrations/signal-migration/src/batch/extract.ts
@@ -37,6 +37,7 @@ export function getCompilationUnitMetadata(knownInputs: KnownInputs, result: Mig
           ...res,
           [inputClassFieldIdStr as string & ClassFieldUniqueKey]: {
             isIncompatible: incompatibility,
+            seenAsSourceInput: info.metadata.inSourceFile,
           },
         };
       },
diff --git a/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts b/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts
@@ -30,20 +30,28 @@ export function mergeCompilationUnitData(
       const existing = result.knownInputs[key];
       if (existing === undefined) {
         result.knownInputs[key] = info;
-      } else if (existing.isIncompatible === null && info.isIncompatible) {
+        continue;
+      }
+
+      if (existing.isIncompatible === null && info.isIncompatible) {
         // input might not be incompatible in one target, but others might invalidate it.
         // merge the incompatibility state.
         existing.isIncompatible = info.isIncompatible;
       }
+      if (!existing.seenAsSourceInput && info.seenAsSourceInput) {
+        existing.seenAsSourceInput = true;
+      }
     }
+  }
 
-    for (const reference of file.references) {
-      const referenceId = computeReferenceId(reference);
-      if (seenReferenceFromIds.has(referenceId)) {
-        continue;
-      }
-      seenReferenceFromIds.add(referenceId);
-      result.references.push(reference);
+  for (const info of Object.values(result.knownInputs)) {
+    // We never saw a source file for this input, globally. Mark it as incompatible,
+    // so that all references and inheritance checks can propagate accordingly.
+    if (!info.seenAsSourceInput) {
+      info.isIncompatible = {
+        kind: IncompatibilityType.VIA_INPUT,
+        reason: InputIncompatibilityReason.OutsideOfMigrationScope,
+      };
     }
   }
 
diff --git a/packages/core/schematics/migrations/signal-migration/src/batch/unit_data.ts b/packages/core/schematics/migrations/signal-migration/src/batch/unit_data.ts
@@ -51,6 +51,7 @@ export interface CompilationUnitData {
         | {kind: IncompatibilityType.VIA_CLASS; reason: ClassIncompatibilityReason}
         | {kind: IncompatibilityType.VIA_INPUT; reason: InputIncompatibilityReason}
         | null;
+      seenAsSourceInput: boolean;
     };
   };
 
diff --git a/packages/core/schematics/migrations/signal-migration/src/best_effort_mode.ts b/packages/core/schematics/migrations/signal-migration/src/best_effort_mode.ts
@@ -11,6 +11,8 @@ import {KnownInputs} from './input_detection/known_inputs';
 
 /** Input reasons that cannot be ignored. */
 const nonIgnorableInputIncompatibilities: InputIncompatibilityReason[] = [
+  // Outside of scope inputs should not be migrated. E.g. references to inputs in `node_modules/`.
+  InputIncompatibilityReason.OutsideOfMigrationScope,
   // Explicitly filtered inputs cannot be skipped via best effort mode.
   InputIncompatibilityReason.SkippedViaConfigFilter,
   // There is no good output for accessor inputs.
diff --git a/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility.ts b/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility.ts
@@ -10,6 +10,7 @@ import ts from 'typescript';
 
 /** Reasons why an input cannot be migrated. */
 export enum InputIncompatibilityReason {
+  OutsideOfMigrationScope,
   SkippedViaConfigFilter,
   Accessor,
   WriteAssignment,
diff --git a/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human.ts b/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human.ts
@@ -74,6 +74,11 @@ export function getMessageForInputIncompatibility(reason: InputIncompatibilityRe
         short: 'Your application code writes to the input. This prevents migration.',
         extra: 'Signal inputs are readonly, so migrating would break your build.',
       };
+    case InputIncompatibilityReason.OutsideOfMigrationScope:
+      return {
+        short: 'This input is not part of any source files in your project.',
+        extra: 'The migration excludes inputs if no source file declaring the input was seen.',
+      };
   }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -74,6 +74,11 @@ export function getMessageForInputIncompatibility(reason: InputIncompatibilityRe`
`74`	`74`	`short: 'Your application code writes to the input. This prevents migration.',`
`75`	`75`	`extra: 'Signal inputs are readonly, so migrating would break your build.',`
`76`	`76`	`};`
	`77`	`+ case InputIncompatibilityReason.OutsideOfMigrationScope:`
	`78`	`+ return {`
	`79`	`+ short: 'This input is not part of any source files in your project.',`
	`80`	`+ extra: 'The migration excludes inputs if no source file declaring the input was seen.',`
	`81`	`+ };`
`77`	`82`	`}`
`78`	`83`	`}`
`79`	`84`