Skip to content

Commit

Permalink
fix(ivy): ensure ngcc compiles @angular/core with correct internal …
Browse files Browse the repository at this point in the history
…imports (angular#26236)

PR Close angular#26236
  • Loading branch information
petebacondarwin authored and jasonaden committed Oct 8, 2018
1 parent 807070f commit 83302d1
Show file tree
Hide file tree
Showing 20 changed files with 357 additions and 210 deletions.
13 changes: 7 additions & 6 deletions packages/compiler-cli/src/ngcc/src/analyzer.ts
Expand Up @@ -48,16 +48,17 @@ export class Analyzer {
handlers: DecoratorHandler<any, any>[] = [
new BaseDefDecoratorHandler(this.typeChecker, this.host),
new ComponentDecoratorHandler(
this.typeChecker, this.host, this.scopeRegistry, false, this.resourceLoader, this.rootDirs),
new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
new InjectableDecoratorHandler(this.host, false),
new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
new PipeDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
this.typeChecker, this.host, this.scopeRegistry, this.isCore, this.resourceLoader,
this.rootDirs),
new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore),
new InjectableDecoratorHandler(this.host, this.isCore),
new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore),
new PipeDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore),
];

constructor(
private typeChecker: ts.TypeChecker, private host: NgccReflectionHost,
private rootDirs: string[]) {}
private rootDirs: string[], private isCore: boolean) {}

/**
* Analyize a parsed file to generate the information about decorated classes that
Expand Down
4 changes: 3 additions & 1 deletion packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts
Expand Up @@ -13,7 +13,9 @@ import {DtsMapper} from './dts_mapper';
import {Fesm2015ReflectionHost} from './fesm2015_host';

export class Esm2015ReflectionHost extends Fesm2015ReflectionHost {
constructor(checker: ts.TypeChecker, protected dtsMapper: DtsMapper) { super(checker); }
constructor(isCore: boolean, checker: ts.TypeChecker, protected dtsMapper: DtsMapper) {
super(isCore, checker);
}

/**
* Get the number of generic type parameters of a given class.
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngcc/src/host/esm5_host.ts
Expand Up @@ -31,7 +31,7 @@ import {Fesm2015ReflectionHost, ParamInfo, getPropertyValueFromSymbol, isAssignm
*
*/
export class Esm5ReflectionHost extends Fesm2015ReflectionHost {
constructor(checker: ts.TypeChecker) { super(checker); }
constructor(isCore: boolean, checker: ts.TypeChecker) { super(isCore, checker); }

/**
* Check whether the given node actually represents a class.
Expand Down
128 changes: 102 additions & 26 deletions packages/compiler-cli/src/ngcc/src/host/fesm2015_host.ts
Expand Up @@ -9,7 +9,7 @@
import {normalize} from 'canonical-path';
import * as ts from 'typescript';

import {ClassMember, ClassMemberKind, CtorParameter, Decorator} from '../../../ngtsc/host';
import {ClassMember, ClassMemberKind, CtorParameter, Decorator, Import} from '../../../ngtsc/host';
import {TypeScriptReflectionHost, reflectObjectLiteral} from '../../../ngtsc/metadata';
import {findAll, getNameText, isDefined} from '../utils';

Expand Down Expand Up @@ -48,7 +48,7 @@ export const CONSTRUCTOR_PARAMS = 'ctorParameters' as ts.__String;
* a static method called `ctorParameters`.
*/
export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements NgccReflectionHost {
constructor(checker: ts.TypeChecker) { super(checker); }
constructor(protected isCore: boolean, checker: ts.TypeChecker) { super(checker); }

/**
* Examine a declaration (for example, of a class or function) and return metadata about any
Expand Down Expand Up @@ -281,6 +281,19 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
return null;
}

/**
* Determine if an identifier was imported from another module and return `Import` metadata
* describing its origin.
*
* @param id a TypeScript `ts.Identifer` to reflect.
*
* @returns metadata about the `Import` if the identifier was imported from another module, or
* `null` if the identifier doesn't resolve to an import but instead is locally defined.
*/
getImportOfIdentifier(id: ts.Identifier): Import|null {
return super.getImportOfIdentifier(id) || this.getImportOfNamespacedIdentifier(id);
}

///////////// Protected Helpers /////////////

/**
Expand Down Expand Up @@ -336,7 +349,8 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
decoratorsIdentifier.parent.operatorToken.kind === ts.SyntaxKind.EqualsToken) {
// AST of the array of decorator values
const decoratorsArray = decoratorsIdentifier.parent.right;
return this.reflectDecorators(decoratorsArray).filter(isImportedFromCore);
return this.reflectDecorators(decoratorsArray)
.filter(decorator => this.isFromCore(decorator));
}
}
return null;
Expand All @@ -362,7 +376,8 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
helperCalls.forEach(helperCall => {
const {classDecorators} =
this.reflectDecoratorsFromHelperCall(helperCall, makeClassTargetFilter(symbol.name));
classDecorators.filter(isImportedFromCore).forEach(decorator => decorators.push(decorator));
classDecorators.filter(decorator => this.isFromCore(decorator))
.forEach(decorator => decorators.push(decorator));
});
return decorators.length ? decorators : null;
}
Expand Down Expand Up @@ -405,7 +420,8 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
if (propDecoratorsMap && ts.isObjectLiteralExpression(propDecoratorsMap)) {
const propertiesMap = reflectObjectLiteral(propDecoratorsMap);
propertiesMap.forEach((value, name) => {
const decorators = this.reflectDecorators(value).filter(isImportedFromCore);
const decorators =
this.reflectDecorators(value).filter(decorator => this.isFromCore(decorator));
if (decorators.length) {
memberDecorators.set(name, decorators);
}
Expand Down Expand Up @@ -437,7 +453,7 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
memberDecorators.forEach((decorators, memberName) => {
if (memberName) {
const memberDecorators = memberDecoratorMap.get(memberName) || [];
const coreDecorators = decorators.filter(isImportedFromCore);
const coreDecorators = decorators.filter(decorator => this.isFromCore(decorator));
memberDecoratorMap.set(memberName, memberDecorators.concat(coreDecorators));
}
});
Expand Down Expand Up @@ -731,8 +747,9 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
.map(paramInfo => {
const type = paramInfo && paramInfo.get('type') || null;
const decoratorInfo = paramInfo && paramInfo.get('decorators') || null;
const decorators =
decoratorInfo && this.reflectDecorators(decoratorInfo).filter(isImportedFromCore);
const decorators = decoratorInfo &&
this.reflectDecorators(decoratorInfo)
.filter(decorator => this.isFromCore(decorator));
return {type, decorators};
});
}
Expand Down Expand Up @@ -816,6 +833,70 @@ export class Fesm2015ReflectionHost extends TypeScriptReflectionHost implements
protected getStatementsForClass(classSymbol: ts.Symbol): ts.Statement[] {
return Array.from(classSymbol.valueDeclaration.getSourceFile().statements);
}

/**
* Try to get the import info for this identifier as though it is a namespaced import.
* For example, if the identifier is the `__metadata` part of a property access chain like:
*
* ```
* tslib_1.__metadata
* ```
*
* then it might be that `tslib_1` is a namespace import such as:
*
* ```
* import * as tslib_1 from 'tslib';
* ```
* @param id the TypeScript identifier to find the import info for.
* @returns The import info if this is a namespaced import or `null`.
*/
protected getImportOfNamespacedIdentifier(id: ts.Identifier): Import|null {
if (!(ts.isPropertyAccessExpression(id.parent) && id.parent.name === id)) {
return null;
}

const namespaceIdentifier = getFarLeftIdentifier(id.parent);
const namespaceSymbol =
namespaceIdentifier && this.checker.getSymbolAtLocation(namespaceIdentifier);
const declaration = namespaceSymbol && namespaceSymbol.declarations.length === 1 ?
namespaceSymbol.declarations[0] :
null;
const namespaceDeclaration =
declaration && ts.isNamespaceImport(declaration) ? declaration : null;
if (!namespaceDeclaration) {
return null;
}

const importDeclaration = namespaceDeclaration.parent.parent;
if (!ts.isStringLiteral(importDeclaration.moduleSpecifier)) {
// Should not happen as this would be invalid TypesScript
return null;
}

return {
from: importDeclaration.moduleSpecifier.text,
name: id.text,
};
}

/**
* Test whether a decorator was imported from `@angular/core`.
*
* Is the decorator:
* * externally imported from `@angulare/core`?
* * the current hosted program is actually `@angular/core` and
* - relatively internally imported; or
* - not imported, from the current file.
*
* @param decorator the decorator to test.
*/
isFromCore(decorator: Decorator): boolean {
if (this.isCore) {
return !decorator.import || /^\./.test(decorator.import.from);
} else {
return !!decorator.import && decorator.import.from === '@angular/core';
}
}
}

///////////// Exported Helpers /////////////
Expand Down Expand Up @@ -846,24 +927,6 @@ export function isAssignment(expression: ts.Expression):
expression.operatorToken.kind === ts.SyntaxKind.EqualsToken;
}

/**
* Test whether a decorator was imported from `@angular/core`.
*
* Is the decorator:
* * extermally mported from `@angulare/core`?
* * relatively internally imported where the decoratee is already in `@angular/core`?
*
* Note we do not support decorators that are not imported at all.
*
* @param decorator the decorator to test.
*/
export function isImportedFromCore(decorator: Decorator): boolean {
const importFrom = decorator.import && decorator.import.from || '';
return importFrom === '@angular/core' ||
(/^\./.test(importFrom) &&
/node_modules[\\\/]@angular[\\\/]core/.test(decorator.node.getSourceFile().fileName));
}

/**
* The type of a function that can be used to filter out helpers based on their target.
* This is used in `reflectDecoratorsFromHelperCall()`.
Expand Down Expand Up @@ -952,3 +1015,16 @@ function isClassMemberType(node: ts.Declaration): node is ts.ClassElement|
ts.PropertyAccessExpression|ts.BinaryExpression {
return ts.isClassElement(node) || isPropertyAccess(node) || ts.isBinaryExpression(node);
}

/**
* Compute the left most identifier in a property access chain. E.g. the `a` of `a.b.c.d`.
* @param propertyAccess The starting property access expression from which we want to compute
* the left most identifier.
* @returns the left most identifier in the chain or `null` if it is not an identifier.
*/
function getFarLeftIdentifier(propertyAccess: ts.PropertyAccessExpression): ts.Identifier|null {
while (ts.isPropertyAccessExpression(propertyAccess.expression)) {
propertyAccess = propertyAccess.expression;
}
return ts.isIdentifier(propertyAccess.expression) ? propertyAccess.expression : null;
}
62 changes: 49 additions & 13 deletions packages/compiler-cli/src/ngcc/src/packages/transformer.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {dirname, relative, resolve} from 'canonical-path';
import {existsSync, readFileSync, writeFileSync} from 'fs';
import {existsSync, lstatSync, readFileSync, readdirSync, writeFileSync} from 'fs';
import {mkdir, mv} from 'shelljs';
import * as ts from 'typescript';

Expand All @@ -24,10 +24,10 @@ import {FileParser} from '../parsing/file_parser';
import {Esm2015Renderer} from '../rendering/esm2015_renderer';
import {Esm5Renderer} from '../rendering/esm5_renderer';
import {FileInfo, Renderer} from '../rendering/renderer';

import {checkMarkerFile, writeMarkerFile} from './build_marker';
import {EntryPoint, EntryPointFormat} from './entry_point';


/**
* A Package is stored in a directory on disk and that directory can contain one or more package
* formats - e.g. fesm2015, UMD, etc. Additionally, each package provides typings (`.d.ts` files).
Expand Down Expand Up @@ -78,14 +78,19 @@ export class Transformer {
throw new Error(
`Missing entry point file for format, ${format}, in package, ${entryPoint.path}.`);
}
const packageProgram = ts.createProgram([entryPointFilePath], options, host);
const isCore = entryPoint.name === '@angular/core';
const r3SymbolsPath = isCore ? this.findR3SymbolsPath(dirname(entryPointFilePath)) : null;
const rootPaths = r3SymbolsPath ? [entryPointFilePath, r3SymbolsPath] : [entryPointFilePath];
const packageProgram = ts.createProgram(rootPaths, options, host);
const typeChecker = packageProgram.getTypeChecker();
const dtsMapper = new DtsMapper(dirname(entryPointFilePath), dirname(entryPoint.typings));
const reflectionHost = this.getHost(format, packageProgram, dtsMapper);
const reflectionHost = this.getHost(isCore, format, packageProgram, dtsMapper);
const r3SymbolsFile = r3SymbolsPath && packageProgram.getSourceFile(r3SymbolsPath) || null;

const parser = this.getFileParser(format, packageProgram, reflectionHost);
const analyzer = new Analyzer(typeChecker, reflectionHost, rootDirs);
const renderer = this.getRenderer(format, packageProgram, reflectionHost);
const analyzer = new Analyzer(typeChecker, reflectionHost, rootDirs, isCore);
const renderer =
this.getRenderer(format, packageProgram, reflectionHost, isCore, r3SymbolsFile);

// Parse and analyze the files.
const entryPointFile = packageProgram.getSourceFile(entryPointFilePath) !;
Expand All @@ -111,15 +116,16 @@ export class Transformer {
writeMarkerFile(entryPoint, format);
}

getHost(format: string, program: ts.Program, dtsMapper: DtsMapper): NgccReflectionHost {
getHost(isCore: boolean, format: string, program: ts.Program, dtsMapper: DtsMapper):
NgccReflectionHost {
switch (format) {
case 'esm2015':
return new Esm2015ReflectionHost(program.getTypeChecker(), dtsMapper);
return new Esm2015ReflectionHost(isCore, program.getTypeChecker(), dtsMapper);
case 'fesm2015':
return new Fesm2015ReflectionHost(program.getTypeChecker());
return new Fesm2015ReflectionHost(isCore, program.getTypeChecker());
case 'esm5':
case 'fesm5':
return new Esm5ReflectionHost(program.getTypeChecker());
return new Esm5ReflectionHost(isCore, program.getTypeChecker());
default:
throw new Error(`Relection host for "${format}" not yet implemented.`);
}
Expand All @@ -138,14 +144,16 @@ export class Transformer {
}
}

getRenderer(format: string, program: ts.Program, host: NgccReflectionHost): Renderer {
getRenderer(
format: string, program: ts.Program, host: NgccReflectionHost, isCore: boolean,
rewriteCoreImportsTo: ts.SourceFile|null): Renderer {
switch (format) {
case 'esm2015':
case 'fesm2015':
return new Esm2015Renderer(host);
return new Esm2015Renderer(host, isCore, rewriteCoreImportsTo);
case 'esm5':
case 'fesm5':
return new Esm5Renderer(host);
return new Esm5Renderer(host, isCore, rewriteCoreImportsTo);
default:
throw new Error(`Renderer for "${format}" not yet implemented.`);
}
Expand Down Expand Up @@ -210,4 +218,32 @@ export class Transformer {
}
writeFileSync(file.path, file.contents, 'utf8');
}

findR3SymbolsPath(directory: string): string|null {
const r3SymbolsFilePath = resolve(directory, 'r3_symbols.js');
if (existsSync(r3SymbolsFilePath)) {
return r3SymbolsFilePath;
}

const subDirectories =
readdirSync(directory)
// Not interested in hidden files
.filter(p => !p.startsWith('.'))
// Ignore node_modules
.filter(p => p !== 'node_modules')
// Only interested in directories (and only those that are not symlinks)
.filter(p => {
const stat = lstatSync(resolve(directory, p));
return stat.isDirectory() && !stat.isSymbolicLink();
});

for (const subDirectory of subDirectories) {
const r3SymbolsFilePath = this.findR3SymbolsPath(resolve(directory, subDirectory));
if (r3SymbolsFilePath) {
return r3SymbolsFilePath;
}
}

return null;
}
}
@@ -0,0 +1,21 @@

/**
* @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 {ImportManager} from '../../../ngtsc/translator';

export class NgccImportManager extends ImportManager {
constructor(private isFlat: boolean, isCore: boolean, prefix?: string) { super(isCore, prefix); }

generateNamedImport(moduleName: string, symbol: string): string|null {
if (this.isFlat && this.isCore && moduleName === '@angular/core') {
return null;
}
return super.generateNamedImport(moduleName, symbol);
}
}

0 comments on commit 83302d1

Please sign in to comment.