fix(core): static-query migration should gracefully exit if AOT compi…

…ler throws (#30269)

The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.

Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.

PR Close #30269

packages/core/schematics/migrations/static-queries/index.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {logging} from '@angular-devkit/core';
 import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
 import {dirname, relative} from 'path';
 import {from} from 'rxjs';
@@ -75,7 +76,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
   if (buildProjects.size) {
     const strategy = await promptForMigrationStrategy(logger);
     for (let project of Array.from(buildProjects.values())) {
-      failures.push(...await runStaticQueryMigration(tree, project, strategy));
+      failures.push(...await runStaticQueryMigration(tree, project, strategy, logger));
     }
   }
 
@@ -84,7 +85,8 @@ async function runMigration(tree: Tree, context: SchematicContext) {
   for (const tsconfigPath of testPaths) {
     const project = await analyzeProject(tree, tsconfigPath, basePath);
     if (project) {
-      failures.push(...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS));
+      failures.push(
+          ...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS, logger));
     }
   }
 
@@ -139,7 +141,8 @@ function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
  * not need to be static and can be set up with "static: false".
  */
 async function runStaticQueryMigration(
-    tree: Tree, project: AnalyzedProject, selectedStrategy: SELECTED_STRATEGY) {
+    tree: Tree, project: AnalyzedProject, selectedStrategy: SELECTED_STRATEGY,
+    logger: logging.LoggerApi) {
   const {sourceFiles, typeChecker, host, queryVisitor, tsconfigPath, basePath} = project;
   const printer = ts.createPrinter();
   const failureMessages: string[] = [];
@@ -173,10 +176,25 @@ async function runStaticQueryMigration(
     strategy = new QueryTemplateStrategy(tsconfigPath, classMetadata, host);
   }
 
-  // In case the strategy could not be set up properly, we just exit the
-  // migration. We don't want to throw an exception as this could mean
-  // that other migrations are interrupted.
-  if (!strategy.setup()) {
+  try {
+    strategy.setup();
+  } catch (e) {
+    // In case the strategy could not be set up properly, we just exit the
+    // migration. We don't want to throw an exception as this could mean
+    // that other migrations are interrupted.
+    logger.warn(
+        `Could not setup migration strategy for "${project.tsconfigPath}". The ` +
+        `following error has been reported:`);
+    if (selectedStrategy === SELECTED_STRATEGY.TEMPLATE) {
+      logger.warn(
+          `The template migration strategy uses the Angular compiler ` +
+          `internally and therefore projects that no longer build successfully after ` +
+          `the update cannot use the template migration strategy. Please ensure ` +
+          `there are no AOT compilation errors.`);
+    }
+    logger.error(e);
+    logger.info(
+        'Migration can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"');
     return [];
   }
 

packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts

@@ -63,14 +63,12 @@ export class QueryTemplateStrategy implements TimingStrategy {
     ];
 
     if (ngDiagnostics.length) {
-      this._printDiagnosticFailures(ngDiagnostics);
-      return false;
+      throw this._createDiagnosticsError(ngDiagnostics);
     }
 
     analyzedModules.files.forEach(file => {
       file.directives.forEach(directive => this._analyzeDirective(directive, analyzedModules));
     });
-    return true;
   }
 
   /** Analyzes a given directive by determining the timing of all matched view queries. */
@@ -180,12 +178,12 @@ export class QueryTemplateStrategy implements TimingStrategy {
         .template;
   }
 
-  private _printDiagnosticFailures(diagnostics: (ts.Diagnostic|Diagnostic)[]) {
-    console.error('Could not create Angular AOT compiler to determine query timing.');
-    console.error('The following diagnostics were detected:\n');
-    console.error(diagnostics.map(d => d.messageText).join(`\n`));
-    console.error('Please make sure that there is no compilation failure. The migration');
-    console.error('can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"');
+  private _createDiagnosticsError(diagnostics: (ts.Diagnostic|Diagnostic)[]) {
+    return new Error(
+        `Could not create Angular AOT compiler to determine query timing.\n` +
+        `The following diagnostics were detected:\n` +
+        `${diagnostics.map(d => d.messageText).join(`\n `)}\n` +
+        `Please make sure that there is no AOT compilation failure.`);
   }
 
   private _getViewQueryUniqueKey(filePath: string, className: string, propName: string) {

packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts

@@ -16,7 +16,7 @@ import {TimingResult, TimingStrategy} from '../timing-strategy';
  * of detecting the timing of queries based on how they are used in tests.
  */
 export class QueryTestStrategy implements TimingStrategy {
-  setup() { return true; }
+  setup() {}
 
   /**
    * Detects the timing for a given query. For queries within tests, we always

packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts

@@ -9,8 +9,8 @@
 import {NgQueryDefinition, QueryTiming} from '../angular/query-definition';
 
 export interface TimingStrategy {
-  /** Sets up the given strategy. Should return false if the strategy could not be set up. */
-  setup(): boolean;
+  /** Sets up the given strategy. Throws if the strategy could not be set up. */
+  setup(): void;
   /** Detects the timing result for a given query. */
   detectTiming(query: NgQueryDefinition): TimingResult;
 }

packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts

@@ -37,11 +37,7 @@ const STATIC_QUERY_LIFECYCLE_HOOKS = {
 export class QueryUsageStrategy implements TimingStrategy {
   constructor(private classMetadata: ClassMetadataMap, private typeChecker: ts.TypeChecker) {}
 
-  setup() {
-    // No setup is needed for this strategy and therefore we always return "true" as
-    // the setup is successful.
-    return true;
-  }
+  setup() {}
 
   /**
    * Analyzes the usage of the given query and determines the query timing based

packages/core/schematics/test/static_queries_migration_template_spec.ts

@@ -19,6 +19,7 @@ describe('static-queries migration with template strategy', () => {
   let tmpDirPath: string;
   let previousWorkingDir: string;
   let warnOutput: string[];
+  let errorOutput: string[];
 
   beforeEach(() => {
     runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
@@ -36,9 +37,12 @@ describe('static-queries migration with template strategy', () => {
     }));
 
     warnOutput = [];
+    errorOutput = [];
     runner.logger.subscribe(logEntry => {
       if (logEntry.level === 'warn') {
         warnOutput.push(logEntry.message);
+      } else if (logEntry.level === 'error') {
+        errorOutput.push(logEntry.message);
       }
     });
 
@@ -457,17 +461,55 @@ describe('static-queries migration with template strategy', () => {
         // **NOTE**: Analysis will fail as there is no "NgModule" that declares the component.
       `);
 
-      spyOn(console, 'error');
-
       // We don't expect an error to be thrown as this could interrupt other
       // migrations which are scheduled with "ng update" in the CLI.
       await runMigration();
 
-      expect(console.error)
-          .toHaveBeenCalledWith('Could not create Angular AOT compiler to determine query timing.');
-      expect(console.error)
-          .toHaveBeenCalledWith(
-              jasmine.stringMatching(/Cannot determine the module for class MyComp/));
+      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/);
+    });
+
+    it('should gracefully exit migration if AOT compiler throws exception', async() => {
+      writeFile('/my-component.ts', `
+        import {Component, ViewChild} from '@angular/core';
+
+        @Component({template: '<span #test></span>'})
+        export class MyComp {
+          @ViewChild('test') query: any;
+        }
+      `);
+      writeFile('/app-module.ts', `
+        import {NgModule} from '@angular/core';
+        import {MyComp} from './components';
+
+        @NgModule({declarations: [MyComp]})
+        export class MyModule {}
+      `);
+
+      writeFile('/components.ts', `export * from './my-component';`);
+      writeFile('/index.ts', `export * from './app-module';`);
+
+      // Enable flat-module bundling in order to simulate a common AOT compiler
+      // failure that can happen in CLI projects that use flat-module bundling
+      // e.g. with ng-packagr. https://github.com/angular/angular/issues/20931
+      writeFile('/tsconfig.json', JSON.stringify({
+        compilerOptions: {
+          experimentalDecorators: true,
+          lib: ['es2015'],
+        },
+        angularCompilerOptions: {
+          flatModuleId: 'flat-module',
+          flatModuleOutFile: 'flat-module-bundle.js',
+        },
+        files: ['index.ts']
+      }));
+
+      await runMigration();
+
+      expect(errorOutput.length).toBe(1);
+      expect(errorOutput[0]).toMatch(/^TypeError: Cannot read property 'module' of undefined/);
     });
 
     it('should add a todo for content queries which are not detectable', async() => {