Skip to content

Commit

Permalink
feat(ngcc): migrate services that are missing @Injectable() (#33362)
Browse files Browse the repository at this point in the history
A class that is provided as Angular service is required to have an
`@Injectable()` decorator so that the compiler generates its injectable
definition for the runtime. Applications are automatically migrated
using the "missing-injectable" schematic, however libraries built for
older version of Angular may not yet satisfy this requirement.

This commit ports the "missing-injectable" schematic to a migration that
is ran when ngcc is processing a library. This ensures that any service
that is provided from an NgModule or Directive/Component will have an
`@Injectable()` decorator.

PR Close #33362
  • Loading branch information
JoostK authored and AndrewKushnir committed Oct 25, 2019
1 parent 0de2dbf commit 31b9492
Show file tree
Hide file tree
Showing 8 changed files with 1,085 additions and 140 deletions.
Expand Up @@ -119,7 +119,8 @@ export class DecorationAnalyzer {
.map(sourceFile => this.analyzeFile(sourceFile))
.filter(isDefined);
const migrationHost = new DefaultMigrationHost(
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers, analyzedFiles);
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers,
this.bundle.entryPoint.path, analyzedFiles);
analyzedFiles.forEach(analyzedFile => this.migrateFile(migrationHost, analyzedFile));
analyzedFiles.forEach(analyzedFile => this.resolveFile(analyzedFile));
const compiledFiles = analyzedFiles.map(analyzedFile => this.compileFile(analyzedFile));
Expand Down
26 changes: 24 additions & 2 deletions packages/compiler-cli/ngcc/src/analysis/migration_host.ts
Expand Up @@ -6,15 +6,18 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
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 {NgccReflectionHost} from '../host/ngcc_host';
import {MigrationHost} from '../migrations/migration';

import {AnalyzedClass, AnalyzedFile} from './types';
import {analyzeDecorators} from './util';
import {analyzeDecorators, isWithinPackage} from './util';

/**
* The standard implementation of `MigrationHost`, which is created by the
Expand All @@ -24,7 +27,7 @@ export class DefaultMigrationHost implements MigrationHost {
constructor(
readonly reflectionHost: NgccReflectionHost, readonly metadata: MetadataReader,
readonly evaluator: PartialEvaluator, private handlers: DecoratorHandler<any, any>[],
private analyzedFiles: AnalyzedFile[]) {}
private entryPointPath: AbsoluteFsPath, private analyzedFiles: AnalyzedFile[]) {}

injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void {
const classSymbol = this.reflectionHost.getClassSymbol(clazz) !;
Expand All @@ -41,6 +44,25 @@ export class DefaultMigrationHost implements MigrationHost {
mergeAnalyzedClasses(oldAnalyzedClass, newAnalyzedClass);
}
}

getAllDecorators(clazz: ClassDeclaration): Decorator[]|null {
const sourceFile = clazz.getSourceFile();
const analyzedFile = this.analyzedFiles.find(file => file.sourceFile === sourceFile);
if (analyzedFile === undefined) {
return null;
}

const analyzedClass = analyzedFile.analyzedClasses.find(c => c.declaration === clazz);
if (analyzedClass === undefined) {
return null;
}

return analyzedClass.decorators;
}

isInScope(clazz: ClassDeclaration): boolean {
return isWithinPackage(this.entryPointPath, clazz.getSourceFile());
}
}

function getOrCreateAnalyzedFile(
Expand Down
16 changes: 16 additions & 0 deletions packages/compiler-cli/ngcc/src/migrations/migration.ts
Expand Up @@ -42,4 +42,20 @@ export interface MigrationHost {
* @param decorator the decorator to inject.
*/
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void;

/**
* Retrieves all decorators that are associated with the class, including synthetic decorators
* that have been injected before.
* @param clazz the class for which all decorators are retrieved.
* @returns the list of the decorators, or null if the class was not decorated.
*/
getAllDecorators(clazz: ClassDeclaration): Decorator[]|null;

/**
* Determines whether the provided class in within scope of the entry-point that is currently
* being compiled.
* @param clazz the class for which to determine whether it is within the current entry-point.
* @returns true if the file is part of the compiled entry-point, false otherwise.
*/
isInScope(clazz: ClassDeclaration): boolean;
}
@@ -0,0 +1,192 @@
/**
* @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 {forwardRefResolver} from '../../../src/ngtsc/annotations';
import {Reference} from '../../../src/ngtsc/imports';
import {ResolvedValue, ResolvedValueMap} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';

import {Migration, MigrationHost} from './migration';
import {createInjectableDecorator, isClassDeclaration} from './utils';

/**
* Ensures that classes that are provided as an Angular service in either `NgModule.providers` or
* `Directive.providers`/`Component.viewProviders` are decorated with one of the `@Injectable`,
* `@Directive`, `@Component` or `@Pipe` decorators, adding an `@Injectable()` decorator when none
* are present.
*
* At least one decorator is now mandatory, as otherwise the compiler would not compile an
* injectable definition for the service. This is unlike View Engine, where having just an unrelated
* decorator may have been sufficient for the service to become injectable.
*
* In essence, this migration operates on classes that are themselves an NgModule, Directive or
* Component. Their metadata is statically evaluated so that their "providers"/"viewProviders"
* properties can be analyzed. For any provider that refers to an undecorated class, the class will
* be migrated to have an `@Injectable()` decorator.
*
* This implementation mirrors the "missing-injectable" schematic.
*/
export class MissingInjectableMigration implements Migration {
apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null {
const decorators = host.reflectionHost.getDecoratorsOfDeclaration(clazz);
if (decorators === null) {
return null;
}

for (const decorator of decorators) {
const name = getAngularCoreDecoratorName(decorator);
if (name === 'NgModule') {
migrateNgModuleProviders(decorator, host);
} else if (name === 'Directive') {
migrateDirectiveProviders(decorator, host, /* isComponent */ false);
} else if (name === 'Component') {
migrateDirectiveProviders(decorator, host, /* isComponent */ true);
}
}

return null;
}
}

/**
* Iterates through all `NgModule.providers` and adds the `@Injectable()` decorator to any provider
* that is not otherwise decorated.
*/
function migrateNgModuleProviders(decorator: Decorator, host: MigrationHost): void {
if (decorator.args === null || decorator.args.length !== 1) {
return;
}

const metadata = host.evaluator.evaluate(decorator.args[0], forwardRefResolver);
if (!(metadata instanceof Map)) {
return;
}

migrateProviders(metadata, 'providers', host);
// TODO(alxhub): we should probably also check for `ModuleWithProviders` here.
}

/**
* Iterates through all `Directive.providers` and if `isComponent` is set to true also
* `Component.viewProviders` and adds the `@Injectable()` decorator to any provider that is not
* otherwise decorated.
*/
function migrateDirectiveProviders(
decorator: Decorator, host: MigrationHost, isComponent: boolean): void {
if (decorator.args === null || decorator.args.length !== 1) {
return;
}

const metadata = host.evaluator.evaluate(decorator.args[0], forwardRefResolver);
if (!(metadata instanceof Map)) {
return;
}

migrateProviders(metadata, 'providers', host);
if (isComponent) {
migrateProviders(metadata, 'viewProviders', host);
}
}

/**
* Given an object with decorator metadata, iterates through the list of providers to add
* `@Injectable()` to any provider that is not otherwise decorated.
*/
function migrateProviders(metadata: ResolvedValueMap, field: string, host: MigrationHost): void {
if (!metadata.has(field)) {
return;
}
const providers = metadata.get(field) !;
if (!Array.isArray(providers)) {
return;
}

for (const provider of providers) {
migrateProvider(provider, host);
}
}

/**
* Analyzes a single provider entry and determines the class that is required to have an
* `@Injectable()` decorator.
*/
function migrateProvider(provider: ResolvedValue, host: MigrationHost): void {
if (provider instanceof Map) {
if (!provider.has('provide') || provider.has('useValue') || provider.has('useFactory') ||
provider.has('useExisting')) {
return;
}
if (provider.has('useClass')) {
// {provide: ..., useClass: SomeClass, deps: [...]} does not require a decorator on SomeClass,
// as the provider itself configures 'deps'. Only if 'deps' is missing will this require a
// factory to exist on SomeClass.
if (!provider.has('deps')) {
migrateProviderClass(provider.get('useClass') !, host);
}
} else {
migrateProviderClass(provider.get('provide') !, host);
}
} else if (Array.isArray(provider)) {
for (const v of provider) {
migrateProvider(v, host);
}
} else {
migrateProviderClass(provider, host);
}
}

/**
* Given a provider class, adds the `@Injectable()` decorator if no other relevant Angular decorator
* is present on the class.
*/
function migrateProviderClass(provider: ResolvedValue, host: MigrationHost): void {
// Providers that do not refer to a class cannot be migrated.
if (!(provider instanceof Reference)) {
return;
}

const clazz = provider.node as ts.Declaration;
if (isClassDeclaration(clazz) && host.isInScope(clazz) && needsInjectableDecorator(clazz, host)) {
host.injectSyntheticDecorator(clazz, createInjectableDecorator(clazz));
}
}

const NO_MIGRATE_DECORATORS = new Set(['Injectable', 'Directive', 'Component', 'Pipe']);

/**
* Determines if the given class needs to be decorated with `@Injectable()` based on whether it
* already has an Angular decorator applied.
*/
function needsInjectableDecorator(clazz: ClassDeclaration, host: MigrationHost): boolean {
const decorators = host.getAllDecorators(clazz);
if (decorators === null) {
return true;
}

for (const decorator of decorators) {
const name = getAngularCoreDecoratorName(decorator);
if (name !== null && NO_MIGRATE_DECORATORS.has(name)) {
return false;
}
}

return true;
}

/**
* Determines the original name of a decorator if it is from '@angular/core'. For other decorators,
* null is returned.
*/
export function getAngularCoreDecoratorName(decorator: Decorator): string|null {
if (decorator.import === null || decorator.import.from !== '@angular/core') {
return null;
}

return decorator.import.name;
}
21 changes: 21 additions & 0 deletions packages/compiler-cli/ngcc/src/migrations/utils.ts
Expand Up @@ -54,6 +54,27 @@ export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator {
};
}

/**
* Create an empty `Injectable` decorator that will be associated with the `clazz`.
*/
export function createInjectableDecorator(clazz: ClassDeclaration): Decorator {
const decoratorType = ts.createIdentifier('Injectable');
const decoratorNode = ts.createObjectLiteral([
ts.createPropertyAssignment('type', decoratorType),
ts.createPropertyAssignment('args', ts.createArrayLiteral([])),
]);

setParentPointers(clazz.getSourceFile(), decoratorNode);

return {
name: 'Injectable',
identifier: decoratorType,
import: {name: 'Injectable', from: '@angular/core'},
node: decoratorNode,
args: [],
};
}

/**
* Ensure that a tree of AST nodes have their parents wired up.
*/
Expand Down

0 comments on commit 31b9492

Please sign in to comment.