From 0cdf5980e299d70434f410421966be403f7f09c3 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Tue, 14 May 2019 20:35:27 +0200
Subject: [PATCH] fix(core): static-query migration should not fallback to test
 strategy (#30458)

Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.

PR Close #30458
---
 .../migrations/static-queries/index.ts        | 19 +++++++--
 .../static_queries_migration_template_spec.ts | 42 ++++++++++++++++---
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/packages/core/schematics/migrations/static-queries/index.ts b/packages/core/schematics/migrations/static-queries/index.ts
index efc8a5f525..19bdba3934 100644
--- a/packages/core/schematics/migrations/static-queries/index.ts
+++ b/packages/core/schematics/migrations/static-queries/index.ts
@@ -61,11 +61,12 @@ async function runMigration(tree: Tree, context: SchematicContext) {
         'to explicit timing.');
   }
 
+  const analyzedFiles = new Set<string>();
   const buildProjects = new Set<AnalyzedProject>();
   const failures = [];
 
   for (const tsconfigPath of buildPaths) {
-    const project = analyzeProject(tree, tsconfigPath, basePath);
+    const project = analyzeProject(tree, tsconfigPath, basePath, analyzedFiles);
     if (project) {
       buildProjects.add(project);
     }
@@ -83,7 +84,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
   // For the "test" tsconfig projects we always want to use the test strategy as
   // we can't detect the proper timing within spec files.
   for (const tsconfigPath of testPaths) {
-    const project = await analyzeProject(tree, tsconfigPath, basePath);
+    const project = await analyzeProject(tree, tsconfigPath, basePath, analyzedFiles);
     if (project) {
       failures.push(
           ...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS, logger));
@@ -103,7 +104,8 @@ async function runMigration(tree: Tree, context: SchematicContext) {
  * Analyzes the given TypeScript project by looking for queries that need to be
  * migrated. In case there are no queries that can be migrated, null is returned.
  */
-function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
+function analyzeProject(
+    tree: Tree, tsconfigPath: string, basePath: string, analyzedFiles: Set<string>):
     AnalyzedProject|null {
       const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
       const host = ts.createCompilerHost(parsed.options, true);
@@ -125,7 +127,16 @@ function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
 
       // Analyze all project source-files and collect all queries that
       // need to be migrated.
-      sourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));
+      sourceFiles.forEach(sourceFile => {
+        const relativePath = relative(basePath, sourceFile.fileName);
+
+        // Only look for queries within the current source files if the
+        // file has not been analyzed before.
+        if (!analyzedFiles.has(relativePath)) {
+          analyzedFiles.add(relativePath);
+          queryVisitor.visitNode(sourceFile);
+        }
+      });
 
       if (queryVisitor.resolvedQueries.size === 0) {
         return null;
diff --git a/packages/core/schematics/test/static_queries_migration_template_spec.ts b/packages/core/schematics/test/static_queries_migration_template_spec.ts
index e8f6b96815..31483df180 100644
--- a/packages/core/schematics/test/static_queries_migration_template_spec.ts
+++ b/packages/core/schematics/test/static_queries_migration_template_spec.ts
@@ -515,8 +515,6 @@ describe('static-queries migration with template strategy', () => {
       await runMigration();
 
       expect(errorOutput.length).toBe(1);
-      expect(errorOutput[0])
-          .toMatch(/^Error: Could not create Angular AOT compiler to determine query timing./);
       expect(errorOutput[0]).toMatch(/Cannot determine the module for class MyComp/);
     });
 
@@ -673,15 +671,49 @@ describe('static-queries migration with template strategy', () => {
         export class MyModule {}
       `);
 
-      spyOn(console, 'error').and.callThrough();
-
       await runMigration();
 
-      expect(console.error).toHaveBeenCalledTimes(0);
+      expect(errorOutput.length).toBe(0);
       expect(tree.readContent('/src/test.ts'))
           .toContain(`@ViewChild('test', /* TODO: add static flag */ {}) query: any;`);
       expect(tree.readContent('/src/app.component.ts'))
           .toContain(`@ViewChild('test', { static: true }) query: any;`);
     });
+
+    it('should not fall back to test strategy if selected strategy fails', async() => {
+      writeFile('/src/tsconfig.spec.json', JSON.stringify({
+        compilerOptions: {
+          experimentalDecorators: true,
+          lib: ['es2015'],
+        },
+        files: [
+          'test.ts',
+        ],
+      }));
+
+      writeFile('/src/test.ts', `import * as mod from './app.module';`);
+      writeFile('/src/app.component.ts', `
+        import {Component, ViewChild} from '@angular/core';
+
+        @Component({template: '<span #test>Test</span>'})
+        export class AppComponent {
+          @ViewChild('test') query: any;
+        }
+      `);
+
+      writeFile('/src/app.module.ts', `
+        import {NgModule} from '@angular/core';
+        import {AppComponent} from './app.component';
+
+        @NgModule({declarations: [AppComponent, ThisCausesAnError]})
+        export class MyModule {}
+      `);
+
+      await runMigration();
+
+      expect(errorOutput.length).toBe(1);
+      expect(errorOutput[0]).toMatch(/Unexpected value 'undefined'/);
+      expect(tree.readContent('/src/app.component.ts')).toContain(`@ViewChild('test') query: any;`);
+    });
   });
 });