From 6f8c336346e921ea0ca66f574267f69c9ef0c479 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Fri, 18 Jan 2019 15:43:46 +0000 Subject: [PATCH] fix(@angular-devkit/build-optimizer): identify relative imports in angular 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. --- .../build_optimizer/src/_golden-api.d.ts | 2 + .../src/build-optimizer/build-optimizer.ts | 34 +++++++++++-- .../src/build-optimizer/rollup-plugin.ts | 5 +- .../build_optimizer/src/index.ts | 6 ++- .../src/transforms/scrub-file.ts | 51 ++++++++++++++----- .../src/transforms/scrub-file_spec.ts | 49 +++++++++++++++++- 6 files changed, 127 insertions(+), 20 deletions(-) diff --git a/etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts b/etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts index e3e9a7ef1a8d..efd778ab0f9e 100644 --- a/etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts +++ b/etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts @@ -12,6 +12,8 @@ export declare function getPrefixFunctionsTransformer(): ts.TransformerFactory; +export declare function getScrubFileTransformerForCore(program: ts.Program): ts.TransformerFactory; + export declare function getWrapEnumsTransformer(): ts.TransformerFactory; export declare function testImportTslib(content: string): boolean; diff --git a/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts b/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts index 13e69da33d83..2ca30df2b7af 100644 --- a/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts +++ b/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts @@ -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'; @@ -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)); @@ -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) { @@ -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. @@ -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; diff --git a/packages/angular_devkit/build_optimizer/src/build-optimizer/rollup-plugin.ts b/packages/angular_devkit/build_optimizer/src/build-optimizer/rollup-plugin.ts index 54742407e872..29b85fd7ceb9 100644 --- a/packages/angular_devkit/build_optimizer/src/build-optimizer/rollup-plugin.ts +++ b/packages/angular_devkit/build_optimizer/src/build-optimizer/rollup-plugin.ts @@ -20,6 +20,7 @@ const DEBUG = false; export interface Options { sideEffectFreeModules?: string[]; + angularCoreModules?: string[]; } export default function optimizer(options: Options) { @@ -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) { diff --git a/packages/angular_devkit/build_optimizer/src/index.ts b/packages/angular_devkit/build_optimizer/src/index.ts index 1cc3bda13f17..56f62c1f4b56 100644 --- a/packages/angular_devkit/build_optimizer/src/index.ts +++ b/packages/angular_devkit/build_optimizer/src/index.ts @@ -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'; diff --git a/packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts b/packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts index 2f89eb1336d4..6ddb861be181 100644 --- a/packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts +++ b/packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts @@ -50,13 +50,21 @@ const angularSpecifiers = [ ]; export function getScrubFileTransformer(program: ts.Program): ts.TransformerFactory { - const checker = program.getTypeChecker(); + return scrubFileTransformer(program.getTypeChecker(), false); +} + +export function getScrubFileTransformerForCore( + program: ts.Program, +): ts.TransformerFactory { + return scrubFileTransformer(program.getTypeChecker(), true); +} +function scrubFileTransformer(checker: ts.TypeChecker, isAngularCoreFile: boolean) { return (context: ts.TransformationContext): ts.Transformer => { const transformer: ts.Transformer = (sf: ts.SourceFile) => { - const ngMetadata = findAngularMetadata(sf); + const ngMetadata = findAngularMetadata(sf, isAngularCoreFile); const tslibImports = findTslibImports(sf); const nodes: ts.Node[] = []; @@ -116,22 +124,27 @@ function nameOfSpecifier(node: ts.ImportSpecifier): string { return node.name && node.name.text || ''; } -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(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; @@ -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 { diff --git a/packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts b/packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts index f9bae119b78c..918924c751c8 100644 --- a/packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts +++ b/packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts @@ -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; }());'; @@ -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', () => {