Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(ngcc): enable migrations to apply schematics to libraries (#33362)
When upgrading an Angular application to a new version using the Angular
CLI, built-in schematics are being run to update user code from
deprecated patterns to the new way of working. For libraries that have
been built for older versions of Angular however, such schematics have
not been executed which means that deprecated code patterns may still be
present, potentially resulting in incorrect behavior.

Some of the logic of schematics has been ported over to ngcc migrations,
which are automatically run on libraries. These migrations achieve the
same goal of the regular schematics, but operating on published library
sources instead of used code.

PR Close #33362
  • Loading branch information
JoostK authored and AndrewKushnir committed Oct 25, 2019
1 parent 2e5e1dd commit 6b26748
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 22 deletions.
49 changes: 29 additions & 20 deletions packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts
Expand Up @@ -7,6 +7,7 @@
*/
import {ConstantPool} from '@angular/compiler';
import * as ts from 'typescript';

import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
Expand All @@ -18,9 +19,12 @@ import {ClassDeclaration} from '../../../src/ngtsc/reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../src/ngtsc/scope';
import {CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform';
import {NgccClassSymbol, NgccReflectionHost} from '../host/ngcc_host';
import {Migration, MigrationHost} from '../migrations/migration';
import {Migration} from '../migrations/migration';
import {MissingInjectableMigration} from '../migrations/missing_injectable_migration';
import {UndecoratedParentMigration} from '../migrations/undecorated_parent_migration';
import {EntryPointBundle} from '../packages/entry_point_bundle';
import {isDefined} from '../utils';

import {DefaultMigrationHost} from './migration_host';
import {AnalyzedClass, AnalyzedFile, CompiledClass, CompiledFile, DecorationAnalyses} from './types';
import {analyzeDecorators, isWithinPackage} from './util';
Expand Down Expand Up @@ -100,7 +104,7 @@ export class DecorationAnalyzer {
this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
this.isCore),
];
migrations: Migration[] = [];
migrations: Migration[] = [new UndecoratedParentMigration(), new MissingInjectableMigration()];

constructor(
private fs: FileSystem, private bundle: EntryPointBundle,
Expand All @@ -118,10 +122,9 @@ export class DecorationAnalyzer {
.filter(sourceFile => isWithinPackage(this.packagePath, sourceFile))
.map(sourceFile => this.analyzeFile(sourceFile))
.filter(isDefined);
const migrationHost = new DefaultMigrationHost(
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers,
this.bundle.entryPoint.path, analyzedFiles);
analyzedFiles.forEach(analyzedFile => this.migrateFile(migrationHost, analyzedFile));

this.applyMigrations(analyzedFiles);

analyzedFiles.forEach(analyzedFile => this.resolveFile(analyzedFile));
const compiledFiles = analyzedFiles.map(analyzedFile => this.compileFile(analyzedFile));
compiledFiles.forEach(
Expand All @@ -147,21 +150,27 @@ export class DecorationAnalyzer {
return analyzedClass;
}

protected migrateFile(migrationHost: MigrationHost, analyzedFile: AnalyzedFile): void {
analyzedFile.analyzedClasses.forEach(({declaration}) => {
this.migrations.forEach(migration => {
try {
const result = migration.apply(declaration, migrationHost);
if (result !== null) {
this.diagnosticHandler(result);
}
} catch (e) {
if (isFatalDiagnosticError(e)) {
this.diagnosticHandler(e.toDiagnostic());
} else {
throw e;
protected applyMigrations(analyzedFiles: AnalyzedFile[]): void {
const migrationHost = new DefaultMigrationHost(
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers,
this.bundle.entryPoint.path, analyzedFiles);

this.migrations.forEach(migration => {
analyzedFiles.forEach(analyzedFile => {
analyzedFile.analyzedClasses.forEach(({declaration}) => {
try {
const result = migration.apply(declaration, migrationHost);
if (result !== null) {
this.diagnosticHandler(result);
}
} catch (e) {
if (isFatalDiagnosticError(e)) {
this.diagnosticHandler(e.toDiagnostic());
} else {
throw e;
}
}
}
});
});
});
}
Expand Down
Expand Up @@ -199,10 +199,10 @@ runInEachFileSystem(() => {
it('should call `apply()` on each migration for each class', () => {
expect(migrationLogs).toEqual([
'migration1:MyComponent',
'migration2:MyComponent',
'migration1:MyDirective',
'migration2:MyDirective',
'migration1:MyOtherComponent',
'migration2:MyComponent',
'migration2:MyDirective',
'migration2:MyOtherComponent',
]);
});
Expand Down

0 comments on commit 6b26748

Please sign in to comment.