Skip to content

Commit

Permalink
fix(@angular-devkit/build-optimizer): identify relative imports in an…
Browse files Browse the repository at this point in the history
…gular core

Build optimizer was broken for non-FESM files inside @angular/core because it couldn't identify relative imports were still inside core.

This change adds a known list of angular core files as a default, and also allows passing in a override.
  • Loading branch information
filipesilva authored and mgechev committed Jan 18, 2019
1 parent 44d6e34 commit a858d04
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 20 deletions.
2 changes: 2 additions & 0 deletions etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts
Expand Up @@ -12,6 +12,8 @@ export declare function getPrefixFunctionsTransformer(): ts.TransformerFactory<t

export declare function getScrubFileTransformer(program: ts.Program): ts.TransformerFactory<ts.SourceFile>;

export declare function getScrubFileTransformerForCore(program: ts.Program): ts.TransformerFactory<ts.SourceFile>;

export declare function getWrapEnumsTransformer(): ts.TransformerFactory<ts.SourceFile>;

export declare function testImportTslib(content: string): boolean;
Expand Down
Expand Up @@ -15,7 +15,11 @@ import { getFoldFileTransformer } from '../transforms/class-fold';
import { getImportTslibTransformer, testImportTslib } from '../transforms/import-tslib';
import { getPrefixClassesTransformer, testPrefixClasses } from '../transforms/prefix-classes';
import { getPrefixFunctionsTransformer } from '../transforms/prefix-functions';
import { getScrubFileTransformer, testScrubFile } from '../transforms/scrub-file';
import {
getScrubFileTransformer,
getScrubFileTransformerForCore,
testScrubFile,
} from '../transforms/scrub-file';
import { getWrapEnumsTransformer } from '../transforms/wrap-enums';


Expand Down Expand Up @@ -44,6 +48,18 @@ const ngFactories = [
/\.ngstyle\.[jt]s/,
];

// Known locations for the source files of @angular/core.
const coreFilesRegex = [
/[\\/]node_modules[\\/]@angular[\\/]core[\\/]esm5[\\/]/,
/[\\/]node_modules[\\/]@angular[\\/]core[\\/]fesm5[\\/]/,
/[\\/]node_modules[\\/]@angular[\\/]core[\\/]esm2015[\\/]/,
/[\\/]node_modules[\\/]@angular[\\/]core[\\/]fesm2015[\\/]/,
];

function isKnownCoreFile(filePath: string) {
return coreFilesRegex.some(re => re.test(filePath));
}

function isKnownSideEffectFree(filePath: string) {
return ngFactories.some((re) => re.test(filePath)) ||
whitelistedAngularModules.some((re) => re.test(filePath));
Expand All @@ -57,11 +73,12 @@ export interface BuildOptimizerOptions {
emitSourceMap?: boolean;
strict?: boolean;
isSideEffectFree?: boolean;
isAngularCoreFile?: boolean;
}

export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascriptOutput {

const { inputFilePath } = options;
const { inputFilePath, isAngularCoreFile } = options;
let { originalFilePath, content } = options;

if (!originalFilePath && inputFilePath) {
Expand All @@ -84,6 +101,15 @@ export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascr
};
}

let selectedGetScrubFileTransformer = getScrubFileTransformer;

if (
isAngularCoreFile === true ||
(isAngularCoreFile === undefined && originalFilePath && isKnownCoreFile(originalFilePath))
) {
selectedGetScrubFileTransformer = getScrubFileTransformerForCore;
}

const isWebpackBundle = content.indexOf('__webpack_require__') !== -1;

// Determine which transforms to apply.
Expand All @@ -97,14 +123,14 @@ export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascr
// We only apply it to whitelisted modules, since we know they are safe.
// getPrefixFunctionsTransformer needs to be before getFoldFileTransformer.
getPrefixFunctionsTransformer,
getScrubFileTransformer,
selectedGetScrubFileTransformer,
getFoldFileTransformer,
);
typeCheck = true;
} else if (testScrubFile(content)) {
// Always test as these require the type checker
getTransforms.push(
getScrubFileTransformer,
selectedGetScrubFileTransformer,
getFoldFileTransformer,
);
typeCheck = true;
Expand Down
Expand Up @@ -20,6 +20,7 @@ const DEBUG = false;

export interface Options {
sideEffectFreeModules?: string[];
angularCoreModules?: string[];
}

export default function optimizer(options: Options) {
Expand All @@ -34,8 +35,10 @@ export default function optimizer(options: Options) {
const normalizedId = id.replace(/\\/g, '/');
const isSideEffectFree = options.sideEffectFreeModules &&
options.sideEffectFreeModules.some(m => normalizedId.indexOf(m) >= 0);
const isAngularCoreFile = options.angularCoreModules &&
options.angularCoreModules.some(m => normalizedId.indexOf(m) >= 0);
const { content: code, sourceMap: map } = buildOptimizer({
content, inputFilePath: id, emitSourceMap: true, isSideEffectFree,
content, inputFilePath: id, emitSourceMap: true, isSideEffectFree, isAngularCoreFile,
});
if (!code) {
if (DEBUG) {
Expand Down
6 changes: 5 additions & 1 deletion packages/angular_devkit/build_optimizer/src/index.ts
Expand Up @@ -14,5 +14,9 @@ export { getFoldFileTransformer } from './transforms/class-fold';
export { getImportTslibTransformer, testImportTslib } from './transforms/import-tslib';
export { getPrefixClassesTransformer, testPrefixClasses } from './transforms/prefix-classes';
export { getPrefixFunctionsTransformer } from './transforms/prefix-functions';
export { getScrubFileTransformer, testScrubFile } from './transforms/scrub-file';
export {
getScrubFileTransformer,
getScrubFileTransformerForCore,
testScrubFile,
} from './transforms/scrub-file';
export { getWrapEnumsTransformer } from './transforms/wrap-enums';
Expand Up @@ -50,13 +50,21 @@ const angularSpecifiers = [
];

export function getScrubFileTransformer(program: ts.Program): ts.TransformerFactory<ts.SourceFile> {
const checker = program.getTypeChecker();
return scrubFileTransformer(program.getTypeChecker(), false);
}

export function getScrubFileTransformerForCore(
program: ts.Program,
): ts.TransformerFactory<ts.SourceFile> {
return scrubFileTransformer(program.getTypeChecker(), true);
}

function scrubFileTransformer(checker: ts.TypeChecker, isAngularCoreFile: boolean) {
return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {

const transformer: ts.Transformer<ts.SourceFile> = (sf: ts.SourceFile) => {

const ngMetadata = findAngularMetadata(sf);
const ngMetadata = findAngularMetadata(sf, isAngularCoreFile);
const tslibImports = findTslibImports(sf);

const nodes: ts.Node[] = [];
Expand Down Expand Up @@ -116,22 +124,27 @@ function nameOfSpecifier(node: ts.ImportSpecifier): string {
return node.name && node.name.text || '<unknown>';
}

function findAngularMetadata(node: ts.Node): ts.Node[] {
function findAngularMetadata(node: ts.Node, isAngularCoreFile: boolean): ts.Node[] {
let specs: ts.Node[] = [];
// Find all specifiers from imports of `@angular/core`.
ts.forEachChild(node, (child) => {
if (child.kind === ts.SyntaxKind.ImportDeclaration) {
const importDecl = child as ts.ImportDeclaration;
if (isAngularCoreImport(importDecl)) {
if (isAngularCoreImport(importDecl, isAngularCoreFile)) {
specs.push(...collectDeepNodes<ts.ImportSpecifier>(node, ts.SyntaxKind.ImportSpecifier)
.filter((spec) => isAngularCoreSpecifier(spec)));
}
}
});

const localDecl = findAllDeclarations(node)
.filter((decl) => angularSpecifiers.indexOf((decl.name as ts.Identifier).text) !== -1);
if (localDecl.length === angularSpecifiers.length) {
specs = specs.concat(localDecl);
// Check if the current module contains all know `@angular/core` specifiers.
// If it does, we assume it's a `@angular/core` FESM.
if (isAngularCoreFile) {
const localDecl = findAllDeclarations(node)
.filter((decl) => angularSpecifiers.indexOf((decl.name as ts.Identifier).text) !== -1);
if (localDecl.length === angularSpecifiers.length) {
specs = specs.concat(localDecl);
}
}

return specs;
Expand All @@ -154,11 +167,23 @@ function findAllDeclarations(node: ts.Node): ts.VariableDeclaration[] {
return nodes;
}

function isAngularCoreImport(node: ts.ImportDeclaration): boolean {
return true &&
node.moduleSpecifier &&
node.moduleSpecifier.kind === ts.SyntaxKind.StringLiteral &&
(node.moduleSpecifier as ts.StringLiteral).text === '@angular/core';
function isAngularCoreImport(node: ts.ImportDeclaration, isAngularCoreFile: boolean): boolean {
if (!(node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier))) {
return false;
}
const importText = node.moduleSpecifier.text;

// Imports to `@angular/core` are always core imports.
if (importText === '@angular/core') {
return true;
}

// Relative imports from a Angular core file are also core imports.
if (isAngularCoreFile && (importText.startsWith('./') || importText.startsWith('../'))) {
return true;
}

return false;
}

function isAngularCoreSpecifier(node: ts.ImportSpecifier): boolean {
Expand Down
Expand Up @@ -9,11 +9,17 @@
// tslint:disable-next-line:no-implicit-dependencies
import { tags } from '@angular-devkit/core';
import { transformJavascript } from '../helpers/transform-javascript';
import { getScrubFileTransformer, testScrubFile } from './scrub-file';
import {
getScrubFileTransformer,
getScrubFileTransformerForCore,
testScrubFile,
} from './scrub-file';


const transform = (content: string) => transformJavascript(
{ content, getTransforms: [getScrubFileTransformer], typeCheck: true }).content;
const transformCore = (content: string) => transformJavascript(
{ content, getTransforms: [getScrubFileTransformerForCore], typeCheck: true }).content;

describe('scrub-file', () => {
const clazz = 'var Clazz = (function () { function Clazz() { } return Clazz; }());';
Expand Down Expand Up @@ -289,6 +295,47 @@ describe('scrub-file', () => {
expect(testScrubFile(input)).toBeTruthy();
expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
});

it('recognizes decorator imports in Angular core', () => {
const input = tags.stripIndent`
import * as tslib_1 from "tslib";
import { Injectable } from './di';
var Console = /** @class */ (function () {
function Console() {
}
Console.prototype.log = function (message) {
console.log(message);
};
Console.prototype.warn = function (message) {
console.warn(message);
};
Console = tslib_1.__decorate([
Injectable()
], Console);
return Console;
}());
export { Console };
`;
const output = tags.stripIndent`
import * as tslib_1 from "tslib";
import { Injectable } from './di';
var Console = /** @class */ (function () {
function Console() {
}
Console.prototype.log = function (message) {
console.log(message);
};
Console.prototype.warn = function (message) {
console.warn(message);
};
return Console;
}());
export { Console };
`;

expect(testScrubFile(input)).toBeTruthy();
expect(tags.oneLine`${transformCore(input)}`).toEqual(tags.oneLine`${output}`);
});
});

describe('__metadata', () => {
Expand Down

0 comments on commit a858d04

Please sign in to comment.