fix(compiler-cli): incorrectly detecting forward refs when symbol already exists in file (#48988)

crisbeto · atscott · commit 0cf11167f131 · 2023-02-07T09:00:46.000-08:00
In #48898 the `isForwardRef` flag was added to indicate whether a reference should be wrapped in a `forwardRef`. This logic assumed that the node can't be referring to another node within the same file, however from testing it looks like that's not actually the case, because we hit the same code path when an external import to the same symbol exists already. PR Close #48988
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
@@ -719,12 +719,19 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     }
     const emitted = emittedRef.expression;
     if (emitted instanceof WrappedNodeExpr) {
+      let isForwardReference = false;
+      if (emitted.node.getStart() > inContext.getStart()) {
+        const declaration = this.programDriver.getProgram()
+                                .getTypeChecker()
+                                .getTypeAtLocation(emitted.node)
+                                .getSymbol()
+                                ?.declarations?.[0];
+        if (declaration && declaration.getSourceFile() === inContext.getSourceFile()) {
+          isForwardReference = true;
+        }
+      }
       // An appropriate identifier is already in scope.
-      return {
-        kind,
-        symbolName: emitted.node.text,
-        isForwardReference: emitted.node.getStart() > inContext.getStart()
-      };
+      return {kind, symbolName: emitted.node.text, isForwardReference};
     } else if (
         emitted instanceof ExternalExpr && emitted.value.moduleName !== null &&
         emitted.value.name !== null) {
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
@@ -109,10 +109,16 @@ describe('standalone migration', () => {
     `);
 
     writeFile('/node_modules/@angular/router/index.d.ts', `
-      import {ModuleWithProviders} from '@angular/core';
+      import {ModuleWithProviders, ɵɵDirectiveDeclaration, ɵɵNgModuleDeclaration} from '@angular/core';
+
+      export declare class RouterLink {
+        // Router link is intentionally not standalone.
+        static ɵdir: ɵɵDirectiveDeclaration<RouterLink, '[routerLink]', never, {}, {}, never, never, false>;
+      }
 
       export declare class RouterModule {
         static forRoot(routes: any[], config?: any): ModuleWithProviders<RouterModule>;
+        static ɵmod: ɵɵNgModuleDeclaration<CommonModule, [typeof RouterLink], [], [typeof RouterLink]>;
       }
     `);
 
@@ -1340,6 +1346,40 @@ describe('standalone migration', () => {
     `));
   });
 
+  it('should not generate a forwardRef when adding an imported module dependency', async () => {
+    writeFile('./comp.ts', `
+      import {Component, NgModule} from '@angular/core';
+      import {RouterModule} from '@angular/router';
+
+      @Component({
+        selector: 'comp',
+        template: '<div routerLink="/"></div>'
+      })
+      export class MyComp {}
+
+      @NgModule({imports: [RouterModule], declarations: [MyComp]})
+      export class Mod {}
+    `);
+
+    await runMigration('convert-to-standalone');
+
+    expect(stripWhitespace(tree.readContent('comp.ts'))).toEqual(stripWhitespace(`
+      import {Component, NgModule} from '@angular/core';
+      import {RouterModule} from '@angular/router';
+
+      @Component({
+        selector: 'comp',
+        template: '<div routerLink="/"></div>',
+        standalone: true,
+        imports: [RouterModule]
+      })
+      export class MyComp {}
+
+      @NgModule({imports: [RouterModule, MyComp]})
+      export class Mod {}
+    `));
+  });
+
   it('should not duplicate doc strings', async () => {
     writeFile('module.ts', `
       import {NgModule, Directive} from '@angular/core';
