diff --git a/packages/core/schematics/migrations/static-queries/index.ts b/packages/core/schematics/migrations/static-queries/index.ts index 6ea16c484e..089d369d7c 100644 --- a/packages/core/schematics/migrations/static-queries/index.ts +++ b/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 []; } diff --git a/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts index c19c9a6c08..dc2c489f2c 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts +++ b/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) { diff --git a/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts index 30c783c0aa..7488db2e62 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts +++ b/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 diff --git a/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts index 404c0aa167..b530f60055 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts +++ b/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; } diff --git a/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts index 75d31337c9..735ec58faa 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts +++ b/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 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 cdb8b62061..8a97036433 100644 --- a/packages/core/schematics/test/static_queries_migration_template_spec.ts +++ b/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: ''}) + 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() => {