fix(core): correctly perform lazy routes migration for components with additional decorators (#58796)

JoostK · thePunderWoman · commit 99715104a1a7 · 2024-11-21T16:43:30.000Z
This fixes an issue where the lazy-routes migration would crash for component classes a decorator without arguments in front of the `@Component` decorator (in particular, it needed to be the first decorator). Fixes #58793 PR Close #58796
diff --git a/packages/compiler-cli/src/ngtsc/annotations/index.ts b/packages/compiler-cli/src/ngtsc/annotations/index.ts
@@ -10,6 +10,7 @@
 
 export {
   forwardRefResolver,
+  findAngularDecorator,
   getAngularDecorators,
   isAngularDecorator,
   NoopReferencesRegistry,
diff --git a/packages/core/schematics/ng-generate/route-lazy-loading/BUILD.bazel b/packages/core/schematics/ng-generate/route-lazy-loading/BUILD.bazel
@@ -20,6 +20,8 @@ ts_library(
     deps = [
         "//packages/compiler-cli",
         "//packages/compiler-cli/private",
+        "//packages/compiler-cli/src/ngtsc/annotations",
+        "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/core/schematics/utils",
         "@npm//@angular-devkit/schematics",
         "@npm//@types/node",
diff --git a/packages/core/schematics/ng-generate/route-lazy-loading/to-lazy-routes.ts b/packages/core/schematics/ng-generate/route-lazy-loading/to-lazy-routes.ts
@@ -8,6 +8,10 @@
 
 import ts from 'typescript';
 
+import {
+  ReflectionHost,
+  TypeScriptReflectionHost,
+} from '@angular/compiler-cli/src/ngtsc/reflection/index';
 import {ChangeTracker, PendingChange} from '../../utils/change_tracker';
 
 import {findClassDeclaration} from '../../utils/typescript/class_declaration';
@@ -46,6 +50,7 @@ export function migrateFileToLazyRoutes(
   skippedRoutes: RouteMigrationData[];
 } {
   const typeChecker = program.getTypeChecker();
+  const reflector = new TypeScriptReflectionHost(typeChecker);
   const printer = ts.createPrinter();
   const tracker = new ChangeTracker(printer);
 
@@ -58,6 +63,7 @@ export function migrateFileToLazyRoutes(
   const {skippedRoutes, migratedRoutes} = migrateRoutesArray(
     routeArraysToMigrate,
     typeChecker,
+    reflector,
     tracker,
   );
 
@@ -146,6 +152,7 @@ function findRoutesArrayToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.Typ
 function migrateRoutesArray(
   routesArray: RouteData[],
   typeChecker: ts.TypeChecker,
+  reflector: ReflectionHost,
   tracker: ChangeTracker,
 ): {migratedRoutes: RouteMigrationData[]; skippedRoutes: RouteMigrationData[]} {
   const migratedRoutes: RouteMigrationData[] = [];
@@ -159,7 +166,7 @@ function migrateRoutesArray(
           migratedRoutes: migrated,
           skippedRoutes: toBeSkipped,
           importsToRemove: toBeRemoved,
-        } = migrateRoute(element, route, typeChecker, tracker);
+        } = migrateRoute(element, route, typeChecker, reflector, tracker);
         migratedRoutes.push(...migrated);
         skippedRoutes.push(...toBeSkipped);
         importsToRemove.push(...toBeRemoved);
@@ -182,6 +189,7 @@ function migrateRoute(
   element: ts.ObjectLiteralExpression,
   route: RouteData,
   typeChecker: ts.TypeChecker,
+  reflector: ReflectionHost,
   tracker: ChangeTracker,
 ): {
   migratedRoutes: RouteMigrationData[];
@@ -207,7 +215,7 @@ function migrateRoute(
           migratedRoutes: migrated,
           skippedRoutes: toBeSkipped,
           importsToRemove: toBeRemoved,
-        } = migrateRoute(childRoute, route, typeChecker, tracker);
+        } = migrateRoute(childRoute, route, typeChecker, reflector, tracker);
         migratedRoutes.push(...migrated);
         skippedRoutes.push(...toBeSkipped);
         importsToRemove.push(...toBeRemoved);
@@ -228,7 +236,7 @@ function migrateRoute(
   }
 
   // if component is not a standalone component, skip it
-  if (!isStandaloneComponent(componentDeclaration)) {
+  if (!isStandaloneComponent(componentDeclaration, reflector)) {
     skippedRoutes.push({path: routePath, file: route.routeFilePath});
     return routeMigrationResults;
   }
diff --git a/packages/core/schematics/ng-generate/route-lazy-loading/util.ts b/packages/core/schematics/ng-generate/route-lazy-loading/util.ts
@@ -7,27 +7,35 @@
  */
 
 import ts from 'typescript';
+import {findAngularDecorator} from '@angular/compiler-cli/src/ngtsc/annotations';
+import {ReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection/index';
 import {findLiteralProperty} from '../../utils/typescript/property_name';
 
 /**
  * Checks whether a component is standalone.
  * @param node Class being checked.
+ * @param reflector The reflection host to use.
  */
-export function isStandaloneComponent(node: ts.ClassDeclaration): boolean {
-  const decorator = node.modifiers?.find((m) => m.kind === ts.SyntaxKind.Decorator);
-  if (!decorator) {
+export function isStandaloneComponent(
+  node: ts.ClassDeclaration,
+  reflector: ReflectionHost,
+): boolean {
+  const decorators = reflector.getDecoratorsOfDeclaration(node);
+  if (decorators === null) {
+    return false;
+  }
+  const decorator = findAngularDecorator(decorators, 'Component', false);
+  if (decorator === undefined || decorator.args === null || decorator.args.length !== 1) {
     return false;
   }
 
-  if (ts.isCallExpression(decorator.expression)) {
-    const arg = decorator.expression.arguments[0];
-    if (ts.isObjectLiteralExpression(arg)) {
-      const property = findLiteralProperty(arg, 'standalone') as ts.PropertyAssignment;
-      if (property) {
-        return property.initializer.getText() === 'true';
-      } else {
-        return true; // standalone is true by default in v19
-      }
+  const arg = decorator.args[0];
+  if (ts.isObjectLiteralExpression(arg)) {
+    const property = findLiteralProperty(arg, 'standalone') as ts.PropertyAssignment;
+    if (property) {
+      return property.initializer.getText() === 'true';
+    } else {
+      return true; // standalone is true by default in v19
     }
   }
 
diff --git a/packages/core/schematics/test/standalone_routes_spec.ts b/packages/core/schematics/test/standalone_routes_spec.ts
@@ -733,6 +733,49 @@ describe('route lazy loading migration', () => {
     );
   });
 
+  it('should support components with additional decorators', async () => {
+    writeFile(
+      'app.module.ts',
+      `
+      import {NgModule} from '@angular/core';
+      import {RouterModule} from '@angular/router';
+      import {TestComponent} from './test';
+
+      @NgModule({
+        imports: [RouterModule.forRoot([{path: 'test', component: TestComponent}])],
+      })
+      export class AppModule {}
+    `,
+    );
+
+    writeFile(
+      'test.ts',
+      `
+      import {Component, Directive} from '@angular/core';
+
+      function OtherDecorator() {}
+
+      @OtherDecorator()
+      @Component({template: 'hello', standalone: true})
+      export class TestComponent {}
+    `,
+    );
+
+    await runMigration('route-lazy-loading');
+
+    expect(stripWhitespace(tree.readContent('app.module.ts'))).toContain(
+      stripWhitespace(`
+         import {NgModule} from '@angular/core';
+         import {RouterModule} from '@angular/router';
+
+         @NgModule({
+           imports: [RouterModule.forRoot([{path: 'test', loadComponent: () => import('./test').then(m => m.TestComponent)}])],
+         })
+         export class AppModule {}
+      `),
+    );
+  });
+
   it('should not migrate routes if the routes array doesnt have type and is not referenced', async () => {
     writeFile(
       'routes.ts',
