Skip to content

Commit

Permalink
feat(ngcc): add a migration for undecorated child classes (#33362)
Browse files Browse the repository at this point in the history
In Angular View Engine, there are two kinds of decorator inheritance:

1) both the parent and child classes have decorators

This case is supported by InheritDefinitionFeature, which merges some fields
of the definitions (such as the inputs or queries).

2) only the parent class has a decorator

If the child class is missing a decorator, the compiler effectively behaves
as if the parent class' decorator is applied to the child class as well.
This is the "undecorated child" scenario, and this commit adds a migration
to ngcc to support this pattern in Ivy.

This migration has 2 phases. First, the NgModules of the application are
scanned for classes in 'declarations' which are missing decorators, but
whose base classes do have decorators. These classes are the undecorated
children. This scan is performed recursively, so even if a declared class
has a base class that itself inherits a decorator, this case is handled.

Next, a synthetic decorator (either @component or @directive) is created
on the child class. This decorator copies some critical information such
as 'selector' and 'exportAs', as well as supports any decorated fields
(@input, etc). A flag is passed to the decorator compiler which causes a
special feature `CopyDefinitionFeature` to be included on the compiled
definition. This feature copies at runtime the remaining aspects of the
parent definition which `InheritDefinitionFeature` does not handle,
completing the "full" inheritance of the child class' decorator from its
parent class.

PR Close #33362
  • Loading branch information
alxhub authored and AndrewKushnir committed Oct 25, 2019
1 parent 818c514 commit b381497
Show file tree
Hide file tree
Showing 16 changed files with 451 additions and 24 deletions.
Expand Up @@ -21,6 +21,7 @@ import {CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '
import {NgccClassSymbol, NgccReflectionHost} from '../host/ngcc_host';
import {Migration} from '../migrations/migration';
import {MissingInjectableMigration} from '../migrations/missing_injectable_migration';
import {UndecoratedChildMigration} from '../migrations/undecorated_child_migration';
import {UndecoratedParentMigration} from '../migrations/undecorated_parent_migration';
import {EntryPointBundle} from '../packages/entry_point_bundle';
import {isDefined} from '../utils';
Expand All @@ -29,6 +30,7 @@ import {DefaultMigrationHost} from './migration_host';
import {AnalyzedClass, AnalyzedFile, CompiledClass, CompiledFile, DecorationAnalyses} from './types';
import {analyzeDecorators, isWithinPackage} from './util';


/**
* Simple class that resolves and loads files directly from the filesystem.
*/
Expand Down Expand Up @@ -104,7 +106,11 @@ export class DecorationAnalyzer {
this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
this.isCore),
];
migrations: Migration[] = [new UndecoratedParentMigration(), new MissingInjectableMigration()];
migrations: Migration[] = [
new UndecoratedParentMigration(),
new UndecoratedChildMigration(),
new MissingInjectableMigration(),
];

constructor(
private fs: FileSystem, private bundle: EntryPointBundle,
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler-cli/ngcc/src/analysis/migration_host.ts
Expand Up @@ -12,7 +12,7 @@ import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {MetadataReader} from '../../../src/ngtsc/metadata';
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {DecoratorHandler} from '../../../src/ngtsc/transform';
import {DecoratorHandler, HandlerFlags} from '../../../src/ngtsc/transform';
import {NgccReflectionHost} from '../host/ngcc_host';
import {MigrationHost} from '../migrations/migration';

Expand All @@ -29,9 +29,10 @@ export class DefaultMigrationHost implements MigrationHost {
readonly evaluator: PartialEvaluator, private handlers: DecoratorHandler<any, any>[],
private entryPointPath: AbsoluteFsPath, private analyzedFiles: AnalyzedFile[]) {}

injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void {
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator, flags?: HandlerFlags):
void {
const classSymbol = this.reflectionHost.getClassSymbol(clazz) !;
const newAnalyzedClass = analyzeDecorators(classSymbol, [decorator], this.handlers);
const newAnalyzedClass = analyzeDecorators(classSymbol, [decorator], this.handlers, flags);
if (newAnalyzedClass === null) {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/ngcc/src/analysis/util.ts
Expand Up @@ -10,7 +10,7 @@ import * as ts from 'typescript';
import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
import {AbsoluteFsPath, absoluteFromSourceFile, relative} from '../../../src/ngtsc/file_system';
import {Decorator} from '../../../src/ngtsc/reflection';
import {DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform';
import {DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence} from '../../../src/ngtsc/transform';
import {NgccClassSymbol} from '../host/ngcc_host';

import {AnalyzedClass, MatchingHandler} from './types';
Expand All @@ -21,7 +21,7 @@ export function isWithinPackage(packagePath: AbsoluteFsPath, sourceFile: ts.Sour

export function analyzeDecorators(
classSymbol: NgccClassSymbol, decorators: Decorator[] | null,
handlers: DecoratorHandler<any, any>[]): AnalyzedClass|null {
handlers: DecoratorHandler<any, any>[], flags?: HandlerFlags): AnalyzedClass|null {
const declaration = classSymbol.declaration.valueDeclaration;
const matchingHandlers = handlers
.map(handler => {
Expand Down Expand Up @@ -64,7 +64,7 @@ export function analyzeDecorators(
const allDiagnostics: ts.Diagnostic[] = [];
for (const {handler, detected} of detections) {
try {
const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata);
const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata, flags);
if (diagnostics !== undefined) {
allDiagnostics.push(...diagnostics);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/compiler-cli/ngcc/src/migrations/migration.ts
Expand Up @@ -6,11 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';

import {MetadataReader} from '../../../src/ngtsc/metadata';
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {HandlerFlags} from '../../../src/ngtsc/transform';
import {NgccReflectionHost} from '../host/ngcc_host';


/**
* Implement this interface and add it to the `DecorationAnalyzer.migrations` collection to get ngcc
* to modify the analysis of the decorators in the program in order to migrate older code to work
Expand Down Expand Up @@ -41,7 +44,8 @@ export interface MigrationHost {
* @param clazz the class to receive the new decorator.
* @param decorator the decorator to inject.
*/
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void;
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator, flags?: HandlerFlags):
void;

/**
* Retrieves all decorators that are associated with the class, including synthetic decorators
Expand Down
@@ -0,0 +1,75 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import * as ts from 'typescript';

import {readBaseClass} from '../../../src/ngtsc/annotations/src/util';
import {Reference} from '../../../src/ngtsc/imports';
import {ClassDeclaration} from '../../../src/ngtsc/reflection';
import {HandlerFlags} from '../../../src/ngtsc/transform';

import {Migration, MigrationHost} from './migration';
import {createComponentDecorator, createDirectiveDecorator, hasDirectiveDecorator, hasPipeDecorator} from './utils';

export class UndecoratedChildMigration implements Migration {
apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null {
// This migration looks at NgModules and considers the directives (and pipes) it declares.
// It verifies that these classes have decorators.
const moduleMeta = host.metadata.getNgModuleMetadata(new Reference(clazz));
if (moduleMeta === null) {
// Not an NgModule; don't care.
return null;
}

// Examine each of the declarations to see if it needs to be migrated.
for (const decl of moduleMeta.declarations) {
this.maybeMigrate(decl, host);
}

return null;
}

maybeMigrate(ref: Reference<ClassDeclaration>, host: MigrationHost): void {
if (hasDirectiveDecorator(host, ref.node) || hasPipeDecorator(host, ref.node)) {
// Stop if one of the classes in the chain is actually decorated with @Directive, @Component,
// or @Pipe.
return;
}

const baseRef = readBaseClass(ref.node, host.reflectionHost, host.evaluator);
if (baseRef === null) {
// Stop: can't migrate a class with no parent.
return;
} else if (baseRef === 'dynamic') {
// Stop: can't migrate a class with an indeterminate parent.
return;
}

// Apply the migration recursively, to handle inheritance chains.
this.maybeMigrate(baseRef, host);

// After the above call, `host.metadata` should have metadata for the base class, if indeed this
// is a directive inheritance chain.
const baseMeta = host.metadata.getDirectiveMetadata(baseRef);
if (baseMeta === null) {
// Stop: this isn't a directive inheritance chain after all.
return;
}

// Otherwise, decorate the class with @Component() or @Directive(), as appropriate.
if (baseMeta.isComponent) {
host.injectSyntheticDecorator(
ref.node, createComponentDecorator(ref.node, baseMeta), HandlerFlags.FULL_INHERITANCE);
} else {
host.injectSyntheticDecorator(
ref.node, createDirectiveDecorator(ref.node, baseMeta), HandlerFlags.FULL_INHERITANCE);
}

// Success!
}
}
60 changes: 58 additions & 2 deletions packages/compiler-cli/ngcc/src/migrations/utils.ts
Expand Up @@ -23,6 +23,14 @@ export function hasDirectiveDecorator(host: MigrationHost, clazz: ClassDeclarati
return host.metadata.getDirectiveMetadata(ref) !== null;
}

/**
* Returns true if the `clazz` is decorated as a `Pipe`.
*/
export function hasPipeDecorator(host: MigrationHost, clazz: ClassDeclaration): boolean {
const ref = new Reference(clazz);
return host.metadata.getPipeMetadata(ref) !== null;
}

/**
* Returns true if the `clazz` has its own constructor function.
*/
Expand All @@ -33,14 +41,53 @@ export function hasConstructor(host: MigrationHost, clazz: ClassDeclaration): bo
/**
* Create an empty `Directive` decorator that will be associated with the `clazz`.
*/
export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator {
export function createDirectiveDecorator(
clazz: ClassDeclaration,
metadata?: {selector: string | null, exportAs: string[] | null}): Decorator {
const args: ts.Expression[] = [];
if (metadata !== undefined) {
const metaArgs: ts.PropertyAssignment[] = [];
if (metadata.selector !== null) {
metaArgs.push(property('selector', metadata.selector));
}
if (metadata.exportAs !== null) {
metaArgs.push(property('exportAs', metadata.exportAs));
}
args.push(reifySourceFile(ts.createObjectLiteral(metaArgs)));
}
return {
name: 'Directive',
identifier: null,
import: {name: 'Directive', from: '@angular/core'},
node: null,
synthesizedFor: clazz.name, args,
};
}

/**
* Create an empty `Component` decorator that will be associated with the `clazz`.
*/
export function createComponentDecorator(
clazz: ClassDeclaration,
metadata: {selector: string | null, exportAs: string[] | null}): Decorator {
const metaArgs: ts.PropertyAssignment[] = [
property('template', ''),
];
if (metadata.selector !== null) {
metaArgs.push(property('selector', metadata.selector));
}
if (metadata.exportAs !== null) {
metaArgs.push(property('exportAs', metadata.exportAs));
}
return {
name: 'Component',
identifier: null,
import: {name: 'Component', from: '@angular/core'},
node: null,
synthesizedFor: clazz.name,
args: [],
args: [
reifySourceFile(ts.createObjectLiteral(metaArgs)),
],
};
}

Expand All @@ -58,6 +105,15 @@ export function createInjectableDecorator(clazz: ClassDeclaration): Decorator {
};
}

function property(name: string, value: string | string[]): ts.PropertyAssignment {
if (typeof value === 'string') {
return ts.createPropertyAssignment(name, ts.createStringLiteral(value));
} else {
return ts.createPropertyAssignment(
name, ts.createArrayLiteral(value.map(v => ts.createStringLiteral(v))));
}
}

const EMPTY_SF = ts.createSourceFile('(empty)', '', ts.ScriptTarget.Latest);

/**
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/ngcc/test/BUILD.bazel
Expand Up @@ -56,6 +56,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/test/helpers",
"@npm//rxjs",
"@npm//typescript",
],
)

Expand Down

0 comments on commit b381497

Please sign in to comment.