refactor(migrations): cleanup TODOs in signal input migration (#57566)

devversion · AndrewKushnir · commit bb29c8bbd79f · 2024-08-29T07:51:38.000-07:00
Cleans up remaining TODOs in the signal input migration, and other small clean ups. PR Close #57566
diff --git a/packages/core/schematics/migrations/signal-migration/src/convert-input/convert_to_signal.ts b/packages/core/schematics/migrations/signal-migration/src/convert-input/convert_to_signal.ts
@@ -31,13 +31,12 @@ export function convertToSignalInput(
     resolvedMetadata: metadata,
     resolvedType,
     preferShorthandIfPossible,
-    isUndefinedInitialValue,
     originalInputDecorator,
+    initialValue,
   }: ConvertInputPreparation,
   checker: ts.TypeChecker,
   importManager: ImportManager,
 ): string {
-  let initialValue = node.initializer;
   let optionsLiteral: ts.ObjectLiteralExpression | null = null;
 
   // We need an options array for the input because:
@@ -63,7 +62,7 @@ export function convertToSignalInput(
   // The initial value is `undefined` or none is present:
   //    - We may be able to use the `input()` shorthand
   //    - or we use an explicit `undefined` initial value.
-  if (isUndefinedInitialValue) {
+  if (initialValue === undefined) {
     // Shorthand not possible, so explicitly add `undefined`.
     if (preferShorthandIfPossible === null) {
       initialValue = ts.factory.createIdentifier('undefined');
@@ -95,7 +94,6 @@ export function convertToSignalInput(
   // Always add an initial value when the input is optional, and we have one, or we need one
   // to be able to pass options as the second argument.
   if (!metadata.required && (initialValue !== undefined || optionsLiteral !== null)) {
-    // TODO: undefined `input()` shorthand support!
     inputArgs.push(initialValue ?? ts.factory.createIdentifier('undefined'));
   }
 
diff --git a/packages/core/schematics/migrations/signal-migration/src/convert-input/prepare_and_check.ts b/packages/core/schematics/migrations/signal-migration/src/convert-input/prepare_and_check.ts
@@ -23,9 +23,9 @@ import assert from 'assert';
 export interface ConvertInputPreparation {
   resolvedType: ts.TypeNode | undefined;
   preferShorthandIfPossible: {originalType: ts.TypeNode} | null;
-  isUndefinedInitialValue: boolean;
   resolvedMetadata: ExtractedInput;
   originalInputDecorator: Decorator;
+  initialValue: ts.Expression | undefined;
 }
 
 /**
@@ -126,7 +126,7 @@ export function prepareAndCheckForConversion(
     resolvedMetadata: metadata,
     resolvedType: typeToAdd,
     preferShorthandIfPossible,
-    isUndefinedInitialValue,
     originalInputDecorator: metadata.inputDecorator,
+    initialValue: isUndefinedInitialValue ? undefined : initialValue,
   };
 }
diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/7_migrate_template_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/7_migrate_template_references.ts
@@ -41,8 +41,6 @@ export function pass7__migrateTemplateReferences(
     }
     seenFileReferences.add(fileReferenceId);
 
-    // TODO: Control flow, or wait for Joost's PR?
-
     // Expand shorthands like `{bla}` to `{bla: bla()}`.
     const appendText = reference.from.isObjectShorthandExpression
       ? `: ${reference.from.read.name}()`
diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/catalyst_test.ts b/packages/core/schematics/migrations/signal-migration/test/golden-test/catalyst_test.ts
@@ -14,8 +14,7 @@ class MyComp {
 
 it('should work', () => {
   const inputs = {
-    hello: 'Damn',
-    // TODO:
+    hello: 'test',
   } as Partial<MyComp>;
   bootstrapTemplate('<my-comp [hello]="hello">', inputs);
 });
diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/constructor_initializations.ts b/packages/core/schematics/migrations/signal-migration/test/golden-test/constructor_initializations.ts
@@ -0,0 +1,14 @@
+// tslint:disable
+
+import {Input, Directive} from '@angular/core';
+
+@Directive()
+export class MyComp {
+  @Input() firstName: string;
+
+  constructor() {
+    // TODO: Consider initializations inside the constructor.
+    // Those are not migrated right now though, as they are writes.
+    this.firstName = 'Initial value';
+  }
+}
diff --git a/packages/core/schematics/migrations/signal-migration/test/golden.txt b/packages/core/schematics/migrations/signal-migration/test/golden.txt
@@ -92,11 +92,26 @@ class MyComp {
 
 it('should work', () => {
   const inputs = {
-    hello: 'Damn',
-    // TODO:
+    hello: 'test',
   } as Partial<UnwrapSignalInputs<MyComp>>;
   bootstrapTemplate('<my-comp [hello]="hello">', inputs);
 });
+@@@@@@ constructor_initializations.ts @@@@@@
+
+// tslint:disable
+
+import {Input, Directive} from '@angular/core';
+
+@Directive()
+export class MyComp {
+  @Input() firstName: string;
+
+  constructor() {
+    // TODO: Consider initializations inside the constructor.
+    // Those are not migrated right now though, as they are writes.
+    this.firstName = 'Initial value';
+  }
+}
 @@@@@@ cross_references.ts @@@@@@
 
 // tslint:disable
diff --git a/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt b/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt
@@ -92,11 +92,26 @@ class MyComp {
 
 it('should work', () => {
   const inputs = {
-    hello: 'Damn',
-    // TODO:
+    hello: 'test',
   } as Partial<UnwrapSignalInputs<MyComp>>;
   bootstrapTemplate('<my-comp [hello]="hello">', inputs);
 });
+@@@@@@ constructor_initializations.ts @@@@@@
+
+// tslint:disable
+
+import { Directive, input } from '@angular/core';
+
+@Directive()
+export class MyComp {
+  readonly firstName = input<string>();
+
+  constructor() {
+    // TODO: Consider initializations inside the constructor.
+    // Those are not migrated right now though, as they are writes.
+    this.firstName = 'Initial value';
+  }
+}
 @@@@@@ cross_references.ts @@@@@@
 
 // tslint:disable

Original file line number	Diff line number	Diff line change
`@@ -41,8 +41,6 @@ export function pass7__migrateTemplateReferences(`
`41`	`41`	`}`
`42`	`42`	`seenFileReferences.add(fileReferenceId);`
`43`	`43`
`44`		`- // TODO: Control flow, or wait for Joost's PR?`
`45`		`-`
`46`	`44`	// Expand shorthands like `{bla}` to `{bla: bla()}`.
`47`	`45`	`const appendText = reference.from.isObjectShorthandExpression`
`48`	`46`	? `: ${reference.from.read.name}()`