From 51a0b593898f2d1afd14817405695092ae39d5ea Mon Sep 17 00:00:00 2001
From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com>
Date: Tue, 30 Sep 2025 13:39:26 -0500
Subject: [PATCH] fix(migrations): handle shorthand property declarations in
 NgModule (#64160)

The migration now correctly detects shorthand declarations in NgModule metadata

PR Close #64160
---
 .../standalone-migration/to-standalone.ts     | 128 +++++-----
 .../test/standalone_migration_spec.ts         | 223 ++++++++++++++++++
 2 files changed, 298 insertions(+), 53 deletions(-)

diff --git a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
index aba57cb5d9c..3af575d32f2 100644
--- a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
+++ b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
@@ -314,33 +314,43 @@ function moveDeclarationsToImports(
   );
 
   // Separate the declarations that we want to keep and ones we need to copy into the `imports`.
-  if (ts.isPropertyAssignment(declarationsProp)) {
-    // If the declarations are an array, we can analyze it to
-    // find any classes from the current migration.
-    if (ts.isArrayLiteralExpression(declarationsProp.initializer)) {
-      for (const el of declarationsProp.initializer.elements) {
-        if (ts.isIdentifier(el)) {
-          const correspondingClass = findClassDeclaration(el, typeChecker);
+  if (
+    ts.isPropertyAssignment(declarationsProp) ||
+    ts.isShorthandPropertyAssignment(declarationsProp)
+  ) {
+    // Handle both regular and shorthand property assignments
+    if (ts.isPropertyAssignment(declarationsProp)) {
+      // If the declarations are an array, we can analyze it to
+      // find any classes from the current migration.
+      if (ts.isArrayLiteralExpression(declarationsProp.initializer)) {
+        for (const el of declarationsProp.initializer.elements) {
+          if (ts.isIdentifier(el)) {
+            const correspondingClass = findClassDeclaration(el, typeChecker);
 
-          if (
-            !correspondingClass ||
-            // Check whether the declaration is either standalone already or is being converted
-            // in this migration. We need to check if it's standalone already, in order to correct
-            // some cases where the main app and the test files are being migrated in separate
-            // programs.
-            isStandaloneDeclaration(correspondingClass, allDeclarations, templateTypeChecker)
-          ) {
-            declarationsToCopy.push(el);
+            if (
+              !correspondingClass ||
+              // Check whether the declaration is either standalone already or is being converted
+              // in this migration. We need to check if it's standalone already, in order to correct
+              // some cases where the main app and the test files are being migrated in separate
+              // programs.
+              isStandaloneDeclaration(correspondingClass, allDeclarations, templateTypeChecker)
+            ) {
+              declarationsToCopy.push(el);
+            } else {
+              declarationsToPreserve.push(el);
+            }
           } else {
-            declarationsToPreserve.push(el);
+            declarationsToCopy.push(el);
           }
-        } else {
-          declarationsToCopy.push(el);
         }
+      } else {
+        // Otherwise create a spread that will be copied into the `imports`.
+        declarationsToCopy.push(ts.factory.createSpreadElement(declarationsProp.initializer));
       }
     } else {
-      // Otherwise create a spread that will be copied into the `imports`.
-      declarationsToCopy.push(ts.factory.createSpreadElement(declarationsProp.initializer));
+      // For shorthand properties, treat them as unanalyzable and use spread syntax
+      // shorthand properties were being ignored, now they're detected and treated as spreads
+      declarationsToCopy.push(ts.factory.createSpreadElement(declarationsProp.name));
     }
   }
 
@@ -360,7 +370,7 @@ function moveDeclarationsToImports(
   }
 
   for (const prop of literal.properties) {
-    if (!isNamedPropertyAssignment(prop)) {
+    if (!isNamedPropertyAssignment(prop) && !ts.isShorthandPropertyAssignment(prop)) {
       properties.push(prop);
       continue;
     }
@@ -368,12 +378,13 @@ function moveDeclarationsToImports(
     // If we have declarations to preserve, update the existing property, otherwise drop it.
     if (prop === declarationsProp) {
       if (declarationsToPreserve.length > 0) {
-        const hasTrailingComma = ts.isArrayLiteralExpression(prop.initializer)
-          ? prop.initializer.elements.hasTrailingComma
-          : hasAnyArrayTrailingComma;
+        const hasTrailingComma =
+          ts.isPropertyAssignment(prop) && ts.isArrayLiteralExpression(prop.initializer)
+            ? prop.initializer.elements.hasTrailingComma
+            : hasAnyArrayTrailingComma;
+
         properties.push(
-          ts.factory.updatePropertyAssignment(
-            prop,
+          ts.factory.createPropertyAssignment(
             prop.name,
             ts.factory.createArrayLiteralExpression(
               ts.factory.createNodeArray(
@@ -390,29 +401,32 @@ function moveDeclarationsToImports(
     // If we have an `imports` array and declarations
     // that should be copied, we merge the two arrays.
     if (prop === importsProp && declarationsToCopy.length > 0) {
-      let initializer: ts.Expression;
+      // Only regular property assignments have initializers that we can merge
+      if (ts.isPropertyAssignment(prop)) {
+        let initializer: ts.Expression;
 
-      if (ts.isArrayLiteralExpression(prop.initializer)) {
-        initializer = ts.factory.updateArrayLiteralExpression(
-          prop.initializer,
-          ts.factory.createNodeArray(
-            [...prop.initializer.elements, ...declarationsToCopy],
-            prop.initializer.elements.hasTrailingComma,
-          ),
-        );
-      } else {
-        initializer = ts.factory.createArrayLiteralExpression(
-          ts.factory.createNodeArray(
-            [ts.factory.createSpreadElement(prop.initializer), ...declarationsToCopy],
-            // Expect the declarations to be greater than 1 since
-            // we have the pre-existing initializer already.
-            hasAnyArrayTrailingComma && declarationsToCopy.length > 1,
-          ),
-        );
+        if (ts.isArrayLiteralExpression(prop.initializer)) {
+          initializer = ts.factory.updateArrayLiteralExpression(
+            prop.initializer,
+            ts.factory.createNodeArray(
+              [...prop.initializer.elements, ...declarationsToCopy],
+              prop.initializer.elements.hasTrailingComma,
+            ),
+          );
+        } else {
+          initializer = ts.factory.createArrayLiteralExpression(
+            ts.factory.createNodeArray(
+              [ts.factory.createSpreadElement(prop.initializer), ...declarationsToCopy],
+              // Expect the declarations to be greater than 1 since
+              // we have the pre-existing initializer already.
+              hasAnyArrayTrailingComma && declarationsToCopy.length > 1,
+            ),
+          );
+        }
+
+        properties.push(ts.factory.updatePropertyAssignment(prop, prop.name, initializer));
+        continue;
       }
-
-      properties.push(ts.factory.updatePropertyAssignment(prop, prop.name, initializer));
-      continue;
     }
 
     // Retain any remaining properties.
@@ -566,11 +580,17 @@ export function findImportLocation(
  * E.g. `declarations: [Foo]` or `declarations: SOME_VAR` would match this description,
  * but not `declarations: []`.
  */
-function hasNgModuleMetadataElements(node: ts.Node): node is ts.PropertyAssignment {
-  return (
-    ts.isPropertyAssignment(node) &&
-    (!ts.isArrayLiteralExpression(node.initializer) || node.initializer.elements.length > 0)
-  );
+function hasNgModuleMetadataElements(
+  node: ts.Node,
+): node is ts.PropertyAssignment | ts.ShorthandPropertyAssignment {
+  if (ts.isPropertyAssignment(node)) {
+    return !ts.isArrayLiteralExpression(node.initializer) || node.initializer.elements.length > 0;
+  }
+  if (ts.isShorthandPropertyAssignment(node)) {
+    // For shorthand properties, we assume they have elements since they reference a variable
+    return true;
+  }
+  return false;
 }
 
 /** Finds all modules whose declarations can be migrated. */
@@ -817,6 +837,7 @@ function analyzeTestingModules(
     const importElements =
       importsProp &&
       hasNgModuleMetadataElements(importsProp) &&
+      ts.isPropertyAssignment(importsProp) &&
       ts.isArrayLiteralExpression(importsProp.initializer)
         ? importsProp.initializer.elements.filter((el) => {
             // Filter out calls since they may be a `ModuleWithProviders`.
@@ -879,6 +900,7 @@ function extractDeclarationsFromTestObject(
   if (
     declarations &&
     hasNgModuleMetadataElements(declarations) &&
+    ts.isPropertyAssignment(declarations) &&
     ts.isArrayLiteralExpression(declarations.initializer)
   ) {
     for (const element of declarations.initializer.elements) {
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
index 29bf7421fd1..288a3ca9a47 100644
--- a/packages/core/schematics/test/standalone_migration_spec.ts
+++ b/packages/core/schematics/test/standalone_migration_spec.ts
@@ -5123,4 +5123,227 @@ describe('standalone migration', () => {
     `),
     );
   });
+
+  it('should handle shorthand property assignment in module declarations', async () => {
+    writeFile(
+      'module.ts',
+      `
+      import {NgModule, Directive, Component} from '@angular/core';
+
+      @Directive({selector: '[dir]', standalone: false})
+      export class BuzzComponent {}
+
+      @Component({selector: 'fizz', template: 'fizz', standalone: false})
+      export class FizzComponent {}
+
+      @Component({selector: 'bar', template: 'bar', standalone: true})
+      export class BarComponent {}
+
+      @Component({selector: 'foo', template: 'foo', standalone: true})
+      export class FooComponent {}
+
+      const declarations = [BuzzComponent, FizzComponent];
+
+      @NgModule({
+        declarations,
+        imports: [BarComponent, FooComponent],
+        exports: [BarComponent, BuzzComponent, FizzComponent, FooComponent],
+      })
+      export class SharedModule {}
+    `,
+    );
+
+    await runMigration('convert-to-standalone');
+
+    const result = tree.readContent('module.ts');
+
+    expect(stripWhitespace(result)).toContain(stripWhitespace(`@Directive({selector: '[dir]'})`));
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@Component({selector: 'fizz', template: 'fizz'})`),
+    );
+
+    expect(result).not.toContain('standalone: false');
+
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@NgModule({
+        imports: [BarComponent, FooComponent, ...declarations],
+        exports: [BarComponent, BuzzComponent, FizzComponent, FooComponent],
+      })`),
+    );
+
+    expect(result).toContain('const declarations = [BuzzComponent, FizzComponent];');
+
+    expect(result).toContain('exports: [BarComponent, BuzzComponent, FizzComponent, FooComponent]');
+  });
+
+  it('should handle shorthand property assignment with mixed imports and declarations', async () => {
+    writeFile(
+      'module.ts',
+      `
+      import {NgModule, Component} from '@angular/core';
+      import {NgIf} from '@angular/common';
+
+      @Component({selector: 'my-comp', template: '<div *ngIf="show">Content</div>', standalone: false})
+      export class MyComponent {}
+
+      @Component({selector: 'other-comp', template: 'other', standalone: false})
+      export class OtherComponent {}
+
+      const declarations = [MyComponent, OtherComponent];
+      const imports = [NgIf];
+
+      @NgModule({
+        declarations,
+        imports,
+        exports: [MyComponent, OtherComponent],
+      })
+      export class TestModule {}
+    `,
+    );
+
+    await runMigration('convert-to-standalone');
+
+    const result = tree.readContent('module.ts');
+
+    expect(result).toContain(`import {NgIf} from '@angular/common'`);
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@Component({
+        selector: 'my-comp',
+        template: '<div *ngIf="show">Content</div>',
+        imports: [NgIf]
+      })`),
+    );
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@Component({selector: 'other-comp', template: 'other'})`),
+    );
+
+    expect(result).not.toContain('standalone: false');
+
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@NgModule({
+        imports,
+        exports: [MyComponent, OtherComponent],
+      })`),
+    );
+
+    expect(result).toContain('const declarations = [MyComponent, OtherComponent];');
+    expect(result).toContain('const imports = [NgIf];');
+  });
+
+  it('should handle shorthand property assignment with spread elements in declarations', async () => {
+    writeFile(
+      'module.ts',
+      `
+      import {NgModule, Component} from '@angular/core';
+
+      @Component({selector: 'comp1', template: 'comp1', standalone: false})
+      export class Component1 {}
+
+      @Component({selector: 'comp2', template: 'comp2', standalone: false})  
+      export class Component2 {}
+
+      const staticDeclarationsA = [Component1];
+      const staticDeclarationsB = [Component2];
+      const declarations = [...staticDeclarationsA, ...staticDeclarationsB];
+
+      @NgModule({
+        declarations,
+        exports: [Component1, Component2],
+      })
+      export class MixedModule {}
+    `,
+    );
+
+    await runMigration('convert-to-standalone');
+
+    const result = tree.readContent('module.ts');
+
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@Component({selector: 'comp1', template: 'comp1'})`),
+    );
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@Component({selector: 'comp2', template: 'comp2'})`),
+    );
+    expect(result).not.toContain('standalone: false');
+
+    expect(stripWhitespace(result)).toContain(
+      stripWhitespace(`@NgModule({
+        imports: [...declarations],
+        exports: [Component1, Component2],
+      })`),
+    );
+
+    expect(result).toContain('const staticDeclarationsA = [Component1];');
+    expect(result).toContain('const staticDeclarationsB = [Component2];');
+    expect(result).toContain(
+      'const declarations = [...staticDeclarationsA, ...staticDeclarationsB];',
+    );
+  });
+
+  it('should handle both regular and shorthand property assignments in the same migration', async () => {
+    writeFile(
+      'regular-module.ts',
+      `
+      import {NgModule, Component} from '@angular/core';
+
+      @Component({selector: 'regular-comp', template: 'regular', standalone: false})
+      export class RegularComponent {}
+
+      @NgModule({
+        declarations: [RegularComponent],
+        exports: [RegularComponent],
+      })
+      export class RegularModule {}
+    `,
+    );
+
+    writeFile(
+      'shorthand-module.ts',
+      `
+      import {NgModule, Component} from '@angular/core';
+
+      @Component({selector: 'shorthand-comp', template: 'shorthand', standalone: false})
+      export class ShorthandComponent {}
+
+      const declarations = [ShorthandComponent];
+
+      @NgModule({
+        declarations,
+        exports: [ShorthandComponent],
+      })
+      export class ShorthandModule {}
+    `,
+    );
+
+    await runMigration('convert-to-standalone');
+
+    const regularResult = tree.readContent('regular-module.ts');
+    const shorthandResult = tree.readContent('shorthand-module.ts');
+
+    expect(stripWhitespace(regularResult)).toContain(
+      stripWhitespace(`@Component({selector: 'regular-comp', template: 'regular'})`),
+    );
+    expect(stripWhitespace(regularResult)).toContain(
+      stripWhitespace(`@NgModule({
+        imports: [RegularComponent],
+        exports: [RegularComponent],
+      })`),
+    );
+
+    expect(stripWhitespace(shorthandResult)).toContain(
+      stripWhitespace(`@Component({selector: 'shorthand-comp', template: 'shorthand'})`),
+    );
+
+    expect(stripWhitespace(shorthandResult)).toContain(
+      stripWhitespace(`@NgModule({
+        imports: [...declarations],
+        exports: [ShorthandComponent],
+      })`),
+    );
+
+    expect(shorthandResult).toContain('const declarations = [ShorthandComponent];');
+
+    expect(regularResult).not.toContain('standalone: false');
+    expect(shorthandResult).not.toContain('standalone: false');
+  });
 });