fix(core): static-query schematic should detect static queries in get…

…ters. (#29609) Queries can also be statically accessed within getters. e.g. ```ts ngOnInit() { this.myQueryGetter.doSomething(); } ``` In that case we need to check if the `myQueryGetter` definition accesses a query statically. As we need to use the type checker for every property acess within lifecylce hooks, the schematic might become slower than before, but considering that this is a one-time execution, it is totally fine using the type-checker extensively. PR Close #29609
angular · Apr 1, 2019 · 33016b8 · 33016b8
1 parent b2c03b8
commit 33016b8
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 0 deletions.
diff --git a/packages/core/schematics/migrations/static-queries/angular/declaration_usage_visitor.ts b/packages/core/schematics/migrations/static-queries/angular/declaration_usage_visitor.ts
@@ -74,6 +74,25 @@ export class DeclarationUsageVisitor {
     }
   }
 
+  private visitPropertyAccessExpression(node: ts.PropertyAccessExpression, nodeQueue: ts.Node[]) {
+    const propertySymbol = this.typeChecker.getSymbolAtLocation(node.name);
+
+    if (!propertySymbol || !propertySymbol.valueDeclaration ||
+        this.visitedJumpExprSymbols.has(propertySymbol)) {
+      return;
+    }
+
+    const valueDeclaration = propertySymbol.valueDeclaration;
+
+    // In case the property access expression refers to a get accessor, we need to visit
+    // the body of the get accessor declaration as there could be logic that uses the
+    // given search node synchronously.
+    if (ts.isGetAccessorDeclaration(valueDeclaration) && valueDeclaration.body) {
+      this.visitedJumpExprSymbols.add(propertySymbol);
+      nodeQueue.push(valueDeclaration.body);
+    }
+  }
+
   isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
     const nodeQueue: ts.Node[] = [searchNode];
     this.visitedJumpExprSymbols.clear();
@@ -97,6 +116,12 @@ export class DeclarationUsageVisitor {
         this.addNewExpressionToQueue(node, nodeQueue);
       }
 
+      // Handle property access expressions. These could resolve to get-accessor declarations
+      // which can contain synchronous logic that accesses the search node.
+      if (ts.isPropertyAccessExpression(node)) {
+        this.visitPropertyAccessExpression(node, nodeQueue);
+      }
+
       // Do not visit nodes that declare a block of statements but are not executed
       // synchronously (e.g. function declarations). We only want to check TypeScript
       // nodes which are synchronously executed in the control flow.

diff --git a/packages/core/schematics/test/static_queries_migration_spec.ts b/packages/core/schematics/test/static_queries_migration_spec.ts
@@ -780,6 +780,67 @@ describe('static-queries migration', () => {
           .toContain(`@${queryType}('test', { static: true }) query: any;`);
     });
 
+    it('should detect static queries used through getter property access', () => {
+      writeFile('/index.ts', `
+        import {Component, ${queryType}} from '@angular/core';
+                        
+        @Component({template: '<span #test></span>'})
+        export class MyComp {
+          private @${queryType}('test') query: any;
+                
+          get myProp() {
+            return this.query.myValue;
+          }
+          
+          ngOnInit() {
+            this.myProp.test();
+          }
+        }
+      `);
+
+      runMigration();
+
+      expect(tree.readContent('/index.ts'))
+          .toContain(`@${queryType}('test', { static: true }) query: any;`);
+    });
+
+    it('should detect static queries used through external getter access', () => {
+      writeFile('/index.ts', `
+        import {Component, ${queryType}} from '@angular/core';
+        import {External} from './external';
+                        
+        @Component({template: '<span #test></span>'})
+        export class MyComp {
+          @${queryType}('test') query: any;
+          
+          private external = new External(this);
+                
+          get myProp() {
+            return this.query.myValue;
+          }
+          
+          ngOnInit() {
+            console.log(this.external.query);
+          }
+        }
+      `);
+
+      writeFile('/external.ts', `
+        import {MyComp} from './index';
+      
+        export class External {
+          constructor(private comp: MyComp) {}
+          
+          get query() { return this.comp.query; }
+        }
+      `);
+
+      runMigration();
+
+      expect(tree.readContent('/index.ts'))
+          .toContain(`@${queryType}('test', { static: true }) query: any;`);
+    });
+
     it('should properly handle multiple tsconfig files', () => {
       writeFile('/src/index.ts', `
         import {Component, ${queryType}} from '@angular/core';