refactor(core): static-query migration should warn about syntax failures (#30628)

Currently if a project has source-files with syntax failures and the migration
has been started on a broken project, we silently migrate *as much as possible*,
but never notify the developer that the project was in a broken state and that
he can re-run the migration after the failures are fixed.

Additionally the template strategy does not need to exit gracefully if it detects
Angular semantic diagnostics on generated files (template type checking). These
diagnostics are not relevant for the query timing analysis.

PR Close #30628
This commit is contained in:
Paul Gschwendtner 2019-05-23 02:56:27 +02:00 committed by Jason Aden
parent 5640cedc82
commit 132c61dfd4
3 changed files with 133 additions and 13 deletions

View File

@ -74,7 +74,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
SELECTED_STRATEGY.TEMPLATE;
for (const tsconfigPath of buildPaths) {
const project = analyzeProject(tree, tsconfigPath, basePath, analyzedFiles);
const project = analyzeProject(tree, tsconfigPath, basePath, analyzedFiles, logger);
if (project) {
buildProjects.add(project);
}
@ -89,7 +89,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, analyzedFiles);
const project = await analyzeProject(tree, tsconfigPath, basePath, analyzedFiles, logger);
if (project) {
failures.push(
...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS, logger));
@ -111,7 +111,8 @@ async function runMigration(tree: Tree, context: SchematicContext) {
* migrated. In case there are no queries that can be migrated, null is returned.
*/
function analyzeProject(
tree: Tree, tsconfigPath: string, basePath: string, analyzedFiles: Set<string>):
tree: Tree, tsconfigPath: string, basePath: string, analyzedFiles: Set<string>,
logger: logging.LoggerApi):
AnalyzedProject|null {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = ts.createCompilerHost(parsed.options, true);
@ -126,6 +127,20 @@ function analyzeProject(
};
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const syntacticDiagnostics = program.getSyntacticDiagnostics();
// Syntactic TypeScript errors can throw off the query analysis and therefore we want
// to notify the developer that we couldn't analyze parts of the project. Developers
// can just re-run the migration after fixing these failures.
if (syntacticDiagnostics.length) {
logger.warn(
`\nTypeScript project "${tsconfigPath}" has syntactical errors which could cause ` +
`an incomplete migration. Please fix the following failures and rerun the migration:`);
logger.error(ts.formatDiagnostics(syntacticDiagnostics, host));
logger.info(
'Migration can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"\n');
}
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
@ -198,7 +213,7 @@ async function runStaticQueryMigration(
} catch (e) {
if (selectedStrategy === SELECTED_STRATEGY.TEMPLATE) {
logger.warn(
`The template migration strategy uses the Angular compiler ` +
`\nThe 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.\n`);

View File

@ -57,13 +57,9 @@ export class QueryTemplateStrategy implements TimingStrategy {
// used to determine the timing for registered queries.
const analyzedModules = (aotProgram as any)['analyzedModules'] as NgAnalyzedModules;
const ngDiagnostics = [
...aotProgram.getNgStructuralDiagnostics(),
...aotProgram.getNgSemanticDiagnostics(),
];
if (ngDiagnostics.length) {
throw this._createDiagnosticsError(ngDiagnostics);
const ngStructuralDiagnostics = aotProgram.getNgStructuralDiagnostics();
if (ngStructuralDiagnostics.length) {
throw this._createDiagnosticsError(ngStructuralDiagnostics);
}
analyzedModules.files.forEach(file => {
@ -178,8 +174,8 @@ export class QueryTemplateStrategy implements TimingStrategy {
.template;
}
private _createDiagnosticsError(diagnostics: (ts.Diagnostic|Diagnostic)[]) {
return new Error(`${diagnostics.map(d => d.messageText).join(`\n `)}`);
private _createDiagnosticsError(diagnostics: ReadonlyArray<Diagnostic>) {
return new Error(ts.formatDiagnostics(diagnostics as ts.Diagnostic[], this.host));
}
private _getViewQueryUniqueKey(filePath: string, className: string, propName: string) {

View File

@ -498,6 +498,115 @@ describe('static-queries migration with template strategy', () => {
/^⮑ {3}index.ts@5:11: Multiple components use the query with different timings./);
});
it('should be able to migrate an application with type checking failure which ' +
'does not affect analysis',
async() => {
// Fakes the `@angular/package` by creating a `ViewChild` decorator
// function that requires developers to specify the "static" flag.
writeFile('/node_modules/@angular/core/index.d.ts', `
export interface ViewChildDecorator {
(selector: Type<any> | Function | string, opts: {
static: boolean;
read?: any;
}): any;
}
export declare const ViewChild: ViewChildDecorator;
`);
writeFile('/index.ts', `
import {NgModule, Component, ViewChild} from '@angular/core';
@Component({
template: '<ng-template><p #myRef></p></ng-template>'
})
export class MyComp {
@ViewChild('myRef') query: any;
}
`);
writeFile('/my-module.ts', `
import {NgModule} from '@angular/core';
import {MyComp} from './index';
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
await runMigration();
expect(errorOutput.length).toBe(0);
expect(tree.readContent('/index.ts'))
.toContain(`@ViewChild('myRef', { static: false }) query: any;`);
});
it('should be able to migrate applications with template type checking failure ' +
'which does not affect analysis',
async() => {
writeFile('/index.ts', `
import {NgModule, Component, ViewChild} from '@angular/core';
@Component({
template: '<p #myRef>{{myVar.hello()}}</p>'
})
export class MyComp {
// This causes a type checking exception as the template
// tries to call a function called "hello()" on this variable.
myVar: boolean = false;
@ViewChild('myRef') query: any;
}
`);
writeFile('/my-module.ts', `
import {NgModule} from '@angular/core';
import {MyComp} from './index';
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
await runMigration();
expect(errorOutput.length).toBe(0);
expect(tree.readContent('/index.ts'))
.toContain(`@ViewChild('myRef', { static: true }) query: any;`);
});
it('should notify user if project has syntax errors which can affect analysis', async() => {
writeFile('/index.ts', `
import {Component, ViewChild} from '@angular/core';
@Component({
template: '<p #myRef></p>'
})
export class MyComp {
@ViewChild('myRef') query: any;
}
`);
writeFile('/file-with-syntax-error.ts', `
export classX ClassWithSyntaxError {
// ...
}
`);
writeFile('/my-module.ts', `
import {NgModule} from '@angular/core';
import {MyComp} from './index';
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
await runMigration();
expect(errorOutput.length).toBe(1);
expect(errorOutput[0]).toMatch(/file-with-syntax-error\.ts\(2,9\): error TS1128.*/);
expect(tree.readContent('/index.ts'))
.toContain(`@ViewChild('myRef', { static: true }) query: any;`);
});
it('should gracefully exit migration if queries could not be analyzed', async() => {
writeFile('/index.ts', `
import {Component, ViewChild} from '@angular/core';