From d9c3086cde35b085cf5fcf8054121fa81c24a6f9 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Fri, 27 Oct 2017 16:23:22 +0100 Subject: [PATCH 1/4] fix(@ngtools/webpack): fix tsconfig parsing and rootfile list Fix #8228 --- .../cli/models/webpack-configs/typescript.ts | 3 - packages/@ngtools/webpack/README.md | 2 +- .../webpack/src/angular_compiler_plugin.ts | 223 ++++++------------ packages/@ngtools/webpack/src/ngtools_api.ts | 11 + packages/@ngtools/webpack/src/type_checker.ts | 111 ++++----- .../assets/webpack/test-app-ng5/package.json | 20 +- .../webpack/test-app-weird-ng5/package.json | 20 +- tests/e2e/setup/500-create-project.ts | 10 + tests/e2e/tests/build/aot/angular-compiler.ts | 65 ----- tests/e2e/tests/build/aot/aot.ts | 2 +- tests/e2e/tests/build/aot/exclude.ts | 6 + tests/e2e/tests/build/build-optimizer.ts | 15 +- tests/e2e/tests/build/platform-server.ts | 4 +- tests/e2e/tests/i18n/build-locale.ts | 22 ++ tests/e2e/tests/lint/lint-no-project.ts | 7 + tests/e2e/tests/lint/lint-with-exclude.ts | 7 + tests/e2e/utils/project.ts | 14 ++ 17 files changed, 242 insertions(+), 300 deletions(-) delete mode 100644 tests/e2e/tests/build/aot/angular-compiler.ts create mode 100644 tests/e2e/tests/i18n/build-locale.ts diff --git a/packages/@angular/cli/models/webpack-configs/typescript.ts b/packages/@angular/cli/models/webpack-configs/typescript.ts index 82f5b43e25ef..4c433c4259a0 100644 --- a/packages/@angular/cli/models/webpack-configs/typescript.ts +++ b/packages/@angular/cli/models/webpack-configs/typescript.ts @@ -92,9 +92,6 @@ function _createAotPlugin(wco: WebpackConfigOptions, options: any) { missingTranslation: buildOptions.missingTranslation, hostReplacementPaths, sourceMap: buildOptions.sourcemaps, - // If we don't explicitely list excludes, it will default to `['**/*.spec.ts']`. - exclude: [], - include: options.include, }, options); return new AngularCompilerPlugin(pluginOptions); } else { diff --git a/packages/@ngtools/webpack/README.md b/packages/@ngtools/webpack/README.md index 86d7a9c0db67..6547dc675f2b 100644 --- a/packages/@ngtools/webpack/README.md +++ b/packages/@ngtools/webpack/README.md @@ -66,7 +66,7 @@ The loader works with webpack plugin to compile your TypeScript. It's important * `mainPath`. Optional if `entryModule` is specified. The `main.ts` file containing the bootstrap code. The plugin will use AST to determine the `entryModule`. * `skipCodeGeneration`. Optional, defaults to false. Disable code generation and do not refactor the code to bootstrap. This replaces `templateUrl: "string"` with `template: require("string")` (and similar for styles) to allow for webpack to properly link the resources. Only available in `AotPlugin`. * `typeChecking`. Optional, defaults to true. Enable type checking through your application. This will slow down compilation, but show syntactic and semantic errors in webpack. Only available in `AotPlugin`. -* `exclude`. Optional. Extra files to exclude from TypeScript compilation. +* `exclude`. Optional. Extra files to exclude from TypeScript compilation. Not supported with `AngularCompilerPlugin`. * `sourceMap`. Optional. Include sourcemaps. * `compilerOptions`. Optional. Override options in `tsconfig.json`. diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index 913797537bd4..20fd152e7acc 100644 --- a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts @@ -47,6 +47,7 @@ import { createProgram, createCompilerHost, formatDiagnostics, + readConfiguration, } from './ngtools_api'; import { findAstNodes } from './transformers/ast_helpers'; @@ -72,8 +73,6 @@ export interface AngularCompilerPluginOptions { platform?: PLATFORM; // Use tsconfig to include path globs. - exclude?: string | string[]; - include?: string[]; compilerOptions?: ts.CompilerOptions; } @@ -86,13 +85,11 @@ export class AngularCompilerPlugin implements Tapable { private _options: AngularCompilerPluginOptions; // TS compilation. - private _compilerOptions: ts.CompilerOptions; - private _angularCompilerOptions: CompilerOptions; - private _tsFilenames: string[]; + private _compilerOptions: CompilerOptions; + private _rootNames: string[]; private _program: (ts.Program | Program); - private _compilerHost: WebpackCompilerHost; + private _compilerHost: WebpackCompilerHost & CompilerHost; private _moduleResolutionCache: ts.ModuleResolutionCache; - private _angularCompilerHost: WebpackCompilerHost & CompilerHost; private _resourceLoader: WebpackResourceLoader; // Contains `moduleImportPath#exportName` => `fullModulePath`. private _lazyRoutes: LazyRouteMap = Object.create(null); @@ -162,63 +159,14 @@ export class AngularCompilerPlugin implements Tapable { this._basePath = basePath; - // Read the tsconfig. - const configResult = ts.readConfigFile(this._tsConfigPath, ts.sys.readFile); - if (configResult.error) { - const diagnostic = configResult.error; - const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'); - - if (diagnostic.file) { - const { line, character } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); - throw new Error(`${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message})`); - } else { - throw new Error(message); - } - } - - const tsConfigJson = configResult.config; - - // Extend compilerOptions. - if (options.hasOwnProperty('compilerOptions')) { - tsConfigJson.compilerOptions = Object.assign({}, - tsConfigJson.compilerOptions, - options.compilerOptions - ); - } - - // Default exclude to **/*.spec.ts files. - if (!options.hasOwnProperty('exclude')) { - options['exclude'] = ['**/*.spec.ts']; - } - - // Add custom excludes to default TypeScript excludes. - if (options.hasOwnProperty('exclude')) { - // If the tsconfig doesn't contain any excludes, we must add the default ones before adding - // any extra ones (otherwise we'd include all of these which can cause unexpected errors). - // This is the same logic as present in TypeScript. - if (!tsConfigJson.exclude) { - tsConfigJson['exclude'] = ['node_modules', 'bower_components', 'jspm_packages']; - if (tsConfigJson.compilerOptions && tsConfigJson.compilerOptions.outDir) { - tsConfigJson.exclude.push(tsConfigJson.compilerOptions.outDir); - } - } - - // Join our custom excludes with the existing ones. - tsConfigJson.exclude = tsConfigJson.exclude.concat(options.exclude); - } - - // Add extra includes. - if (options.hasOwnProperty('include') && Array.isArray(options.include)) { - tsConfigJson.include = tsConfigJson.include || []; - tsConfigJson.include.push(...options.include); - } - // Parse the tsconfig contents. - const tsConfig = ts.parseJsonConfigFileContent( - tsConfigJson, ts.sys, basePath, undefined, this._tsConfigPath); + const config = readConfiguration(this._tsConfigPath); + if (config.errors && config.errors.length) { + throw new Error(formatDiagnostics(config.errors)); + } - this._tsFilenames = tsConfig.fileNames; - this._compilerOptions = tsConfig.options; + this._rootNames = config.rootNames; + this._compilerOptions = config.options; // Overwrite outDir so we can find generated files next to their .ts origin in compilerHost. this._compilerOptions.outDir = ''; @@ -250,36 +198,29 @@ export class AngularCompilerPlugin implements Tapable { // to the webpack dependency tree and rebuilds triggered by file edits. this._compilerOptions.noEmitOnError = false; - // Compose Angular Compiler Options. - this._angularCompilerOptions = Object.assign( - this._compilerOptions, - tsConfig.raw['angularCompilerOptions'], - { basePath } - ); - // Set JIT (no code generation) or AOT mode. if (options.skipCodeGeneration !== undefined) { this._JitMode = options.skipCodeGeneration; } // Process i18n options. - if (options.hasOwnProperty('i18nInFile')) { - this._angularCompilerOptions.i18nInFile = options.i18nInFile; + if (options.i18nInFile !== undefined) { + this._compilerOptions.i18nInFile = options.i18nInFile; } - if (options.hasOwnProperty('i18nInFormat')) { - this._angularCompilerOptions.i18nInFormat = options.i18nInFormat; + if (options.i18nInFormat !== undefined) { + this._compilerOptions.i18nInFormat = options.i18nInFormat; } - if (options.hasOwnProperty('i18nOutFile')) { - this._angularCompilerOptions.i18nOutFile = options.i18nOutFile; + if (options.i18nOutFile !== undefined) { + this._compilerOptions.i18nOutFile = options.i18nOutFile; } - if (options.hasOwnProperty('i18nOutFormat')) { - this._angularCompilerOptions.i18nOutFormat = options.i18nOutFormat; + if (options.i18nOutFormat !== undefined) { + this._compilerOptions.i18nOutFormat = options.i18nOutFormat; } - if (options.hasOwnProperty('locale') && options.locale) { - this._angularCompilerOptions.i18nInLocale = this._validateLocale(options.locale); + if (options.locale !== undefined) { + this._compilerOptions.i18nInLocale = this._validateLocale(options.locale); } - if (options.hasOwnProperty('missingTranslation')) { - this._angularCompilerOptions.i18nInMissingTranslations = + if (options.missingTranslation !== undefined) { + this._compilerOptions.i18nInMissingTranslations = options.missingTranslation as 'error' | 'warning' | 'ignore'; } @@ -287,18 +228,24 @@ export class AngularCompilerPlugin implements Tapable { // creation. if (this._options.entryModule) { this._entryModule = this._options.entryModule; - } else if (this._angularCompilerOptions.entryModule) { + } else if (this._compilerOptions.entryModule) { this._entryModule = path.resolve(this._basePath, - this._angularCompilerOptions.entryModule); + this._compilerOptions.entryModule); } // Create the webpack compiler host. - this._compilerHost = new WebpackCompilerHost(this._compilerOptions, this._basePath); - this._compilerHost.enableCaching(); + const webpackCompilerHost = new WebpackCompilerHost(this._compilerOptions, this._basePath); + webpackCompilerHost.enableCaching(); // Create and set a new WebpackResourceLoader. this._resourceLoader = new WebpackResourceLoader(); - this._compilerHost.setResourceLoader(this._resourceLoader); + webpackCompilerHost.setResourceLoader(this._resourceLoader); + + // Use the WebpackCompilerHost with a resource loader to create an AngularCompilerHost. + this._compilerHost = createCompilerHost({ + options: this._compilerOptions, + tsHost: webpackCompilerHost + }) as CompilerHost & WebpackCompilerHost; // Override some files in the FileSystem. if (this._options.hostOverrideFileSystem) { @@ -342,28 +289,24 @@ export class AngularCompilerPlugin implements Tapable { private _createOrUpdateProgram() { return Promise.resolve() .then(() => { - const changedTsFiles = this._getChangedTsFiles(); - - changedTsFiles.forEach((file) => { - if (!this._tsFilenames.includes(file)) { - // TODO: figure out if action is needed for files that were removed from the - // compilation. - this._tsFilenames.push(file); - } - }); + // Get the root files from the ts config. + // When a new root name (like a lazy route) is added, it won't be available from + // following imports on the existing files, so we need to get the new list of root files. + this._rootNames = readConfiguration(this._tsConfigPath).rootNames; - // Update the forked type checker. + // Update the forked type checker with all changed compilation files. + // This includes templates, that also need to be reloaded on the type checker. if (this._forkTypeChecker && !this._firstRun) { - this._updateForkedTypeChecker(changedTsFiles); + this._updateForkedTypeChecker(this._rootNames, this._getChangedCompilationFiles()); } if (this._JitMode) { // Create the TypeScript program. time('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); this._program = ts.createProgram( - this._tsFilenames, - this._angularCompilerOptions, - this._angularCompilerHost, + this._rootNames, + this._compilerOptions, + this._compilerHost, this._program as ts.Program ); timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); @@ -372,26 +315,19 @@ export class AngularCompilerPlugin implements Tapable { } else { time('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); // Create the Angular program. - try { - this._program = createProgram({ - rootNames: this._tsFilenames, - options: this._angularCompilerOptions, - host: this._angularCompilerHost, - oldProgram: this._program as Program + this._program = createProgram({ + rootNames: this._rootNames, + options: this._compilerOptions, + host: this._compilerHost, + oldProgram: this._program as Program + }); + timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); + + time('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); + return this._program.loadNgStructureAsync() + .then(() => { + timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); }); - timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); - - time('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); - return this._program.loadNgStructureAsync() - .then(() => { - timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); - }); - } catch (e) { - // TODO: remove this when the issue is addressed. - // Temporary workaround for https://github.com/angular/angular/issues/19951 - this._program = undefined; - throw e; - } } }) .then(() => { @@ -412,7 +348,7 @@ export class AngularCompilerPlugin implements Tapable { const result = __NGTOOLS_PRIVATE_API_2.listLazyRoutes({ program: this._getTsProgram(), host: this._compilerHost, - angularCompilerOptions: Object.assign({}, this._angularCompilerOptions, { + angularCompilerOptions: Object.assign({}, this._compilerOptions, { // genDir seems to still be needed in @angular\compiler-cli\src\compiler_host.js:226. genDir: '' }), @@ -506,10 +442,8 @@ export class AngularCompilerPlugin implements Tapable { ); } } else { - // Found a new route, add it to the map and read it into the compiler host. + // Found a new route, add it to the map. this._lazyRoutes[moduleKey] = modulePath; - this._angularCompilerHost.readFile(lazyRouteTSFile); - this._angularCompilerHost.invalidate(lazyRouteTSFile); } }); } @@ -545,7 +479,7 @@ export class AngularCompilerPlugin implements Tapable { this._typeCheckerProcess = fork(path.resolve(__dirname, typeCheckerFile), [], forkOptions); this._typeCheckerProcess.send(new InitMessage(this._compilerOptions, this._basePath, - this._JitMode, this._tsFilenames)); + this._JitMode, this._rootNames)); // Cleanup. const killTypeCheckerProcess = () => { @@ -557,8 +491,8 @@ export class AngularCompilerPlugin implements Tapable { process.once('uncaughtException', killTypeCheckerProcess); } - private _updateForkedTypeChecker(changedTsFiles: string[]) { - this._typeCheckerProcess.send(new UpdateMessage(changedTsFiles)); + private _updateForkedTypeChecker(rootNames: string[], changedCompilationFiles: string[]) { + this._typeCheckerProcess.send(new UpdateMessage(rootNames, changedCompilationFiles)); } @@ -682,22 +616,12 @@ export class AngularCompilerPlugin implements Tapable { // Update the resource loader with the new webpack compilation. this._resourceLoader.update(compilation); + // Create a new process for the type checker on the second build if there isn't one yet. + if (this._forkTypeChecker && !this._firstRun && !this._typeCheckerProcess) { + this._createForkedTypeChecker(); + } + this._donePromise = Promise.resolve() - .then(() => { - // Create a new process for the type checker. - if (this._forkTypeChecker && !this._firstRun && !this._typeCheckerProcess) { - this._createForkedTypeChecker(); - } - }) - .then(() => { - if (this._firstRun) { - // Use the WebpackResourceLoader with a resource loader to create an AngularCompilerHost. - this._angularCompilerHost = createCompilerHost({ - options: this._angularCompilerOptions, - tsHost: this._compilerHost - }) as CompilerHost & WebpackCompilerHost; - } - }) .then(() => this._update()) .then(() => { timeEnd('AngularCompilerPlugin._make'); @@ -731,10 +655,6 @@ export class AngularCompilerPlugin implements Tapable { const changedTsFiles = this._getChangedTsFiles(); if (this._ngCompilerSupportsNewApi) { this._processLazyRoutes(this._listLazyRoutesFromProgram()); - // TODO: remove this when the issue is addressed. - // Fix for a bug in compiler where the program needs to be updated after - // _listLazyRoutesFromProgram is called. - return this._createOrUpdateProgram(); } else if (this._firstRun) { this._processLazyRoutes(this._getLazyRoutesFromNgtools()); } else if (changedTsFiles.length > 0) { @@ -742,7 +662,7 @@ export class AngularCompilerPlugin implements Tapable { } }) .then(() => { - // Build transforms, emit and report errorsn. + // Build transforms, emit and report errors. // We now have the final list of changed TS files. // Go through each changed file and add transforms as needed. @@ -772,11 +692,11 @@ export class AngularCompilerPlugin implements Tapable { } // If we have a locale, auto import the locale data file. - if (this._angularCompilerOptions.i18nInLocale) { + if (this._compilerOptions.i18nInLocale) { transformOps.push(...registerLocaleData( sf, this.entryModule, - this._angularCompilerOptions.i18nInLocale + this._compilerOptions.i18nInLocale )); } } else if (this._platform === PLATFORM.Server) { @@ -846,7 +766,7 @@ export class AngularCompilerPlugin implements Tapable { } // Write the extracted messages to disk. - const i18nOutFilePath = path.resolve(this._basePath, this._angularCompilerOptions.i18nOutFile); + const i18nOutFilePath = path.resolve(this._basePath, this._compilerOptions.i18nOutFile); const i18nOutFileContent = this._compilerHost.readFile(i18nOutFilePath); if (i18nOutFileContent) { _recursiveMkDir(path.dirname(i18nOutFilePath)) @@ -1001,7 +921,7 @@ export class AngularCompilerPlugin implements Tapable { if (!hasErrors(allDiagnostics)) { time('AngularCompilerPlugin._emit.ng.emit'); - const extractI18n = !!this._angularCompilerOptions.i18nOutFile; + const extractI18n = !!this._compilerOptions.i18nOutFile; const emitFlags = extractI18n ? EmitFlags.I18nBundle : EmitFlags.Default; emitResult = angularProgram.emit({ emitFlags, customTransformers }); allDiagnostics.push(...emitResult.diagnostics); @@ -1009,9 +929,6 @@ export class AngularCompilerPlugin implements Tapable { this.writeI18nOutFile(); } timeEnd('AngularCompilerPlugin._emit.ng.emit'); - } else { - // Throw away the old program if there was an error. - this._program = undefined; } } } catch (e) { diff --git a/packages/@ngtools/webpack/src/ngtools_api.ts b/packages/@ngtools/webpack/src/ngtools_api.ts index 4f37069ac08b..e66ea1a71809 100644 --- a/packages/@ngtools/webpack/src/ngtools_api.ts +++ b/packages/@ngtools/webpack/src/ngtools_api.ts @@ -112,6 +112,16 @@ export interface CreateCompilerHostInterface { export interface FormatDiagnosticsInterface { (diags: Diagnostics): string; } +export interface ParsedConfiguration { + project: string; + options: CompilerOptions; + rootNames: string[]; + emitFlags: any; + errors: Diagnostics; +} +export interface ReadConfigurationInterface { + (project: string, existingOptions?: ts.CompilerOptions): ParsedConfiguration; +} // Manually check for Compiler CLI availability and supported version. // This is needed because @ngtools/webpack does not depend directly on @angular/compiler-cli, since @@ -173,4 +183,5 @@ try { export const createProgram: CreateProgramInterface = ngtools2.createProgram; export const createCompilerHost: CreateCompilerHostInterface = ngtools2.createCompilerHost; export const formatDiagnostics: FormatDiagnosticsInterface = ngtools2.formatDiagnostics; +export const readConfiguration: ReadConfigurationInterface = compilerCli.readConfiguration; export const EmitFlags = ngtools2.EmitFlags; diff --git a/packages/@ngtools/webpack/src/type_checker.ts b/packages/@ngtools/webpack/src/type_checker.ts index f761fb447574..e17f363b78ff 100644 --- a/packages/@ngtools/webpack/src/type_checker.ts +++ b/packages/@ngtools/webpack/src/type_checker.ts @@ -34,14 +34,14 @@ export class InitMessage extends TypeCheckerMessage { public compilerOptions: ts.CompilerOptions, public basePath: string, public jitMode: boolean, - public tsFilenames: string[], + public rootNames: string[], ) { super(MESSAGE_KIND.Init); } } export class UpdateMessage extends TypeCheckerMessage { - constructor(public changedTsFiles: string[]) { + constructor(public rootNames: string[], public changedCompilationFiles: string[]) { super(MESSAGE_KIND.Update); } } @@ -51,36 +51,32 @@ let lastCancellationToken: CancellationToken; process.on('message', (message: TypeCheckerMessage) => { time('TypeChecker.message'); - try { - switch (message.kind) { - case MESSAGE_KIND.Init: - const initMessage = message as InitMessage; - typeChecker = new TypeChecker( - initMessage.compilerOptions, - initMessage.basePath, - initMessage.jitMode, - initMessage.tsFilenames, - ); - break; - case MESSAGE_KIND.Update: - if (!typeChecker) { - throw new Error('TypeChecker: update message received before initialization'); - } - if (lastCancellationToken) { - // This cancellation token doesn't seem to do much, messages don't seem to be processed - // before the diagnostics finish. - lastCancellationToken.requestCancellation(); - } - const updateMessage = message as UpdateMessage; - lastCancellationToken = new CancellationToken(); - typeChecker.update(updateMessage.changedTsFiles, lastCancellationToken); - break; - default: - throw new Error(`TypeChecker: Unexpected message received: ${message}.`); - } - } catch (error) { - // Ignore errors in the TypeChecker. - // Anything that would throw here will error out the compilation as well. + switch (message.kind) { + case MESSAGE_KIND.Init: + const initMessage = message as InitMessage; + typeChecker = new TypeChecker( + initMessage.compilerOptions, + initMessage.basePath, + initMessage.jitMode, + initMessage.rootNames, + ); + break; + case MESSAGE_KIND.Update: + if (!typeChecker) { + throw new Error('TypeChecker: update message received before initialization'); + } + if (lastCancellationToken) { + // This cancellation token doesn't seem to do much, messages don't seem to be processed + // before the diagnostics finish. + lastCancellationToken.requestCancellation(); + } + const updateMessage = message as UpdateMessage; + lastCancellationToken = new CancellationToken(); + typeChecker.update(updateMessage.rootNames, updateMessage.changedCompilationFiles, + lastCancellationToken); + break; + default: + throw new Error(`TypeChecker: Unexpected message received: ${message}.`); } timeEnd('TypeChecker.message'); }); @@ -88,33 +84,36 @@ process.on('message', (message: TypeCheckerMessage) => { class TypeChecker { private _program: ts.Program | Program; - private _angularCompilerHost: WebpackCompilerHost & CompilerHost; + private _compilerHost: WebpackCompilerHost & CompilerHost; constructor( - private _angularCompilerOptions: CompilerOptions, + private _compilerOptions: CompilerOptions, _basePath: string, private _JitMode: boolean, - private _tsFilenames: string[], + private _rootNames: string[], ) { time('TypeChecker.constructor'); - const compilerHost = new WebpackCompilerHost(_angularCompilerOptions, _basePath); + const compilerHost = new WebpackCompilerHost(_compilerOptions, _basePath); compilerHost.enableCaching(); - this._angularCompilerHost = createCompilerHost({ - options: this._angularCompilerOptions, + // We don't set a async resource loader on the compiler host because we only support + // html templates, which are the only ones that can throw errors, and those can be loaded + // synchronously. + // If we need to also report errors on styles then we'll need to ask the main thread + // for these resources. + this._compilerHost = createCompilerHost({ + options: this._compilerOptions, tsHost: compilerHost }) as CompilerHost & WebpackCompilerHost; timeEnd('TypeChecker.constructor'); } - private _updateTsFilenames(changedTsFiles: string[]) { - time('TypeChecker._updateTsFilenames'); - changedTsFiles.forEach((fileName) => { - this._angularCompilerHost.invalidate(fileName); - if (!this._tsFilenames.includes(fileName)) { - this._tsFilenames.push(fileName); - } + private _update(rootNames: string[], changedCompilationFiles: string[]) { + time('TypeChecker._update'); + this._rootNames = rootNames; + changedCompilationFiles.forEach((fileName) => { + this._compilerHost.invalidate(fileName); }); - timeEnd('TypeChecker._updateTsFilenames'); + timeEnd('TypeChecker._update'); } private _createOrUpdateProgram() { @@ -122,9 +121,9 @@ class TypeChecker { // Create the TypeScript program. time('TypeChecker._createOrUpdateProgram.ts.createProgram'); this._program = ts.createProgram( - this._tsFilenames, - this._angularCompilerOptions, - this._angularCompilerHost, + this._rootNames, + this._compilerOptions, + this._compilerHost, this._program as ts.Program ) as ts.Program; timeEnd('TypeChecker._createOrUpdateProgram.ts.createProgram'); @@ -132,9 +131,9 @@ class TypeChecker { time('TypeChecker._createOrUpdateProgram.ng.createProgram'); // Create the Angular program. this._program = createProgram({ - rootNames: this._tsFilenames, - options: this._angularCompilerOptions, - host: this._angularCompilerHost, + rootNames: this._rootNames, + options: this._compilerOptions, + host: this._compilerHost, oldProgram: this._program as Program }) as Program; timeEnd('TypeChecker._createOrUpdateProgram.ng.createProgram'); @@ -153,6 +152,9 @@ class TypeChecker { if (errors.length > 0) { const message = formatDiagnostics(errors); console.error(bold(red('ERROR in ' + message))); + } else { + // Reset the changed file tracker only if there are no errors. + this._compilerHost.resetChangedFileTracker(); } if (warnings.length > 0) { @@ -162,8 +164,9 @@ class TypeChecker { } } - public update(changedTsFiles: string[], cancellationToken: CancellationToken) { - this._updateTsFilenames(changedTsFiles); + public update(rootNames: string[], changedCompilationFiles: string[], + cancellationToken: CancellationToken) { + this._update(rootNames, changedCompilationFiles); this._createOrUpdateProgram(); this._diagnose(cancellationToken); } diff --git a/tests/e2e/assets/webpack/test-app-ng5/package.json b/tests/e2e/assets/webpack/test-app-ng5/package.json index 162363bc8c19..0f814bdc97c0 100644 --- a/tests/e2e/assets/webpack/test-app-ng5/package.json +++ b/tests/e2e/assets/webpack/test-app-ng5/package.json @@ -2,18 +2,18 @@ "name": "test", "license": "MIT", "dependencies": { - "@angular/common": "^5.0.0-beta.5", - "@angular/compiler": "^5.0.0-beta.5", - "@angular/compiler-cli": "^5.0.0-beta.5", - "@angular/core": "^5.0.0-beta.5", - "@angular/http": "^5.0.0-beta.5", - "@angular/platform-browser": "^5.0.0-beta.5", - "@angular/platform-browser-dynamic": "^5.0.0-beta.5", - "@angular/platform-server": "^5.0.0-beta.5", - "@angular/router": "^5.0.0-beta.5", + "@angular/common": "^5.0.0-rc.8", + "@angular/compiler": "^5.0.0-rc.8", + "@angular/compiler-cli": "^5.0.0-rc.8", + "@angular/core": "^5.0.0-rc.8", + "@angular/http": "^5.0.0-rc.8", + "@angular/platform-browser": "^5.0.0-rc.8", + "@angular/platform-browser-dynamic": "^5.0.0-rc.8", + "@angular/platform-server": "^5.0.0-rc.8", + "@angular/router": "^5.0.0-rc.8", "@ngtools/webpack": "0.0.0", "core-js": "^2.4.1", - "rxjs": "^5.4.2", + "rxjs": "^5.5.0", "zone.js": "^0.8.14" }, "devDependencies": { diff --git a/tests/e2e/assets/webpack/test-app-weird-ng5/package.json b/tests/e2e/assets/webpack/test-app-weird-ng5/package.json index b3ff99540a73..0a81cf73f81e 100644 --- a/tests/e2e/assets/webpack/test-app-weird-ng5/package.json +++ b/tests/e2e/assets/webpack/test-app-weird-ng5/package.json @@ -2,18 +2,18 @@ "name": "test", "license": "MIT", "dependencies": { - "@angular/common": "^5.0.0-beta.5", - "@angular/compiler": "^5.0.0-beta.5", - "@angular/compiler-cli": "^5.0.0-beta.5", - "@angular/core": "^5.0.0-beta.5", - "@angular/http": "^5.0.0-beta.5", - "@angular/platform-browser": "^5.0.0-beta.5", - "@angular/platform-browser-dynamic": "^5.0.0-beta.5", - "@angular/platform-server": "^5.0.0-beta.5", - "@angular/router": "^5.0.0-beta.5", + "@angular/common": "^5.0.0-rc.8", + "@angular/compiler": "^5.0.0-rc.8", + "@angular/compiler-cli": "^5.0.0-rc.8", + "@angular/core": "^5.0.0-rc.8", + "@angular/http": "^5.0.0-rc.8", + "@angular/platform-browser": "^5.0.0-rc.8", + "@angular/platform-browser-dynamic": "^5.0.0-rc.8", + "@angular/platform-server": "^5.0.0-rc.8", + "@angular/router": "^5.0.0-rc.8", "@ngtools/webpack": "0.0.0", "core-js": "^2.4.1", - "rxjs": "^5.4.2", + "rxjs": "^5.5.0", "zone.js": "^0.8.14" }, "devDependencies": { diff --git a/tests/e2e/setup/500-create-project.ts b/tests/e2e/setup/500-create-project.ts index b4aa8f108e99..300e5b3f1cca 100644 --- a/tests/e2e/setup/500-create-project.ts +++ b/tests/e2e/setup/500-create-project.ts @@ -52,6 +52,16 @@ export default function() { .then(() => updateTsConfig(json => { json['compilerOptions']['sourceRoot'] = '/'; })) + .then(() => updateJsonFile('src/tsconfig.spec.json', json => { + if (argv.nightly) { + // ************************************************************************************* + // REMOVE THIS WITH UPDATED NG5 SCHEMATICS + // In ng5 we have to tell users users to update their tsconfig.json. + // `src/tsconfig.spec.json` needs to be updated with `"include": [ "**/*.ts" ]` + // ************************************************************************************* + json['include'] = ['**/*.ts']; + } + })) .then(() => git('config', 'user.email', 'angular-core+e2e@google.com')) .then(() => git('config', 'user.name', 'Angular CLI E2e')) .then(() => git('config', 'commit.gpgSign', 'false')) diff --git a/tests/e2e/tests/build/aot/angular-compiler.ts b/tests/e2e/tests/build/aot/angular-compiler.ts deleted file mode 100644 index 4d1a1aedf146..000000000000 --- a/tests/e2e/tests/build/aot/angular-compiler.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { ng, silentNpm } from '../../../utils/process'; -import { updateJsonFile } from '../../../utils/project'; -import { expectFileToMatch, rimraf, moveFile, expectFileToExist } from '../../../utils/fs'; -import { getGlobalVariable } from '../../../utils/env'; -import { expectToFail } from '../../../utils/utils'; - - -// THIS TEST REQUIRES TO MOVE NODE_MODULES AND MOVE IT BACK. -export default function () { - // Skip this in ejected tests. - // Something goes wrong when installing node modules after eject. Remove this in ng5. - if (getGlobalVariable('argv').eject) { - return Promise.resolve(); - } - - // These tests should be moved to the default when we use ng5 in new projects. - return Promise.resolve() - .then(() => moveFile('node_modules', '../node_modules')) - .then(() => updateJsonFile('package.json', packageJson => { - const dependencies = packageJson['dependencies']; - dependencies['@angular/animations'] = '5.0.0-beta.6'; - dependencies['@angular/common'] = '5.0.0-beta.6'; - dependencies['@angular/compiler'] = '5.0.0-beta.6'; - dependencies['@angular/core'] = '5.0.0-beta.6'; - dependencies['@angular/forms'] = '5.0.0-beta.6'; - dependencies['@angular/http'] = '5.0.0-beta.6'; - dependencies['@angular/platform-browser'] = '5.0.0-beta.6'; - dependencies['@angular/platform-browser-dynamic'] = '5.0.0-beta.6'; - dependencies['@angular/router'] = '5.0.0-beta.6'; - const devDependencies = packageJson['devDependencies']; - devDependencies['@angular/compiler-cli'] = '5.0.0-beta.6'; - devDependencies['@angular/language-service'] = '5.0.0-beta.6'; - devDependencies['typescript'] = '2.4.2'; - })) - .then(() => silentNpm('install')) - .then(() => ng('build', '--aot')) - .then(() => expectFileToMatch('dist/main.bundle.js', - /bootstrapModuleFactory.*\/\* AppModuleNgFactory \*\//)) - - // Build optimizer should default to true on prod builds. - .then(() => ng('build', '--prod', '--output-hashing=none')) - .then(() => expectToFail(() => expectFileToExist('dist/vendor.js'))) - .then(() => expectToFail(() => expectFileToMatch('dist/main.js', /\.decorators =/))) - - // tests for register_locale_data transformer - .then(() => rimraf('dist')) - .then(() => ng('build', '--locale=fr')) - .then(() => expectFileToMatch('dist/main.bundle.js', - /registerLocaleData/)) - .then(() => expectFileToMatch('dist/main.bundle.js', - /angular_common_locales_fr/)) - .then(() => rimraf('dist')) - .then(() => expectToFail(() => - ng('build', '--locale=no-locale'))) - - // Cleanup - .then(() => { - return rimraf('node_modules') - .then(() => moveFile('../node_modules', 'node_modules')); - }, (err: any) => { - return rimraf('node_modules') - .then(() => moveFile('../node_modules', 'node_modules')) - .then(() => { throw err; }); - }); -} diff --git a/tests/e2e/tests/build/aot/aot.ts b/tests/e2e/tests/build/aot/aot.ts index d10cca8bf7a3..bbeb6c2004d4 100644 --- a/tests/e2e/tests/build/aot/aot.ts +++ b/tests/e2e/tests/build/aot/aot.ts @@ -4,5 +4,5 @@ import {expectFileToMatch} from '../../../utils/fs'; export default function() { return ng('build', '--aot') .then(() => expectFileToMatch('dist/main.bundle.js', - /bootstrapModuleFactory.*\/\* AppModuleNgFactory \*\//)); + /platformBrowser.*bootstrapModuleFactory.*AppModuleNgFactory/)); } diff --git a/tests/e2e/tests/build/aot/exclude.ts b/tests/e2e/tests/build/aot/exclude.ts index 98904d0f9f22..94b474832f23 100644 --- a/tests/e2e/tests/build/aot/exclude.ts +++ b/tests/e2e/tests/build/aot/exclude.ts @@ -7,6 +7,12 @@ export default function () { // Disable parts of it in webpack tests. const ejected = getGlobalVariable('argv').eject; + // Skip this in ng5 tests, it only happens in ng2/4. + // This check should be changed once ng5 because the default. + if (getGlobalVariable('argv').nightly) { + return Promise.resolve(); + } + // Check if **/*.spec.ts files are excluded by default. return Promise.resolve() // This import would cause aot to fail. diff --git a/tests/e2e/tests/build/build-optimizer.ts b/tests/e2e/tests/build/build-optimizer.ts index 3ff84fdce3c1..6e0dd82b2bdf 100644 --- a/tests/e2e/tests/build/build-optimizer.ts +++ b/tests/e2e/tests/build/build-optimizer.ts @@ -1,10 +1,23 @@ import { ng } from '../../utils/process'; import { expectFileToMatch, expectFileToExist } from '../../utils/fs'; import { expectToFail } from '../../utils/utils'; +import { getGlobalVariable } from '../../utils/env'; export default function () { return ng('build', '--aot', '--build-optimizer') .then(() => expectToFail(() => expectFileToExist('dist/vendor.js'))) - .then(() => expectToFail(() => expectFileToMatch('dist/main.js', /\.decorators =/))); + .then(() => expectToFail(() => expectFileToMatch('dist/main.js', /\.decorators =/))) + .then(() => { + // Check if build optimizer is on by default in ng5 prod builds + // This check should be changed once ng5 because the default. + if (!getGlobalVariable('argv').nightly) { + return Promise.resolve(); + } + + return Promise.resolve() + .then(() => ng('build', '--prod')) + .then(() => expectToFail(() => expectFileToExist('dist/vendor.js'))) + .then(() => expectToFail(() => expectFileToMatch('dist/main.js', /\.decorators =/))); + }); } diff --git a/tests/e2e/tests/build/platform-server.ts b/tests/e2e/tests/build/platform-server.ts index f3a238e4c55b..a0942f2d94c3 100644 --- a/tests/e2e/tests/build/platform-server.ts +++ b/tests/e2e/tests/build/platform-server.ts @@ -55,7 +55,7 @@ export default function () { .then(() => ng('build', '--aot=false')) // files were created successfully .then(() => expectFileToMatch('dist/main.bundle.js', - /__webpack_exports__, "AppModule"/)) + /exports.*AppModule/)) .then(() => writeFile('./index.js', ` require('zone.js/dist/zone-node'); require('reflect-metadata'); @@ -76,7 +76,7 @@ export default function () { .then(() => ng('build', '--aot')) // files were created successfully .then(() => expectFileToMatch('dist/main.bundle.js', - /__webpack_exports__, "AppModuleNgFactory"/)) + /exports.*AppModuleNgFactory/)) .then(() => replaceInFile('./index.js', /AppModule/g, 'AppModuleNgFactory')) .then(() => replaceInFile('./index.js', /renderModule/g, 'renderModuleFactory')) .then(() => exec(normalize('node'), 'index.js')) diff --git a/tests/e2e/tests/i18n/build-locale.ts b/tests/e2e/tests/i18n/build-locale.ts new file mode 100644 index 000000000000..9154440d947c --- /dev/null +++ b/tests/e2e/tests/i18n/build-locale.ts @@ -0,0 +1,22 @@ +import { ng } from '../../utils/process'; +import { expectFileToMatch, rimraf } from '../../utils/fs'; +import { getGlobalVariable } from '../../utils/env'; +import { expectToFail } from '../../utils/utils'; + + +export default function () { + // Check if register locale works in ng5 prod builds + // This check should be changed once ng5 because the default. + if (!getGlobalVariable('argv').nightly) { + return Promise.resolve(); + } + + // These tests should be moved to the default when we use ng5 in new projects. + return Promise.resolve() + // tests for register_locale_data transformer + .then(() => ng('build', '--locale=fr')) + .then(() => expectFileToMatch('dist/main.bundle.js', /registerLocaleData/)) + .then(() => expectFileToMatch('dist/main.bundle.js', /angular_common_locales_fr/)) + .then(() => rimraf('dist')) + .then(() => expectToFail(() => ng('build', '--locale=no-locale'))) +} diff --git a/tests/e2e/tests/lint/lint-no-project.ts b/tests/e2e/tests/lint/lint-no-project.ts index 9e374ef040e4..ccec63a0b97f 100644 --- a/tests/e2e/tests/lint/lint-no-project.ts +++ b/tests/e2e/tests/lint/lint-no-project.ts @@ -24,6 +24,13 @@ export default function () { return stdout; }) .then(() => ng('set', 'lint.0.files', '"**/baz.ts"')) + .then(() => { + // Starting with ng5, tsconfig.spec.json includes all ts files, so linting for it must + // also set the files. + if (getGlobalVariable('argv').nightly) { + return ng('set', 'lint.1.files', '"**/baz.ts"'); + } + }) .then(() => writeFile('src/app/foo.ts', 'const foo = "";\n')) .then(() => writeFile('src/app/baz.ts', 'const baz = \'\';\n')) .then(() => ng('lint')) diff --git a/tests/e2e/tests/lint/lint-with-exclude.ts b/tests/e2e/tests/lint/lint-with-exclude.ts index be792e2e9530..996485356c72 100644 --- a/tests/e2e/tests/lint/lint-with-exclude.ts +++ b/tests/e2e/tests/lint/lint-with-exclude.ts @@ -13,6 +13,13 @@ export default function () { return Promise.resolve() .then(() => ng('set', 'lint.0.exclude', '"**/foo.ts"')) + .then(() => { + // Starting with ng5, tsconfig.spec.json includes all ts files, so linting for it must + // also set the files. + if (getGlobalVariable('argv').nightly) { + return ng('set', 'lint.1.exclude', '"**/foo.ts"'); + } + }) .then(() => writeFile(fileName, 'const foo = "";\n')) .then(() => ng('lint')) .then(({ stdout }) => { diff --git a/tests/e2e/utils/project.ts b/tests/e2e/utils/project.ts index a11784cfbd12..8d69782b1278 100644 --- a/tests/e2e/utils/project.ts +++ b/tests/e2e/utils/project.ts @@ -43,6 +43,20 @@ export function createProject(name: string, ...args: string[]) { .then(() => useCIDefaults()) .then(() => argv['ng2'] ? useNg2() : Promise.resolve()) .then(() => argv.nightly || argv['ng-sha'] ? useSha() : Promise.resolve()) + .then(() => { + // ************************************************************************************* + // REMOVE THIS WITH UPDATED NG5 SCHEMATICS + // In ng5 we have to tell users users to update their tsconfig.json. + // `src/tsconfig.spec.json` needs to be updated with `"include": [ "**/*.ts" ]` + // ************************************************************************************* + return updateJsonFile('src/tsconfig.spec.json', json => { + if (argv.nightly) { + json['include'] = ['**/*.ts']; + } + }) + // Ignore error if file doesn't exist. + .catch(() => { return; }); + }) .then(() => console.log(`Project ${name} created... Installing npm.`)) .then(() => silentNpm('install')); } From b4c94b4e8e9536d0972385a5d5ce450faa86e8bc Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 30 Oct 2017 16:32:49 +0000 Subject: [PATCH 2/4] fix(@ngtools/webpack): allow multiple transforms on the same file Fix #8234 Fix #8207 --- .../webpack/src/angular_compiler_plugin.ts | 145 ++++++++------- packages/@ngtools/webpack/src/refactor.ts | 54 +++++- .../webpack/src/transformers/ast_helpers.ts | 79 ++------- .../export_lazy_module_map.spec.ts | 67 ++++--- .../transformers/export_lazy_module_map.ts | 126 +++++++------ .../src/transformers/export_ngfactory.spec.ts | 32 +++- .../src/transformers/export_ngfactory.ts | 98 +++++----- .../webpack/src/transformers/index.ts | 1 + .../webpack/src/transformers/insert_import.ts | 8 +- .../webpack/src/transformers/interfaces.ts | 39 ++++ .../src/transformers/make_transform.ts | 64 +++---- .../multiple_transformers.spec.ts | 64 +++++++ .../transformers/register_locale_data.spec.ts | 32 +++- .../src/transformers/register_locale_data.ts | 138 ++++++++------- .../webpack/src/transformers/remove_import.ts | 10 +- .../transformers/replace_bootstrap.spec.ts | 39 +++- .../src/transformers/replace_bootstrap.ts | 167 ++++++++++-------- .../transformers/replace_resources.spec.ts | 43 ++++- .../src/transformers/replace_resources.ts | 78 ++++---- 19 files changed, 771 insertions(+), 513 deletions(-) create mode 100644 packages/@ngtools/webpack/src/transformers/interfaces.ts create mode 100644 packages/@ngtools/webpack/src/transformers/multiple_transformers.spec.ts diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index 20fd152e7acc..e2165ceb275a 100644 --- a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts @@ -18,8 +18,6 @@ import { } from './virtual_file_system_decorator'; import { resolveEntryModuleFromMain } from './entry_resolver'; import { - TransformOperation, - makeTransform, replaceBootstrap, exportNgFactory, exportLazyModuleMap, @@ -41,7 +39,6 @@ import { CompilerOptions, CompilerHost, Diagnostics, - CustomTransformers, EmitFlags, LazyRoute, createProgram, @@ -49,7 +46,7 @@ import { formatDiagnostics, readConfiguration, } from './ngtools_api'; -import { findAstNodes } from './transformers/ast_helpers'; +import { collectDeepNodes } from './transformers/ast_helpers'; /** @@ -95,8 +92,9 @@ export class AngularCompilerPlugin implements Tapable { private _lazyRoutes: LazyRouteMap = Object.create(null); private _tsConfigPath: string; private _entryModule: string; + private _mainPath: string | undefined; private _basePath: string; - private _transformMap: Map = new Map(); + private _transformers: ts.TransformerFactory[] = []; private _platform: PLATFORM; private _JitMode = false; private _emitSkipped = true; @@ -128,6 +126,9 @@ export class AngularCompilerPlugin implements Tapable { get options() { return this._options; } get done() { return this._donePromise; } get entryModule() { + if (!this._entryModule) { + return undefined; + } const splitted = this._entryModule.split('#'); const path = splitted[0]; const className = splitted[1] || 'default'; @@ -157,6 +158,7 @@ export class AngularCompilerPlugin implements Tapable { basePath = path.resolve(process.cwd(), options.basePath); } + // TODO: check if we can get this from readConfiguration this._basePath = basePath; // Parse the tsconfig contents. @@ -224,15 +226,6 @@ export class AngularCompilerPlugin implements Tapable { options.missingTranslation as 'error' | 'warning' | 'ignore'; } - // Use entryModule if available in options, otherwise resolve it from mainPath after program - // creation. - if (this._options.entryModule) { - this._entryModule = this._options.entryModule; - } else if (this._compilerOptions.entryModule) { - this._entryModule = path.resolve(this._basePath, - this._compilerOptions.entryModule); - } - // Create the webpack compiler host. const webpackCompilerHost = new WebpackCompilerHost(this._compilerOptions, this._basePath); webpackCompilerHost.enableCaching(); @@ -266,8 +259,26 @@ export class AngularCompilerPlugin implements Tapable { // Use an identity function as all our paths are absolute already. this._moduleResolutionCache = ts.createModuleResolutionCache(this._basePath, x => x); + // Resolve mainPath if provided. + if (options.mainPath) { + this._mainPath = this._compilerHost.resolve(options.mainPath); + } + + // Use entryModule if available in options, otherwise resolve it from mainPath after program + // creation. + if (this._options.entryModule) { + this._entryModule = this._options.entryModule; + } else if (this._compilerOptions.entryModule) { + this._entryModule = path.resolve(this._basePath, + this._compilerOptions.entryModule); + } + // Set platform. this._platform = options.platform || PLATFORM.Browser; + + // Make transformers. + this._makeTransformers(); + timeEnd('AngularCompilerPlugin._setupOptions'); } @@ -332,11 +343,10 @@ export class AngularCompilerPlugin implements Tapable { }) .then(() => { // If there's still no entryModule try to resolve from mainPath. - if (!this._entryModule && this._options.mainPath) { + if (!this._entryModule && this._mainPath) { time('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); - const mainPath = path.resolve(this._basePath, this._options.mainPath); this._entryModule = resolveEntryModuleFromMain( - mainPath, this._compilerHost, this._getTsProgram()); + this._mainPath, this._compilerHost, this._getTsProgram()); timeEnd('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); } }); @@ -633,6 +643,41 @@ export class AngularCompilerPlugin implements Tapable { }); } + private _makeTransformers() { + + // TODO use compilerhost.denormalize when #8210 is merged. + const isAppPath = (fileName: string) => + this._rootNames.includes(fileName.replace(/\//g, path.sep)); + const isMainPath = (fileName: string) => fileName === this._mainPath; + const getEntryModule = () => this.entryModule; + const getLazyRoutes = () => this._lazyRoutes; + + if (this._JitMode) { + // Replace resources in JIT. + this._transformers.push(replaceResources(isAppPath)); + } + + if (this._platform === PLATFORM.Browser) { + // If we have a locale, auto import the locale data file. + // This transform must go before replaceBootstrap because it looks for the entry module + // import, which will be replaced. + if (this._compilerOptions.i18nInLocale) { + this._transformers.push(registerLocaleData(isAppPath, getEntryModule, + this._compilerOptions.i18nInLocale)); + } + + if (!this._JitMode) { + // Replace bootstrap in browser AOT. + this._transformers.push(replaceBootstrap(isAppPath, getEntryModule)); + } + } else if (this._platform === PLATFORM.Server) { + this._transformers.push(exportLazyModuleMap(isMainPath, getLazyRoutes)); + if (!this._JitMode) { + this._transformers.push(exportNgFactory(isMainPath, getEntryModule)); + } + } + } + private _update() { time('AngularCompilerPlugin._update'); // We only want to update on TS and template changes, but all kinds of files are on this @@ -662,7 +707,7 @@ export class AngularCompilerPlugin implements Tapable { } }) .then(() => { - // Build transforms, emit and report errors. + // Emit and report errors. // We now have the final list of changed TS files. // Go through each changed file and add transforms as needed. @@ -677,56 +722,9 @@ export class AngularCompilerPlugin implements Tapable { return sourceFile; }); - time('AngularCompilerPlugin._update.transformOps'); - sourceFiles.forEach((sf) => { - const fileName = this._compilerHost.resolve(sf.fileName); - let transformOps = []; - - if (this._JitMode) { - transformOps.push(...replaceResources(sf)); - } - - if (this._platform === PLATFORM.Browser) { - if (!this._JitMode) { - transformOps.push(...replaceBootstrap(sf, this.entryModule)); - } - - // If we have a locale, auto import the locale data file. - if (this._compilerOptions.i18nInLocale) { - transformOps.push(...registerLocaleData( - sf, - this.entryModule, - this._compilerOptions.i18nInLocale - )); - } - } else if (this._platform === PLATFORM.Server) { - if (fileName === this._compilerHost.resolve(this._options.mainPath)) { - transformOps.push(...exportLazyModuleMap(sf, this._lazyRoutes)); - if (!this._JitMode) { - transformOps.push(...exportNgFactory(sf, this.entryModule)); - } - } - } - - // We need to keep a map of transforms for each file, to reapply on each update. - this._transformMap.set(fileName, transformOps); - }); - - const transformOps: TransformOperation[] = []; - for (let fileTransformOps of this._transformMap.values()) { - transformOps.push(...fileTransformOps); - } - timeEnd('AngularCompilerPlugin._update.transformOps'); - - time('AngularCompilerPlugin._update.makeTransform'); - const transformers: CustomTransformers = { - beforeTs: transformOps.length > 0 ? [makeTransform(transformOps)] : [] - }; - timeEnd('AngularCompilerPlugin._update.makeTransform'); - // Emit files. time('AngularCompilerPlugin._update._emit'); - const { emitResult, diagnostics } = this._emit(sourceFiles, transformers); + const { emitResult, diagnostics } = this._emit(sourceFiles); timeEnd('AngularCompilerPlugin._update._emit'); // Report diagnostics. @@ -826,7 +824,7 @@ export class AngularCompilerPlugin implements Tapable { const host = this._compilerHost; const cache = this._moduleResolutionCache; - const esImports = findAstNodes(null, sourceFile, + const esImports = collectDeepNodes(sourceFile, ts.SyntaxKind.ImportDeclaration) .map(decl => { const moduleName = (decl.moduleSpecifier as ts.StringLiteral).text; @@ -858,10 +856,7 @@ export class AngularCompilerPlugin implements Tapable { // This code mostly comes from `performCompilation` in `@angular/compiler-cli`. // It skips the program creation because we need to use `loadNgStructureAsync()`, // and uses CustomTransformers. - private _emit( - sourceFiles: ts.SourceFile[], - customTransformers: ts.CustomTransformers & CustomTransformers - ) { + private _emit(sourceFiles: ts.SourceFile[]) { time('AngularCompilerPlugin._emit'); const program = this._program; const allDiagnostics: Diagnostics = []; @@ -888,7 +883,7 @@ export class AngularCompilerPlugin implements Tapable { const timeLabel = `AngularCompilerPlugin._emit.ts+${sf.fileName}+.emit`; time(timeLabel); emitResult = tsProgram.emit(sf, undefined, undefined, undefined, - { before: customTransformers.beforeTs } + { before: this._transformers } ); allDiagnostics.push(...emitResult.diagnostics); timeEnd(timeLabel); @@ -923,7 +918,11 @@ export class AngularCompilerPlugin implements Tapable { time('AngularCompilerPlugin._emit.ng.emit'); const extractI18n = !!this._compilerOptions.i18nOutFile; const emitFlags = extractI18n ? EmitFlags.I18nBundle : EmitFlags.Default; - emitResult = angularProgram.emit({ emitFlags, customTransformers }); + emitResult = angularProgram.emit({ + emitFlags, customTransformers: { + beforeTs: this._transformers + } + }); allDiagnostics.push(...emitResult.diagnostics); if (extractI18n) { this.writeI18nOutFile(); diff --git a/packages/@ngtools/webpack/src/refactor.ts b/packages/@ngtools/webpack/src/refactor.ts index 71356fdbd913..1d5d040b6375 100644 --- a/packages/@ngtools/webpack/src/refactor.ts +++ b/packages/@ngtools/webpack/src/refactor.ts @@ -3,11 +3,63 @@ import * as path from 'path'; import * as ts from 'typescript'; import {SourceMapConsumer, SourceMapGenerator} from 'source-map'; -import { findAstNodes } from './transformers'; const MagicString = require('magic-string'); +/** + * Find all nodes from the AST in the subtree of node of SyntaxKind kind. + * @param node The root node to check, or null if the whole tree should be searched. + * @param sourceFile The source file where the node is. + * @param kind The kind of nodes to find. + * @param recursive Whether to go in matched nodes to keep matching. + * @param max The maximum number of items to return. + * @return all nodes of kind, or [] if none is found + */ +export function findAstNodes( + node: ts.Node | null, + sourceFile: ts.SourceFile, + kind: ts.SyntaxKind, + recursive = false, + max = Infinity +): T[] { + // TODO: refactor operations that only need `refactor.findAstNodes()` to use this instead. + if (max == 0) { + return []; + } + if (!node) { + node = sourceFile; + } + + let arr: T[] = []; + if (node.kind === kind) { + // If we're not recursively looking for children, stop here. + if (!recursive) { + return [node as T]; + } + + arr.push(node as T); + max--; + } + + if (max > 0) { + for (const child of node.getChildren(sourceFile)) { + findAstNodes(child, sourceFile, kind, recursive, max) + .forEach((node: ts.Node) => { + if (max > 0) { + arr.push(node as T); + } + max--; + }); + + if (max <= 0) { + break; + } + } + } + return arr; +} + export interface TranspileOutput { outputText: string; sourceMap: any | null; diff --git a/packages/@ngtools/webpack/src/transformers/ast_helpers.ts b/packages/@ngtools/webpack/src/transformers/ast_helpers.ts index 8e59157877eb..0a033eb7ba83 100644 --- a/packages/@ngtools/webpack/src/transformers/ast_helpers.ts +++ b/packages/@ngtools/webpack/src/transformers/ast_helpers.ts @@ -1,75 +1,32 @@ // @ignoreDep typescript import * as ts from 'typescript'; import { WebpackCompilerHost } from '../compiler_host'; -import { makeTransform, TransformOperation } from './make_transform'; -/** - * Find all nodes from the AST in the subtree of node of SyntaxKind kind. - * @param node The root node to check, or null if the whole tree should be searched. - * @param sourceFile The source file where the node is. - * @param kind The kind of nodes to find. - * @param recursive Whether to go in matched nodes to keep matching. - * @param max The maximum number of items to return. - * @return all nodes of kind, or [] if none is found - */ -export function findAstNodes( - node: ts.Node | null, - sourceFile: ts.SourceFile, - kind: ts.SyntaxKind, - recursive = false, - max = Infinity -): T[] { - // TODO: refactor operations that only need `refactor.findAstNodes()` to use this instead. - if (max == 0) { - return []; - } - if (!node) { - node = sourceFile; - } - - let arr: T[] = []; - if (node.kind === kind) { - // If we're not recursively looking for children, stop here. - if (!recursive) { - return [node as T]; +// Find all nodes from the AST in the subtree of node of SyntaxKind kind. +export function collectDeepNodes(node: ts.Node, kind: ts.SyntaxKind): T[] { + const nodes: T[] = []; + const helper = (child: ts.Node) => { + if (child.kind === kind) { + nodes.push(child as T); } + ts.forEachChild(child, helper); + }; + ts.forEachChild(node, helper); - arr.push(node as T); - max--; - } - - if (max > 0) { - for (const child of node.getChildren(sourceFile)) { - findAstNodes(child, sourceFile, kind, recursive, max) - .forEach((node: ts.Node) => { - if (max > 0) { - arr.push(node as T); - } - max--; - }); - - if (max <= 0) { - break; - } - } - } - return arr; + return nodes; } export function getFirstNode(sourceFile: ts.SourceFile): ts.Node | null { - const syntaxList = findAstNodes(null, sourceFile, ts.SyntaxKind.SyntaxList, false, 1)[0] || null; - if (syntaxList) { - return (syntaxList && syntaxList.getChildCount() > 0) ? syntaxList.getChildAt(0) : null; + if (sourceFile.statements.length > 0) { + return sourceFile.statements[0] || null; } return null; } export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null { - const syntaxList = findAstNodes(null, sourceFile, ts.SyntaxKind.SyntaxList, false, 1)[0] || null; - if (syntaxList) { - const childCount = syntaxList.getChildCount(); - return childCount > 0 ? syntaxList.getChildAt(childCount - 1) : null; + if (sourceFile.statements.length > 0) { + return sourceFile.statements[sourceFile.statements.length - 1] || null; } return null; } @@ -77,7 +34,7 @@ export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null { export function transformTypescript( content: string, - transformOpsCb: (SourceFile: ts.SourceFile) => TransformOperation[] + transformers: ts.TransformerFactory[] ) { // Set compiler options. @@ -102,13 +59,9 @@ export function transformTypescript( // Create the TypeScript program. const program = ts.createProgram([fileName], compilerOptions, compilerHost); - // Get the transform operations. - const sourceFile = program.getSourceFile(fileName); - const transformOps = transformOpsCb(sourceFile); - // Emit. const { emitSkipped, diagnostics } = program.emit( - undefined, undefined, undefined, undefined, { before: [makeTransform(transformOps)] } + undefined, undefined, undefined, undefined, { before: transformers } ); // Log diagnostics if emit wasn't successfull. diff --git a/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.spec.ts b/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.spec.ts index 4a040d13c9d9..4663e302fbd9 100644 --- a/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.spec.ts +++ b/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.spec.ts @@ -1,12 +1,10 @@ -// @ignoreDep typescript -import * as ts from 'typescript'; import { oneLine, stripIndent } from 'common-tags'; import { transformTypescript } from './ast_helpers'; import { exportLazyModuleMap } from './export_lazy_module_map'; -describe('export_lazy_module_map', () => { - it('should create module map for JIT', () => { - it('should replace resources', () => { +describe('@ngtools/webpack transformers', () => { + describe('export_lazy_module_map', () => { + it('should create module map for JIT', () => { const input = stripIndent` export { AppModule } from './app/app.module'; `; @@ -19,37 +17,60 @@ describe('export_lazy_module_map', () => { `; // tslint:enable:max-line-length - const transformOpsCb = (sourceFile: ts.SourceFile) => exportLazyModuleMap(sourceFile, { - './lazy/lazy.module#LazyModule': '/project/src/app/lazy/lazy.module.ts', - './lazy2/lazy2.module#LazyModule2': '/project/src/app/lazy2/lazy2.module.ts', - }); - const result = transformTypescript(input, transformOpsCb); + const transformer = exportLazyModuleMap( + () => true, + () => ({ + './lazy/lazy.module#LazyModule': '/project/src/app/lazy/lazy.module.ts', + './lazy2/lazy2.module#LazyModule2': '/project/src/app/lazy2/lazy2.module.ts', + }), + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); - }); - it('should create module map for AOT', () => { - const input = stripIndent` + it('should create module map for AOT', () => { + const input = stripIndent` export { AppModule } from './app/app.module'; `; - // tslint:disable:max-line-length - const expected = stripIndent` + // tslint:disable:max-line-length + const expected = stripIndent` import * as __lazy_0__ from "app/lazy/lazy.module.ngfactory.ts"; import * as __lazy_1__ from "app/lazy2/lazy2.module.ngfactory.ts"; export { AppModule } from './app/app.module'; export var LAZY_MODULE_MAP = { "./lazy/lazy.module#LazyModule": __lazy_0__.LazyModuleNgFactory, "./lazy2/lazy2.module#LazyModule2": __lazy_1__.LazyModule2NgFactory }; `; - // tslint:enable:max-line-length + // tslint:enable:max-line-length + + const transformer = exportLazyModuleMap( + () => true, + () => ({ + './lazy/lazy.module.ngfactory#LazyModuleNgFactory': + '/project/src/app/lazy/lazy.module.ngfactory.ts', + './lazy2/lazy2.module.ngfactory#LazyModule2NgFactory': + '/project/src/app/lazy2/lazy2.module.ngfactory.ts', + }), + ); + const result = transformTypescript(input, [transformer]); - const transformOpsCb = (sourceFile: ts.SourceFile) => exportLazyModuleMap(sourceFile, { - './lazy/lazy.module.ngfactory#LazyModuleNgFactory': - '/project/src/app/lazy/lazy.module.ngfactory.ts', - './lazy2/lazy2.module.ngfactory#LazyModule2NgFactory': - '/project/src/app/lazy2/lazy2.module.ngfactory.ts', + expect(oneLine`${result}`).toEqual(oneLine`${expected}`); }); - const result = transformTypescript(input, transformOpsCb); + }); + + it('should not do anything if shouldTransform returns false', () => { + const input = stripIndent` + export { AppModule } from './app/app.module'; + `; + + const transformer = exportLazyModuleMap( + () => false, + () => ({ + './lazy/lazy.module#LazyModule': '/project/src/app/lazy/lazy.module.ts', + './lazy2/lazy2.module#LazyModule2': '/project/src/app/lazy2/lazy2.module.ts', + }), + ); + const result = transformTypescript(input, [transformer]); - expect(oneLine`${result}`).toEqual(oneLine`${expected}`); + expect(oneLine`${result}`).toEqual(oneLine`${input}`); }); }); diff --git a/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.ts b/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.ts index 7d1d212ff3d0..d6ddf92ad7f7 100644 --- a/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.ts +++ b/packages/@ngtools/webpack/src/transformers/export_lazy_module_map.ts @@ -4,70 +4,80 @@ import * as ts from 'typescript'; import { LazyRouteMap } from '../lazy_routes'; import { getLastNode, getFirstNode } from './ast_helpers'; -import { - AddNodeOperation, - TransformOperation -} from './make_transform'; +import { StandardTransform, TransformOperation, AddNodeOperation } from './interfaces'; +import { makeTransform } from './make_transform'; export function exportLazyModuleMap( - sourceFile: ts.SourceFile, - lazyRoutes: LazyRouteMap -): TransformOperation[] { - const ops: TransformOperation[] = []; - const dirName = path.normalize(path.dirname(sourceFile.fileName)); - - const modules = Object.keys(lazyRoutes) - .map((loadChildrenString) => { - const [, moduleName] = loadChildrenString.split('#'); - const modulePath = lazyRoutes[loadChildrenString]; - - return { - modulePath, - moduleName, - loadChildrenString - }; + shouldTransform: (fileName: string) => boolean, + lazyRoutesCb: () => LazyRouteMap, +): ts.TransformerFactory { + + const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) { + const ops: TransformOperation[] = []; + + const lazyRoutes = lazyRoutesCb(); + + if (!shouldTransform(sourceFile.fileName)) { + return ops; + } + + const dirName = path.normalize(path.dirname(sourceFile.fileName)); + + const modules = Object.keys(lazyRoutes) + .map((loadChildrenString) => { + const [, moduleName] = loadChildrenString.split('#'); + const modulePath = lazyRoutes[loadChildrenString]; + + return { + modulePath, + moduleName, + loadChildrenString + }; + }); + + modules.forEach((module, index) => { + const relativePath = path.relative(dirName, module.modulePath!).replace(/\\/g, '/'); + // Create the new namespace import node. + const namespaceImport = ts.createNamespaceImport(ts.createIdentifier(`__lazy_${index}__`)); + const importClause = ts.createImportClause(undefined, namespaceImport); + const newImport = ts.createImportDeclaration(undefined, undefined, importClause, + ts.createLiteral(relativePath)); + + ops.push(new AddNodeOperation( + sourceFile, + getFirstNode(sourceFile), + newImport + )); }); - modules.forEach((module, index) => { - const relativePath = path.relative(dirName, module.modulePath!).replace(/\\/g, '/'); - // Create the new namespace import node. - const namespaceImport = ts.createNamespaceImport(ts.createIdentifier(`__lazy_${index}__`)); - const importClause = ts.createImportClause(undefined, namespaceImport); - const newImport = ts.createImportDeclaration(undefined, undefined, importClause, - ts.createLiteral(relativePath)); + const lazyModuleObjectLiteral = ts.createObjectLiteral( + modules.map((mod, idx) => { + let [modulePath, moduleName] = mod.loadChildrenString.split('#'); + if (modulePath.match(/\.ngfactory/)) { + modulePath = modulePath.replace('.ngfactory', ''); + moduleName = moduleName.replace('NgFactory', ''); + } + + return ts.createPropertyAssignment( + ts.createLiteral(`${modulePath}#${moduleName}`), + ts.createPropertyAccess(ts.createIdentifier(`__lazy_${idx}__`), mod.moduleName)); + }) + ); + + const lazyModuleVariableStmt = ts.createVariableStatement( + [ts.createToken(ts.SyntaxKind.ExportKeyword)], + [ts.createVariableDeclaration('LAZY_MODULE_MAP', undefined, lazyModuleObjectLiteral)] + ); ops.push(new AddNodeOperation( sourceFile, - getFirstNode(sourceFile), - newImport + getLastNode(sourceFile), + undefined, + lazyModuleVariableStmt )); - }); - - const lazyModuleObjectLiteral = ts.createObjectLiteral( - modules.map((mod, idx) => { - let [modulePath, moduleName] = mod.loadChildrenString.split('#'); - if (modulePath.match(/\.ngfactory/)) { - modulePath = modulePath.replace('.ngfactory', ''); - moduleName = moduleName.replace('NgFactory', ''); - } - - return ts.createPropertyAssignment( - ts.createLiteral(`${modulePath}#${moduleName}`), - ts.createPropertyAccess(ts.createIdentifier(`__lazy_${idx}__`), mod.moduleName)); - }) - ); - - const lazyModuleVariableStmt = ts.createVariableStatement( - [ts.createToken(ts.SyntaxKind.ExportKeyword)], - [ts.createVariableDeclaration('LAZY_MODULE_MAP', undefined, lazyModuleObjectLiteral)] - ); - - ops.push(new AddNodeOperation( - sourceFile, - getLastNode(sourceFile), - undefined, - lazyModuleVariableStmt - )); - - return ops; + + return ops; + }; + + return makeTransform(standardTransform); } diff --git a/packages/@ngtools/webpack/src/transformers/export_ngfactory.spec.ts b/packages/@ngtools/webpack/src/transformers/export_ngfactory.spec.ts index 6ee8aa9483f5..324e41bbb4f8 100644 --- a/packages/@ngtools/webpack/src/transformers/export_ngfactory.spec.ts +++ b/packages/@ngtools/webpack/src/transformers/export_ngfactory.spec.ts @@ -1,5 +1,3 @@ -// @ignoreDep typescript -import * as ts from 'typescript'; import { oneLine, stripIndent } from 'common-tags'; import { transformTypescript } from './ast_helpers'; import { exportNgFactory } from './export_ngfactory'; @@ -15,9 +13,11 @@ describe('@ngtools/webpack transformers', () => { export { AppModule } from './app/app.module'; `; - const transformOpsCb = (sourceFile: ts.SourceFile) => exportNgFactory(sourceFile, - { path: '/project/src/app/app.module', className: 'AppModule' }); - const result = transformTypescript(input, transformOpsCb); + const transformer = exportNgFactory( + () => true, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }), + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); @@ -31,11 +31,27 @@ describe('@ngtools/webpack transformers', () => { export { AppModule } from './app'; `; - const transformOpsCb = (sourceFile: ts.SourceFile) => exportNgFactory(sourceFile, - { path: '/project/src/app/app.module', className: 'AppModule' }); - const result = transformTypescript(input, transformOpsCb); + const transformer = exportNgFactory( + () => true, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }), + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); + + it('should not do anything if shouldTransform returns false', () => { + const input = stripIndent` + export { AppModule } from './app/app.module'; + `; + + const transformer = exportNgFactory( + () => false, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }), + ); + const result = transformTypescript(input, [transformer]); + + expect(oneLine`${result}`).toEqual(oneLine`${input}`); + }); }); }); diff --git a/packages/@ngtools/webpack/src/transformers/export_ngfactory.ts b/packages/@ngtools/webpack/src/transformers/export_ngfactory.ts index 828f36348cb9..1b31be26cb95 100644 --- a/packages/@ngtools/webpack/src/transformers/export_ngfactory.ts +++ b/packages/@ngtools/webpack/src/transformers/export_ngfactory.ts @@ -2,58 +2,70 @@ import * as ts from 'typescript'; import { relative, dirname } from 'path'; -import { findAstNodes, getFirstNode } from './ast_helpers'; -import { TransformOperation, AddNodeOperation } from './make_transform'; +import { collectDeepNodes, getFirstNode } from './ast_helpers'; +import { StandardTransform, TransformOperation, AddNodeOperation } from './interfaces'; +import { makeTransform } from './make_transform'; export function exportNgFactory( - sourceFile: ts.SourceFile, - entryModule: { path: string, className: string } -): TransformOperation[] { - const ops: TransformOperation[] = []; - - // Find all identifiers using the entry module class name. - const entryModuleIdentifiers = findAstNodes(null, sourceFile, - ts.SyntaxKind.Identifier, true) - .filter(identifier => identifier.getText() === entryModule.className); - - if (entryModuleIdentifiers.length === 0) { - return []; - } - - const relativeEntryModulePath = relative(dirname(sourceFile.fileName), entryModule.path); - const normalizedEntryModulePath = `./${relativeEntryModulePath}`.replace(/\\/g, '/'); - - // Get the module path from the import. - let modulePath: string; - entryModuleIdentifiers.forEach((entryModuleIdentifier) => { - if (entryModuleIdentifier.parent.kind !== ts.SyntaxKind.ExportSpecifier) { - return; + shouldTransform: (fileName: string) => boolean, + getEntryModule: () => { path: string, className: string }, +): ts.TransformerFactory { + + const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) { + const ops: TransformOperation[] = []; + + const entryModule = getEntryModule(); + + if (!shouldTransform(sourceFile.fileName) || !entryModule) { + return ops; } - const exportSpec = entryModuleIdentifier.parent as ts.ExportSpecifier; - const moduleSpecifier = exportSpec.parent.parent.moduleSpecifier; + // Find all identifiers using the entry module class name. + const entryModuleIdentifiers = collectDeepNodes(sourceFile, + ts.SyntaxKind.Identifier) + .filter(identifier => identifier.text === entryModule.className); - if (moduleSpecifier.kind !== ts.SyntaxKind.StringLiteral) { - return; + if (entryModuleIdentifiers.length === 0) { + return []; } - modulePath = (moduleSpecifier as ts.StringLiteral).text; + const relativeEntryModulePath = relative(dirname(sourceFile.fileName), entryModule.path); + const normalizedEntryModulePath = `./${relativeEntryModulePath}`.replace(/\\/g, '/'); + + // Get the module path from the import. + let modulePath: string; + entryModuleIdentifiers.forEach((entryModuleIdentifier) => { + if (entryModuleIdentifier.parent.kind !== ts.SyntaxKind.ExportSpecifier) { + return; + } + + const exportSpec = entryModuleIdentifier.parent as ts.ExportSpecifier; + const moduleSpecifier = exportSpec.parent.parent.moduleSpecifier; + + if (moduleSpecifier.kind !== ts.SyntaxKind.StringLiteral) { + return; + } + + modulePath = (moduleSpecifier as ts.StringLiteral).text; + + // Add the transform operations. + const factoryClassName = entryModule.className + 'NgFactory'; + const factoryModulePath = normalizedEntryModulePath + '.ngfactory'; - // Add the transform operations. - const factoryClassName = entryModule.className + 'NgFactory'; - const factoryModulePath = normalizedEntryModulePath + '.ngfactory'; + const namedExports = ts.createNamedExports([ts.createExportSpecifier(undefined, + ts.createIdentifier(factoryClassName))]); + const newImport = ts.createExportDeclaration(undefined, undefined, namedExports, + ts.createLiteral(factoryModulePath)); - const namedExports = ts.createNamedExports([ts.createExportSpecifier(undefined, - ts.createIdentifier(factoryClassName))]); - const newImport = ts.createExportDeclaration(undefined, undefined, namedExports, - ts.createLiteral(factoryModulePath)); + ops.push(new AddNodeOperation( + sourceFile, + getFirstNode(sourceFile), + newImport + )); + }); - ops.push(new AddNodeOperation( - sourceFile, - getFirstNode(sourceFile), - newImport - )); - }); + return ops; + }; - return ops; + return makeTransform(standardTransform); } diff --git a/packages/@ngtools/webpack/src/transformers/index.ts b/packages/@ngtools/webpack/src/transformers/index.ts index de09607bb67e..029a645c539b 100644 --- a/packages/@ngtools/webpack/src/transformers/index.ts +++ b/packages/@ngtools/webpack/src/transformers/index.ts @@ -1,3 +1,4 @@ +export * from './interfaces'; export * from './ast_helpers'; export * from './make_transform'; export * from './insert_import'; diff --git a/packages/@ngtools/webpack/src/transformers/insert_import.ts b/packages/@ngtools/webpack/src/transformers/insert_import.ts index 2cdf1b58320e..9b63d521f831 100644 --- a/packages/@ngtools/webpack/src/transformers/insert_import.ts +++ b/packages/@ngtools/webpack/src/transformers/insert_import.ts @@ -1,8 +1,8 @@ // @ignoreDep typescript import * as ts from 'typescript'; -import { findAstNodes, getFirstNode } from './ast_helpers'; -import { AddNodeOperation, TransformOperation } from './make_transform'; +import { collectDeepNodes, getFirstNode } from './ast_helpers'; +import { AddNodeOperation, TransformOperation } from './interfaces'; export function insertStarImport( @@ -13,7 +13,7 @@ export function insertStarImport( before = false, ): TransformOperation[] { const ops: TransformOperation[] = []; - const allImports = findAstNodes(null, sourceFile, ts.SyntaxKind.ImportDeclaration); + const allImports = collectDeepNodes(sourceFile, ts.SyntaxKind.ImportDeclaration); // We don't need to verify if the symbol is already imported, star imports should be unique. @@ -58,7 +58,7 @@ export function insertImport( ): TransformOperation[] { const ops: TransformOperation[] = []; // Find all imports. - const allImports = findAstNodes(null, sourceFile, ts.SyntaxKind.ImportDeclaration); + const allImports = collectDeepNodes(sourceFile, ts.SyntaxKind.ImportDeclaration); const maybeImports = allImports .filter((node: ts.ImportDeclaration) => { // Filter all imports that do not match the modulePath. diff --git a/packages/@ngtools/webpack/src/transformers/interfaces.ts b/packages/@ngtools/webpack/src/transformers/interfaces.ts new file mode 100644 index 000000000000..5bf6440cddfc --- /dev/null +++ b/packages/@ngtools/webpack/src/transformers/interfaces.ts @@ -0,0 +1,39 @@ +import * as ts from 'typescript'; + +export enum OPERATION_KIND { + Remove, + Add, + Replace +} + +export interface StandardTransform { + (sourceFile: ts.SourceFile): TransformOperation[]; +} + +export abstract class TransformOperation { + constructor( + public kind: OPERATION_KIND, + public sourceFile: ts.SourceFile, + public target: ts.Node + ) { } +} + +export class RemoveNodeOperation extends TransformOperation { + constructor(sourceFile: ts.SourceFile, target: ts.Node) { + super(OPERATION_KIND.Remove, sourceFile, target); + } +} + +export class AddNodeOperation extends TransformOperation { + constructor(sourceFile: ts.SourceFile, target: ts.Node, + public before?: ts.Node, public after?: ts.Node) { + super(OPERATION_KIND.Add, sourceFile, target); + } +} + +export class ReplaceNodeOperation extends TransformOperation { + kind: OPERATION_KIND.Replace; + constructor(sourceFile: ts.SourceFile, target: ts.Node, public replacement: ts.Node) { + super(OPERATION_KIND.Replace, sourceFile, target); + } +} diff --git a/packages/@ngtools/webpack/src/transformers/make_transform.ts b/packages/@ngtools/webpack/src/transformers/make_transform.ts index 8886d800315d..02ad7642d376 100644 --- a/packages/@ngtools/webpack/src/transformers/make_transform.ts +++ b/packages/@ngtools/webpack/src/transformers/make_transform.ts @@ -1,59 +1,35 @@ import * as ts from 'typescript'; import { satisfies } from 'semver'; +import { + OPERATION_KIND, + StandardTransform, + TransformOperation, + RemoveNodeOperation, + AddNodeOperation, + ReplaceNodeOperation, +} from './interfaces'; + // Typescript below 2.5.0 needs a workaround. const visitEachChild = satisfies(ts.version, '^2.5.0') ? ts.visitEachChild : visitEachChildWorkaround; -export enum OPERATION_KIND { - Remove, - Add, - Replace -} - -export abstract class TransformOperation { - constructor( - public kind: OPERATION_KIND, - public sourceFile: ts.SourceFile, - public target: ts.Node - ) { } -} - -export class RemoveNodeOperation extends TransformOperation { - constructor(sourceFile: ts.SourceFile, target: ts.Node) { - super(OPERATION_KIND.Remove, sourceFile, target); - } -} - -export class AddNodeOperation extends TransformOperation { - constructor(sourceFile: ts.SourceFile, target: ts.Node, - public before?: ts.Node, public after?: ts.Node) { - super(OPERATION_KIND.Add, sourceFile, target); - } -} - -export class ReplaceNodeOperation extends TransformOperation { - kind: OPERATION_KIND.Replace; - constructor(sourceFile: ts.SourceFile, target: ts.Node, public replacement: ts.Node) { - super(OPERATION_KIND.Replace, sourceFile, target); - } -} - -export function makeTransform(ops: TransformOperation[]): ts.TransformerFactory { - - const sourceFiles = ops.reduce((prev, curr) => - prev.includes(curr.sourceFile) ? prev : prev.concat(curr.sourceFile), []); - - const removeOps = ops.filter((op) => op.kind === OPERATION_KIND.Remove) as RemoveNodeOperation[]; - const addOps = ops.filter((op) => op.kind === OPERATION_KIND.Add) as AddNodeOperation[]; - const replaceOps = ops - .filter((op) => op.kind === OPERATION_KIND.Replace) as ReplaceNodeOperation[]; +export function makeTransform( + standardTransform: StandardTransform +): ts.TransformerFactory { return (context: ts.TransformationContext): ts.Transformer => { const transformer: ts.Transformer = (sf: ts.SourceFile) => { + const ops: TransformOperation[] = standardTransform(sf); + const removeOps = ops + .filter((op) => op.kind === OPERATION_KIND.Remove) as RemoveNodeOperation[]; + const addOps = ops.filter((op) => op.kind === OPERATION_KIND.Add) as AddNodeOperation[]; + const replaceOps = ops + .filter((op) => op.kind === OPERATION_KIND.Replace) as ReplaceNodeOperation[]; + const visitor: ts.Visitor = (node) => { let modified = false; let modifiedNodes = [node]; @@ -91,7 +67,7 @@ export function makeTransform(ops: TransformOperation[]): ts.TransformerFactory< }; // Only visit source files we have ops for. - return sourceFiles.includes(sf) ? ts.visitNode(sf, visitor) : sf; + return ops.length > 0 ? ts.visitNode(sf, visitor) : sf; }; return transformer; diff --git a/packages/@ngtools/webpack/src/transformers/multiple_transformers.spec.ts b/packages/@ngtools/webpack/src/transformers/multiple_transformers.spec.ts new file mode 100644 index 000000000000..c3c610d9bf09 --- /dev/null +++ b/packages/@ngtools/webpack/src/transformers/multiple_transformers.spec.ts @@ -0,0 +1,64 @@ +import { oneLine, stripIndent } from 'common-tags'; +import { transformTypescript } from './ast_helpers'; +import { replaceBootstrap } from './replace_bootstrap'; +import { exportNgFactory } from './export_ngfactory'; +import { exportLazyModuleMap } from './export_lazy_module_map'; + +describe('@ngtools/webpack transformers', () => { + describe('multiple_transformers', () => { + it('should apply multiple transformers on the same file', () => { + const input = stripIndent` + import { enableProdMode } from '@angular/core'; + import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; + + import { AppModule } from './app/app.module'; + import { environment } from './environments/environment'; + + if (environment.production) { + enableProdMode(); + } + + platformBrowserDynamic().bootstrapModule(AppModule); + `; + + // tslint:disable:max-line-length + const output = stripIndent` + import * as __lazy_0__ from "app/lazy/lazy.module.ngfactory.ts"; + import * as __lazy_1__ from "app/lazy2/lazy2.module.ngfactory.ts"; + + import { enableProdMode } from '@angular/core'; + import { environment } from './environments/environment'; + + import * as __NgCli_bootstrap_1 from "./app/app.module.ngfactory"; + import * as __NgCli_bootstrap_2 from "@angular/platform-browser"; + if (environment.production) { + enableProdMode(); + } + __NgCli_bootstrap_2.platformBrowser().bootstrapModuleFactory(__NgCli_bootstrap_1.AppModuleNgFactory); + + export var LAZY_MODULE_MAP = { "./lazy/lazy.module#LazyModule": __lazy_0__.LazyModuleNgFactory, "./lazy2/lazy2.module#LazyModule2": __lazy_1__.LazyModule2NgFactory }; + `; + // tslint:enable:max-line-length + + const shouldTransform = () => true; + const getEntryModule = () => + ({ path: '/project/src/app/app.module', className: 'AppModule' }); + + const transformers = [ + replaceBootstrap(shouldTransform, getEntryModule), + exportNgFactory(shouldTransform, getEntryModule), + exportLazyModuleMap(shouldTransform, + () => ({ + './lazy/lazy.module.ngfactory#LazyModuleNgFactory': + '/project/src/app/lazy/lazy.module.ngfactory.ts', + './lazy2/lazy2.module.ngfactory#LazyModule2NgFactory': + '/project/src/app/lazy2/lazy2.module.ngfactory.ts', + })), + ]; + + const result = transformTypescript(input, transformers); + + expect(oneLine`${result}`).toEqual(oneLine`${output}`); + }); + }); +}); diff --git a/packages/@ngtools/webpack/src/transformers/register_locale_data.spec.ts b/packages/@ngtools/webpack/src/transformers/register_locale_data.spec.ts index 630e18f75df4..c086e2427133 100644 --- a/packages/@ngtools/webpack/src/transformers/register_locale_data.spec.ts +++ b/packages/@ngtools/webpack/src/transformers/register_locale_data.spec.ts @@ -1,5 +1,3 @@ -// @ignoreDep typescript -import * as ts from 'typescript'; import { oneLine, stripIndent } from 'common-tags'; import { transformTypescript } from './ast_helpers'; import { registerLocaleData } from './register_locale_data'; @@ -38,11 +36,35 @@ describe('@ngtools/webpack transformers', () => { platformBrowserDynamic().bootstrapModule(AppModule); `; - const transformOpsCb = (sourceFile: ts.SourceFile) => - registerLocaleData(sourceFile, { path: '/app.module', className: 'AppModule' }, 'fr'); - const result = transformTypescript(input, transformOpsCb); + const transformer = registerLocaleData( + () => true, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }), + 'fr' + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); + + it('should not add locale imports when there is no entry module', () => { + const input = stripIndent` + import { enableProdMode } from '@angular/core'; + import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; + + import { AppModule } from './app/app.module'; + import { environment } from './environments/environment'; + + if (environment.production) { + enableProdMode(); + } + + platformBrowserDynamic().bootstrapModule(AppModule); + `; + + const transformer = registerLocaleData(() => true, () => undefined, 'fr'); + const result = transformTypescript(input, [transformer]); + + expect(oneLine`${result}`).toEqual(oneLine`${input}`); + }); }); }); diff --git a/packages/@ngtools/webpack/src/transformers/register_locale_data.ts b/packages/@ngtools/webpack/src/transformers/register_locale_data.ts index b4f4abb4a719..efc1aa655d10 100644 --- a/packages/@ngtools/webpack/src/transformers/register_locale_data.ts +++ b/packages/@ngtools/webpack/src/transformers/register_locale_data.ts @@ -1,79 +1,91 @@ // @ignoreDep typescript import * as ts from 'typescript'; -import { findAstNodes, getFirstNode } from './ast_helpers'; -import { AddNodeOperation, TransformOperation } from './make_transform'; +import { collectDeepNodes, getFirstNode } from './ast_helpers'; +import { StandardTransform, AddNodeOperation, TransformOperation } from './interfaces'; import { insertStarImport } from './insert_import'; +import { makeTransform } from './make_transform'; export function registerLocaleData( - sourceFile: ts.SourceFile, - entryModule: { path: string, className: string }, + shouldTransform: (fileName: string) => boolean, + getEntryModule: () => { path: string, className: string }, locale: string -): TransformOperation[] { - const ops: TransformOperation[] = []; - - // Find all identifiers using the entry module class name. - const entryModuleIdentifiers = findAstNodes(null, sourceFile, - ts.SyntaxKind.Identifier, true) - .filter(identifier => identifier.getText() === entryModule.className); - - if (entryModuleIdentifiers.length === 0) { - return []; - } - - // Find the bootstrap call - entryModuleIdentifiers.forEach(entryModuleIdentifier => { - // Figure out if it's a `platformBrowserDynamic().bootstrapModule(AppModule)` call. - if (!( - entryModuleIdentifier.parent - && entryModuleIdentifier.parent.kind === ts.SyntaxKind.CallExpression - )) { - return; - } +): ts.TransformerFactory { - const callExpr = entryModuleIdentifier.parent as ts.CallExpression; + const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) { + const ops: TransformOperation[] = []; - if (callExpr.expression.kind !== ts.SyntaxKind.PropertyAccessExpression) { - return; + const entryModule = getEntryModule(); + + if (!shouldTransform(sourceFile.fileName) || !entryModule || !locale) { + return ops; } - const propAccessExpr = callExpr.expression as ts.PropertyAccessExpression; + // Find all identifiers using the entry module class name. + const entryModuleIdentifiers = collectDeepNodes(sourceFile, + ts.SyntaxKind.Identifier) + .filter(identifier => identifier.text === entryModule.className); - if (propAccessExpr.name.text !== 'bootstrapModule' - || propAccessExpr.expression.kind !== ts.SyntaxKind.CallExpression) { - return; + if (entryModuleIdentifiers.length === 0) { + return []; } - const firstNode = getFirstNode(sourceFile); - - // Create the import node for the locale. - const localeNamespaceId = ts.createUniqueName('__NgCli_locale_'); - ops.push(...insertStarImport( - sourceFile, localeNamespaceId, `@angular/common/locales/${locale}`, firstNode, true - )); - - // Create the import node for the registerLocaleData function. - const regIdentifier = ts.createIdentifier(`registerLocaleData`); - const regNamespaceId = ts.createUniqueName('__NgCli_locale_'); - ops.push( - ...insertStarImport(sourceFile, regNamespaceId, '@angular/common', firstNode, true) - ); - - // Create the register function call - const registerFunctionCall = ts.createCall( - ts.createPropertyAccess(regNamespaceId, regIdentifier), - undefined, - [ts.createPropertyAccess(localeNamespaceId, 'default')], - ); - const registerFunctionStatement = ts.createStatement(registerFunctionCall); - - ops.push(new AddNodeOperation( - sourceFile, - firstNode, - registerFunctionStatement, - )); - }); - - return ops; + // Find the bootstrap call + entryModuleIdentifiers.forEach(entryModuleIdentifier => { + // Figure out if it's a `platformBrowserDynamic().bootstrapModule(AppModule)` call. + if (!( + entryModuleIdentifier.parent + && entryModuleIdentifier.parent.kind === ts.SyntaxKind.CallExpression + )) { + return; + } + + const callExpr = entryModuleIdentifier.parent as ts.CallExpression; + + if (callExpr.expression.kind !== ts.SyntaxKind.PropertyAccessExpression) { + return; + } + + const propAccessExpr = callExpr.expression as ts.PropertyAccessExpression; + + if (propAccessExpr.name.text !== 'bootstrapModule' + || propAccessExpr.expression.kind !== ts.SyntaxKind.CallExpression) { + return; + } + + const firstNode = getFirstNode(sourceFile); + + // Create the import node for the locale. + const localeNamespaceId = ts.createUniqueName('__NgCli_locale_'); + ops.push(...insertStarImport( + sourceFile, localeNamespaceId, `@angular/common/locales/${locale}`, firstNode, true + )); + + // Create the import node for the registerLocaleData function. + const regIdentifier = ts.createIdentifier(`registerLocaleData`); + const regNamespaceId = ts.createUniqueName('__NgCli_locale_'); + ops.push( + ...insertStarImport(sourceFile, regNamespaceId, '@angular/common', firstNode, true) + ); + + // Create the register function call + const registerFunctionCall = ts.createCall( + ts.createPropertyAccess(regNamespaceId, regIdentifier), + undefined, + [ts.createPropertyAccess(localeNamespaceId, 'default')], + ); + const registerFunctionStatement = ts.createStatement(registerFunctionCall); + + ops.push(new AddNodeOperation( + sourceFile, + firstNode, + registerFunctionStatement, + )); + }); + + return ops; + }; + + return makeTransform(standardTransform); } diff --git a/packages/@ngtools/webpack/src/transformers/remove_import.ts b/packages/@ngtools/webpack/src/transformers/remove_import.ts index 29df47b9de3f..7eb57425aec1 100644 --- a/packages/@ngtools/webpack/src/transformers/remove_import.ts +++ b/packages/@ngtools/webpack/src/transformers/remove_import.ts @@ -1,8 +1,8 @@ // @ignoreDep typescript import * as ts from 'typescript'; -import { findAstNodes } from './ast_helpers'; -import { RemoveNodeOperation, TransformOperation } from './make_transform'; +import { collectDeepNodes } from './ast_helpers'; +import { RemoveNodeOperation, TransformOperation } from './interfaces'; // Remove an import if we have removed all identifiers for it. // Mainly workaround for https://github.com/Microsoft/TypeScript/issues/17552. @@ -19,7 +19,7 @@ export function removeImport( const identifierText = removedIdentifiers[0].text; // Find all imports that import `identifierText`. - const allImports = findAstNodes(null, sourceFile, ts.SyntaxKind.ImportDeclaration); + const allImports = collectDeepNodes(sourceFile, ts.SyntaxKind.ImportDeclaration); const identifierImports = allImports .filter((node: ts.ImportDeclaration) => { // TODO: try to support removing `import * as X from 'XYZ'`. @@ -40,8 +40,8 @@ export function removeImport( // Find all identifiers with `identifierText` in the source file. - const allNodes = findAstNodes(null, sourceFile, ts.SyntaxKind.Identifier, true) - .filter(identifier => identifier.getText() === identifierText); + const allNodes = collectDeepNodes(sourceFile, ts.SyntaxKind.Identifier) + .filter(identifier => identifier.text === identifierText); // If there are more identifiers than the ones we already removed plus the ones we're going to // remove from imports, don't do anything. diff --git a/packages/@ngtools/webpack/src/transformers/replace_bootstrap.spec.ts b/packages/@ngtools/webpack/src/transformers/replace_bootstrap.spec.ts index 2a1a828cd6ca..5efc25c7a614 100644 --- a/packages/@ngtools/webpack/src/transformers/replace_bootstrap.spec.ts +++ b/packages/@ngtools/webpack/src/transformers/replace_bootstrap.spec.ts @@ -1,5 +1,3 @@ -// @ignoreDep typescript -import * as ts from 'typescript'; import { oneLine, stripIndent } from 'common-tags'; import { transformTypescript } from './ast_helpers'; import { replaceBootstrap } from './replace_bootstrap'; @@ -36,9 +34,11 @@ describe('@ngtools/webpack transformers', () => { `; // tslint:enable:max-line-length - const transformOpsCb = (sourceFile: ts.SourceFile) => replaceBootstrap(sourceFile, - { path: '/project/src/app/app.module', className: 'AppModule' }); - const result = transformTypescript(input, transformOpsCb); + const transformer = replaceBootstrap( + () => true, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }) + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); @@ -73,11 +73,34 @@ describe('@ngtools/webpack transformers', () => { `; // tslint:enable:max-line-length - const transformOpsCb = (sourceFile: ts.SourceFile) => replaceBootstrap(sourceFile, - { path: '/project/src/app/app.module', className: 'AppModule' }); - const result = transformTypescript(input, transformOpsCb); + const transformer = replaceBootstrap( + () => true, + () => ({ path: '/project/src/app/app.module', className: 'AppModule' }) + ); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); + + it('should not replace bootstrap when there is no entry module', () => { + const input = stripIndent` + import { enableProdMode } from '@angular/core'; + import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; + + import { AppModule } from './app/app.module'; + import { environment } from './environments/environment'; + + if (environment.production) { + enableProdMode(); + } + + platformBrowserDynamic().bootstrapModule(AppModule); + `; + + const transformer = replaceBootstrap(() => true, () => undefined); + const result = transformTypescript(input, [transformer]); + + expect(oneLine`${result}`).toEqual(oneLine`${input}`); + }); }); }); diff --git a/packages/@ngtools/webpack/src/transformers/replace_bootstrap.ts b/packages/@ngtools/webpack/src/transformers/replace_bootstrap.ts index ec04db66ac46..134dbfb350b9 100644 --- a/packages/@ngtools/webpack/src/transformers/replace_bootstrap.ts +++ b/packages/@ngtools/webpack/src/transformers/replace_bootstrap.ts @@ -2,99 +2,110 @@ import * as ts from 'typescript'; import { relative, dirname } from 'path'; -import { findAstNodes } from './ast_helpers'; +import { collectDeepNodes } from './ast_helpers'; import { insertStarImport } from './insert_import'; import { removeImport } from './remove_import'; -import { - ReplaceNodeOperation, - TransformOperation -} from './make_transform'; +import { StandardTransform, ReplaceNodeOperation, TransformOperation } from './interfaces'; +import { makeTransform } from './make_transform'; export function replaceBootstrap( - sourceFile: ts.SourceFile, - entryModule: { path: string, className: string } -): TransformOperation[] { - const ops: TransformOperation[] = []; - - // Find all identifiers. - const entryModuleIdentifiers = findAstNodes(null, sourceFile, - ts.SyntaxKind.Identifier, true) - .filter(identifier => identifier.getText() === entryModule.className); - - if (entryModuleIdentifiers.length === 0) { - return []; - } - - const relativeEntryModulePath = relative(dirname(sourceFile.fileName), entryModule.path); - const normalizedEntryModulePath = `./${relativeEntryModulePath}`.replace(/\\/g, '/'); - - // Find the bootstrap calls. - const removedEntryModuleIdentifiers: ts.Identifier[] = []; - const removedPlatformBrowserDynamicIdentifier: ts.Identifier[] = []; - entryModuleIdentifiers.forEach(entryModuleIdentifier => { - // Figure out if it's a `platformBrowserDynamic().bootstrapModule(AppModule)` call. - if (!( - entryModuleIdentifier.parent - && entryModuleIdentifier.parent.kind === ts.SyntaxKind.CallExpression - )) { - return; - } - - const callExpr = entryModuleIdentifier.parent as ts.CallExpression; + shouldTransform: (fileName: string) => boolean, + getEntryModule: () => { path: string, className: string } +): ts.TransformerFactory { - if (callExpr.expression.kind !== ts.SyntaxKind.PropertyAccessExpression) { - return; - } + const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) { + const ops: TransformOperation[] = []; - const propAccessExpr = callExpr.expression as ts.PropertyAccessExpression; + const entryModule = getEntryModule(); - if (propAccessExpr.name.text !== 'bootstrapModule' - || propAccessExpr.expression.kind !== ts.SyntaxKind.CallExpression) { - return; + if (!shouldTransform(sourceFile.fileName) || !entryModule) { + return ops; } - const bootstrapModuleIdentifier = propAccessExpr.name; - const innerCallExpr = propAccessExpr.expression as ts.CallExpression; + // Find all identifiers. + // const entryModuleIdentifiers = findAstNodes(null, sourceFile, + // ts.SyntaxKind.Identifier, true) + const entryModuleIdentifiers = collectDeepNodes(sourceFile, + ts.SyntaxKind.Identifier) + .filter(identifier => identifier.text === entryModule.className); - if (!( - innerCallExpr.expression.kind === ts.SyntaxKind.Identifier - && (innerCallExpr.expression as ts.Identifier).text === 'platformBrowserDynamic' - )) { - return; + if (entryModuleIdentifiers.length === 0) { + return []; } - const platformBrowserDynamicIdentifier = innerCallExpr.expression as ts.Identifier; - - const idPlatformBrowser = ts.createUniqueName('__NgCli_bootstrap_'); - const idNgFactory = ts.createUniqueName('__NgCli_bootstrap_'); - - // Add the transform operations. - const factoryClassName = entryModule.className + 'NgFactory'; - const factoryModulePath = normalizedEntryModulePath + '.ngfactory'; + const relativeEntryModulePath = relative(dirname(sourceFile.fileName), entryModule.path); + const normalizedEntryModulePath = `./${relativeEntryModulePath}`.replace(/\\/g, '/'); + + // Find the bootstrap calls. + const removedEntryModuleIdentifiers: ts.Identifier[] = []; + const removedPlatformBrowserDynamicIdentifier: ts.Identifier[] = []; + entryModuleIdentifiers.forEach(entryModuleIdentifier => { + // Figure out if it's a `platformBrowserDynamic().bootstrapModule(AppModule)` call. + if (!( + entryModuleIdentifier.parent + && entryModuleIdentifier.parent.kind === ts.SyntaxKind.CallExpression + )) { + return; + } + + const callExpr = entryModuleIdentifier.parent as ts.CallExpression; + + if (callExpr.expression.kind !== ts.SyntaxKind.PropertyAccessExpression) { + return; + } + + const propAccessExpr = callExpr.expression as ts.PropertyAccessExpression; + + if (propAccessExpr.name.text !== 'bootstrapModule' + || propAccessExpr.expression.kind !== ts.SyntaxKind.CallExpression) { + return; + } + + const bootstrapModuleIdentifier = propAccessExpr.name; + const innerCallExpr = propAccessExpr.expression as ts.CallExpression; + + if (!( + innerCallExpr.expression.kind === ts.SyntaxKind.Identifier + && (innerCallExpr.expression as ts.Identifier).text === 'platformBrowserDynamic' + )) { + return; + } + + const platformBrowserDynamicIdentifier = innerCallExpr.expression as ts.Identifier; + + const idPlatformBrowser = ts.createUniqueName('__NgCli_bootstrap_'); + const idNgFactory = ts.createUniqueName('__NgCli_bootstrap_'); + + // Add the transform operations. + const factoryClassName = entryModule.className + 'NgFactory'; + const factoryModulePath = normalizedEntryModulePath + '.ngfactory'; + ops.push( + // Replace the entry module import. + ...insertStarImport(sourceFile, idNgFactory, factoryModulePath), + new ReplaceNodeOperation(sourceFile, entryModuleIdentifier, + ts.createPropertyAccess(idNgFactory, ts.createIdentifier(factoryClassName))), + // Replace the platformBrowserDynamic import. + ...insertStarImport(sourceFile, idPlatformBrowser, '@angular/platform-browser'), + new ReplaceNodeOperation(sourceFile, platformBrowserDynamicIdentifier, + ts.createPropertyAccess(idPlatformBrowser, 'platformBrowser')), + new ReplaceNodeOperation(sourceFile, bootstrapModuleIdentifier, + ts.createIdentifier('bootstrapModuleFactory')), + ); + + // Save the import identifiers that we replaced for removal. + removedEntryModuleIdentifiers.push(entryModuleIdentifier); + removedPlatformBrowserDynamicIdentifier.push(platformBrowserDynamicIdentifier); + }); + + // Now that we know all the import identifiers we removed, we can remove the import. ops.push( - // Replace the entry module import. - ...insertStarImport(sourceFile, idNgFactory, factoryModulePath), - new ReplaceNodeOperation(sourceFile, entryModuleIdentifier, - ts.createPropertyAccess(idNgFactory, ts.createIdentifier(factoryClassName))), - // Replace the platformBrowserDynamic import. - ...insertStarImport(sourceFile, idPlatformBrowser, '@angular/platform-browser'), - new ReplaceNodeOperation(sourceFile, platformBrowserDynamicIdentifier, - ts.createPropertyAccess(idPlatformBrowser, 'platformBrowser')), - new ReplaceNodeOperation(sourceFile, bootstrapModuleIdentifier, - ts.createIdentifier('bootstrapModuleFactory')), + ...removeImport(sourceFile, removedEntryModuleIdentifiers), + ...removeImport(sourceFile, removedPlatformBrowserDynamicIdentifier), ); - // Save the import identifiers that we replaced for removal. - removedEntryModuleIdentifiers.push(entryModuleIdentifier); - removedPlatformBrowserDynamicIdentifier.push(platformBrowserDynamicIdentifier); - }); - - // Now that we know all the import identifiers we removed, we can remove the import. - ops.push( - ...removeImport(sourceFile, removedEntryModuleIdentifiers), - ...removeImport(sourceFile, removedPlatformBrowserDynamicIdentifier), - ); + return ops; + }; - return ops; + return makeTransform(standardTransform); } diff --git a/packages/@ngtools/webpack/src/transformers/replace_resources.spec.ts b/packages/@ngtools/webpack/src/transformers/replace_resources.spec.ts index f687f4abbd1b..0382d746c96f 100644 --- a/packages/@ngtools/webpack/src/transformers/replace_resources.spec.ts +++ b/packages/@ngtools/webpack/src/transformers/replace_resources.spec.ts @@ -1,5 +1,3 @@ -// @ignoreDep typescript -import * as ts from 'typescript'; import { oneLine, stripIndent } from 'common-tags'; import { transformTypescript } from './ast_helpers'; import { replaceResources } from './replace_resources'; @@ -37,8 +35,45 @@ describe('@ngtools/webpack transformers', () => { export { AppComponent }; `; - const transformOpsCb = (sourceFile: ts.SourceFile) => replaceResources(sourceFile); - const result = transformTypescript(input, transformOpsCb); + const transformer = replaceResources(() => true); + const result = transformTypescript(input, [transformer]); + + expect(oneLine`${result}`).toEqual(oneLine`${output}`); + }); + + it('should not replace resources if shouldTransform returns false', () => { + const input = stripIndent` + import { Component } from '@angular/core'; + + @Component({ + selector: 'app-root', + templateUrl: './app.component.html', + styleUrls: ['./app.component.css', './app.component.2.css'] + }) + export class AppComponent { + title = 'app'; + } + `; + const output = ` + import * as tslib_1 from "tslib"; + import { Component } from '@angular/core'; + let AppComponent = class AppComponent { + constructor() { + this.title = 'app'; + } + }; + AppComponent = tslib_1.__decorate([ + Component({ + selector: 'app-root', + templateUrl: './app.component.html', + styleUrls: ['./app.component.css', './app.component.2.css'] + }) + ], AppComponent); + export { AppComponent }; + `; + + const transformer = replaceResources(() => false); + const result = transformTypescript(input, [transformer]); expect(oneLine`${result}`).toEqual(oneLine`${output}`); }); diff --git a/packages/@ngtools/webpack/src/transformers/replace_resources.ts b/packages/@ngtools/webpack/src/transformers/replace_resources.ts index 460c749d904e..7d206e809b19 100644 --- a/packages/@ngtools/webpack/src/transformers/replace_resources.ts +++ b/packages/@ngtools/webpack/src/transformers/replace_resources.ts @@ -1,46 +1,59 @@ // @ignoreDep typescript import * as ts from 'typescript'; -import { findAstNodes, getFirstNode } from './ast_helpers'; +import { collectDeepNodes, getFirstNode } from './ast_helpers'; +import { makeTransform } from './make_transform'; import { + StandardTransform, AddNodeOperation, ReplaceNodeOperation, TransformOperation -} from './make_transform'; +} from './interfaces'; -export function replaceResources(sourceFile: ts.SourceFile): TransformOperation[] { - const ops: TransformOperation[] = []; - const replacements = findResources(sourceFile); +export function replaceResources( + shouldTransform: (fileName: string) => boolean +): ts.TransformerFactory { + const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) { + const ops: TransformOperation[] = []; - if (replacements.length > 0) { + if (!shouldTransform(sourceFile.fileName)) { + return ops; + } - // Add the replacement operations. - ops.push(...(replacements.map((rep) => rep.replaceNodeOperation))); + const replacements = findResources(sourceFile); - // If we added a require call, we need to also add typings for it. - // The typings need to be compatible with node typings, but also work by themselves. + if (replacements.length > 0) { - // interface NodeRequire {(id: string): any;} - const nodeRequireInterface = ts.createInterfaceDeclaration([], [], 'NodeRequire', [], [], [ - ts.createCallSignature([], [ - ts.createParameter([], [], undefined, 'id', undefined, - ts.createKeywordTypeNode(ts.SyntaxKind.StringKeyword) - ) - ], ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)) - ]); + // Add the replacement operations. + ops.push(...(replacements.map((rep) => rep.replaceNodeOperation))); - // declare var require: NodeRequire; - const varRequire = ts.createVariableStatement( - [ts.createToken(ts.SyntaxKind.DeclareKeyword)], - [ts.createVariableDeclaration('require', ts.createTypeReferenceNode('NodeRequire', []))] - ); + // If we added a require call, we need to also add typings for it. + // The typings need to be compatible with node typings, but also work by themselves. - ops.push(new AddNodeOperation(sourceFile, getFirstNode(sourceFile), nodeRequireInterface)); - ops.push(new AddNodeOperation(sourceFile, getFirstNode(sourceFile), varRequire)); - } + // interface NodeRequire {(id: string): any;} + const nodeRequireInterface = ts.createInterfaceDeclaration([], [], 'NodeRequire', [], [], [ + ts.createCallSignature([], [ + ts.createParameter([], [], undefined, 'id', undefined, + ts.createKeywordTypeNode(ts.SyntaxKind.StringKeyword) + ) + ], ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)) + ]); + + // declare var require: NodeRequire; + const varRequire = ts.createVariableStatement( + [ts.createToken(ts.SyntaxKind.DeclareKeyword)], + [ts.createVariableDeclaration('require', ts.createTypeReferenceNode('NodeRequire', []))] + ); + + ops.push(new AddNodeOperation(sourceFile, getFirstNode(sourceFile), nodeRequireInterface)); + ops.push(new AddNodeOperation(sourceFile, getFirstNode(sourceFile), varRequire)); + } - return ops; + return ops; + }; + + return makeTransform(standardTransform); } export interface ResourceReplacement { @@ -52,11 +65,9 @@ export function findResources(sourceFile: ts.SourceFile): ResourceReplacement[] const replacements: ResourceReplacement[] = []; // Find all object literals. - findAstNodes(null, sourceFile, - ts.SyntaxKind.ObjectLiteralExpression, true) + collectDeepNodes(sourceFile, ts.SyntaxKind.ObjectLiteralExpression) // Get all their property assignments. - .map(node => findAstNodes(node, sourceFile, - ts.SyntaxKind.PropertyAssignment)) + .map(node => collectDeepNodes(node, ts.SyntaxKind.PropertyAssignment)) // Flatten into a single array (from an array of array). .reduce((prev, curr) => curr ? prev.concat(curr) : prev, []) // We only want property assignments for the templateUrl/styleUrls keys. @@ -81,8 +92,8 @@ export function findResources(sourceFile: ts.SourceFile): ResourceReplacement[] replaceNodeOperation: new ReplaceNodeOperation(sourceFile, node, propAssign) }); } else if (key == 'styleUrls') { - const arr = findAstNodes(node, sourceFile, - ts.SyntaxKind.ArrayLiteralExpression, false); + const arr = collectDeepNodes(node, + ts.SyntaxKind.ArrayLiteralExpression); if (!arr || arr.length == 0 || arr[0].elements.length == 0) { return; } @@ -104,6 +115,7 @@ export function findResources(sourceFile: ts.SourceFile): ResourceReplacement[] }); return replacements; + } function _getContentOfKeyLiteral(node?: ts.Node): string | null { From 8a99f0b8f2a6f151eac88c5888c9ea703607092a Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 30 Oct 2017 18:22:30 +0000 Subject: [PATCH 3/4] fix(@angular/cli): don't search for lazy routes in Angular 5 ng test Fix #8066 --- .../cli/models/webpack-configs/typescript.ts | 33 ++++++----- .../webpack/src/angular_compiler_plugin.ts | 55 ++++++++----------- tests/e2e/setup/500-create-project.ts | 12 ++-- tests/e2e/tests/lint/lint-no-project.ts | 7 --- tests/e2e/tests/lint/lint-with-exclude.ts | 7 --- tests/e2e/utils/project.ts | 24 ++++---- 6 files changed, 63 insertions(+), 75 deletions(-) diff --git a/packages/@angular/cli/models/webpack-configs/typescript.ts b/packages/@angular/cli/models/webpack-configs/typescript.ts index 4c433c4259a0..d2787d6da328 100644 --- a/packages/@angular/cli/models/webpack-configs/typescript.ts +++ b/packages/@angular/cli/models/webpack-configs/typescript.ts @@ -18,7 +18,7 @@ const webpackLoader: string = g['angularCliIsLocal'] : '@ngtools/webpack'; -function _createAotPlugin(wco: WebpackConfigOptions, options: any) { +function _createAotPlugin(wco: WebpackConfigOptions, options: any, useMain = true) { const { appConfig, projectRoot, buildOptions } = wco; options.compilerOptions = options.compilerOptions || {}; @@ -82,7 +82,7 @@ function _createAotPlugin(wco: WebpackConfigOptions, options: any) { if (AngularCompilerPlugin.isSupported()) { const pluginOptions: AngularCompilerPluginOptions = Object.assign({}, { - mainPath: path.join(projectRoot, appConfig.root, appConfig.main), + mainPath: useMain ? path.join(projectRoot, appConfig.root, appConfig.main) : undefined, i18nInFile: buildOptions.i18nFile, i18nInFormat: buildOptions.i18nFormat, i18nOutFile: buildOptions.i18nOutFile, @@ -160,23 +160,28 @@ export function getNonAotTestConfig(wco: WebpackConfigOptions) { const tsConfigPath = path.resolve(projectRoot, appConfig.root, appConfig.testTsconfig); const appTsConfigPath = path.resolve(projectRoot, appConfig.root, appConfig.tsconfig); - // Force include main and polyfills. - // This is needed for AngularCompilerPlugin compatibility with existing projects, - // since TS compilation there is stricter and tsconfig.spec.ts doesn't include them. - const include = [appConfig.main, appConfig.polyfills, '**/*.spec.ts']; - if (appConfig.test) { - include.push(appConfig.test); - } + let pluginOptions: any = { tsConfigPath, skipCodeGeneration: true }; - let pluginOptions: any = { tsConfigPath, skipCodeGeneration: true, include }; + // The options below only apply to AoTPlugin. + if (!AngularCompilerPlugin.isSupported()) { + // Force include main and polyfills. + // This is needed for AngularCompilerPlugin compatibility with existing projects, + // since TS compilation there is stricter and tsconfig.spec.ts doesn't include them. + const include = [appConfig.main, appConfig.polyfills, '**/*.spec.ts']; + if (appConfig.test) { + include.push(appConfig.test); + } - // Fallback to correct module format on projects using a shared tsconfig. - if (tsConfigPath === appTsConfigPath) { - pluginOptions.compilerOptions = { module: 'commonjs' }; + pluginOptions.include = include; + + // Fallback to correct module format on projects using a shared tsconfig. + if (tsConfigPath === appTsConfigPath) { + pluginOptions.compilerOptions = { module: 'commonjs' }; + } } return { module: { rules: [{ test: /\.ts$/, loader: webpackLoader }] }, - plugins: [ _createAotPlugin(wco, pluginOptions) ] + plugins: [ _createAotPlugin(wco, pluginOptions, false) ] }; } diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index e2165ceb275a..c68916ea62e9 100644 --- a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts @@ -59,7 +59,6 @@ export interface AngularCompilerPluginOptions { entryModule?: string; mainPath?: string; skipCodeGeneration?: boolean; - hostOverrideFileSystem?: { [path: string]: string }; hostReplacementPaths?: { [path: string]: string }; i18nInFile?: string; i18nInFormat?: string; @@ -240,13 +239,6 @@ export class AngularCompilerPlugin implements Tapable { tsHost: webpackCompilerHost }) as CompilerHost & WebpackCompilerHost; - // Override some files in the FileSystem. - if (this._options.hostOverrideFileSystem) { - for (const filePath of Object.keys(this._options.hostOverrideFileSystem)) { - this._compilerHost.writeFile(filePath, - this._options.hostOverrideFileSystem[filePath], false); - } - } // Override some files in the FileSystem with paths from the actual file system. if (this._options.hostReplacementPaths) { for (const filePath of Object.keys(this._options.hostReplacementPaths)) { @@ -645,9 +637,8 @@ export class AngularCompilerPlugin implements Tapable { private _makeTransformers() { - // TODO use compilerhost.denormalize when #8210 is merged. const isAppPath = (fileName: string) => - this._rootNames.includes(fileName.replace(/\//g, path.sep)); + !fileName.endsWith('.ngfactory.ts') && !fileName.endsWith('.ngstyle.ts'); const isMainPath = (fileName: string) => fileName === this._mainPath; const getEntryModule = () => this.entryModule; const getLazyRoutes = () => this._lazyRoutes; @@ -693,17 +684,19 @@ export class AngularCompilerPlugin implements Tapable { // Make a new program and load the Angular structure. .then(() => this._createOrUpdateProgram()) .then(() => { - // Try to find lazy routes. - // We need to run the `listLazyRoutes` the first time because it also navigates libraries - // and other things that we might miss using the (faster) findLazyRoutesInAst. - // Lazy routes modules will be read with compilerHost and added to the changed files. - const changedTsFiles = this._getChangedTsFiles(); - if (this._ngCompilerSupportsNewApi) { - this._processLazyRoutes(this._listLazyRoutesFromProgram()); - } else if (this._firstRun) { - this._processLazyRoutes(this._getLazyRoutesFromNgtools()); - } else if (changedTsFiles.length > 0) { - this._processLazyRoutes(this._findLazyRoutesInAst(changedTsFiles)); + if (this.entryModule) { + // Try to find lazy routes if we have an entry module. + // We need to run the `listLazyRoutes` the first time because it also navigates libraries + // and other things that we might miss using the (faster) findLazyRoutesInAst. + // Lazy routes modules will be read with compilerHost and added to the changed files. + const changedTsFiles = this._getChangedTsFiles(); + if (this._ngCompilerSupportsNewApi) { + this._processLazyRoutes(this._listLazyRoutesFromProgram()); + } else if (this._firstRun) { + this._processLazyRoutes(this._getLazyRoutesFromNgtools()); + } else if (changedTsFiles.length > 0) { + this._processLazyRoutes(this._findLazyRoutesInAst(changedTsFiles)); + } } }) .then(() => { @@ -711,16 +704,16 @@ export class AngularCompilerPlugin implements Tapable { // We now have the final list of changed TS files. // Go through each changed file and add transforms as needed. - const sourceFiles = this._getChangedTsFiles().map((fileName) => { - time('AngularCompilerPlugin._update.getSourceFile'); - const sourceFile = this._getTsProgram().getSourceFile(fileName); - if (!sourceFile) { - throw new Error(`${fileName} is not part of the TypeScript compilation. ` - + `Please include it in your tsconfig via the 'files' or 'include' property.`); - } - timeEnd('AngularCompilerPlugin._update.getSourceFile'); - return sourceFile; - }); + const sourceFiles = this._getChangedTsFiles() + .map((fileName) => this._getTsProgram().getSourceFile(fileName)) + // At this point we shouldn't need to filter out undefined files, because any ts file + // that changed should be emitted. + // But due to hostReplacementPaths there can be files (the environment files) + // that changed but aren't part of the compilation, specially on `ng test`. + // So we ignore missing source files files here. + // hostReplacementPaths needs to be fixed anyway to take care of the following issue. + // https://github.com/angular/angular-cli/issues/7305#issuecomment-332150230 + .filter((x) => !!x); // Emit files. time('AngularCompilerPlugin._update._emit'); diff --git a/tests/e2e/setup/500-create-project.ts b/tests/e2e/setup/500-create-project.ts index 300e5b3f1cca..a0ede37a6f01 100644 --- a/tests/e2e/setup/500-create-project.ts +++ b/tests/e2e/setup/500-create-project.ts @@ -52,16 +52,20 @@ export default function() { .then(() => updateTsConfig(json => { json['compilerOptions']['sourceRoot'] = '/'; })) - .then(() => updateJsonFile('src/tsconfig.spec.json', json => { + .then(() => { if (argv.nightly) { // ************************************************************************************* // REMOVE THIS WITH UPDATED NG5 SCHEMATICS // In ng5 we have to tell users users to update their tsconfig.json. - // `src/tsconfig.spec.json` needs to be updated with `"include": [ "**/*.ts" ]` + // `src/tsconfig.spec.json` needs to be updated with "polyfills.ts" on `include`. // ************************************************************************************* - json['include'] = ['**/*.ts']; + return updateJsonFile('src/tsconfig.spec.json', json => { + json['include'] = json['include'].concat('polyfills.ts'); + }) + // Ignore error if file doesn't exist. + .catch(() => { return; }); } - })) + }) .then(() => git('config', 'user.email', 'angular-core+e2e@google.com')) .then(() => git('config', 'user.name', 'Angular CLI E2e')) .then(() => git('config', 'commit.gpgSign', 'false')) diff --git a/tests/e2e/tests/lint/lint-no-project.ts b/tests/e2e/tests/lint/lint-no-project.ts index ccec63a0b97f..9e374ef040e4 100644 --- a/tests/e2e/tests/lint/lint-no-project.ts +++ b/tests/e2e/tests/lint/lint-no-project.ts @@ -24,13 +24,6 @@ export default function () { return stdout; }) .then(() => ng('set', 'lint.0.files', '"**/baz.ts"')) - .then(() => { - // Starting with ng5, tsconfig.spec.json includes all ts files, so linting for it must - // also set the files. - if (getGlobalVariable('argv').nightly) { - return ng('set', 'lint.1.files', '"**/baz.ts"'); - } - }) .then(() => writeFile('src/app/foo.ts', 'const foo = "";\n')) .then(() => writeFile('src/app/baz.ts', 'const baz = \'\';\n')) .then(() => ng('lint')) diff --git a/tests/e2e/tests/lint/lint-with-exclude.ts b/tests/e2e/tests/lint/lint-with-exclude.ts index 996485356c72..be792e2e9530 100644 --- a/tests/e2e/tests/lint/lint-with-exclude.ts +++ b/tests/e2e/tests/lint/lint-with-exclude.ts @@ -13,13 +13,6 @@ export default function () { return Promise.resolve() .then(() => ng('set', 'lint.0.exclude', '"**/foo.ts"')) - .then(() => { - // Starting with ng5, tsconfig.spec.json includes all ts files, so linting for it must - // also set the files. - if (getGlobalVariable('argv').nightly) { - return ng('set', 'lint.1.exclude', '"**/foo.ts"'); - } - }) .then(() => writeFile(fileName, 'const foo = "";\n')) .then(() => ng('lint')) .then(({ stdout }) => { diff --git a/tests/e2e/utils/project.ts b/tests/e2e/utils/project.ts index 8d69782b1278..b8fc7d288a10 100644 --- a/tests/e2e/utils/project.ts +++ b/tests/e2e/utils/project.ts @@ -44,18 +44,18 @@ export function createProject(name: string, ...args: string[]) { .then(() => argv['ng2'] ? useNg2() : Promise.resolve()) .then(() => argv.nightly || argv['ng-sha'] ? useSha() : Promise.resolve()) .then(() => { - // ************************************************************************************* - // REMOVE THIS WITH UPDATED NG5 SCHEMATICS - // In ng5 we have to tell users users to update their tsconfig.json. - // `src/tsconfig.spec.json` needs to be updated with `"include": [ "**/*.ts" ]` - // ************************************************************************************* - return updateJsonFile('src/tsconfig.spec.json', json => { - if (argv.nightly) { - json['include'] = ['**/*.ts']; - } - }) - // Ignore error if file doesn't exist. - .catch(() => { return; }); + if (argv.nightly) { + // ************************************************************************************* + // REMOVE THIS WITH UPDATED NG5 SCHEMATICS + // In ng5 we have to tell users users to update their tsconfig.json. + // `src/tsconfig.spec.json` needs to be updated with "polyfills.ts" on `include`. + // ************************************************************************************* + return updateJsonFile('src/tsconfig.spec.json', json => { + json['include'] = json['include'].concat('polyfills.ts'); + }) + // Ignore error if file doesn't exist. + .catch(() => { return; }); + } }) .then(() => console.log(`Project ${name} created... Installing npm.`)) .then(() => silentNpm('install')); From 1041da77ee9429ea56d5b154bc8ca1f15d965fcb Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 30 Oct 2017 22:39:40 +0000 Subject: [PATCH 4/4] fix(@angular/cli): add polyfills manually in for ng5 unit tests --- .../cli/models/webpack-configs/typescript.ts | 11 +++++++++-- .../webpack/src/angular_compiler_plugin.ts | 15 +++++++++++---- packages/@ngtools/webpack/src/refactor.ts | 1 + tests/e2e/setup/500-create-project.ts | 14 -------------- tests/e2e/utils/project.ts | 14 -------------- 5 files changed, 21 insertions(+), 34 deletions(-) diff --git a/packages/@angular/cli/models/webpack-configs/typescript.ts b/packages/@angular/cli/models/webpack-configs/typescript.ts index d2787d6da328..86b6819deeb0 100644 --- a/packages/@angular/cli/models/webpack-configs/typescript.ts +++ b/packages/@angular/cli/models/webpack-configs/typescript.ts @@ -162,8 +162,15 @@ export function getNonAotTestConfig(wco: WebpackConfigOptions) { let pluginOptions: any = { tsConfigPath, skipCodeGeneration: true }; - // The options below only apply to AoTPlugin. - if (!AngularCompilerPlugin.isSupported()) { + if (AngularCompilerPlugin.isSupported()) { + if (appConfig.polyfills) { + // TODO: remove singleFileIncludes for 2.0, this is just to support old projects that did not + // include 'polyfills.ts' in `tsconfig.spec.json'. + const polyfillsPath = path.resolve(projectRoot, appConfig.root, appConfig.polyfills); + pluginOptions.singleFileIncludes = [polyfillsPath]; + } + } else { + // The options below only apply to AoTPlugin. // Force include main and polyfills. // This is needed for AngularCompilerPlugin compatibility with existing projects, // since TS compilation there is stricter and tsconfig.spec.ts doesn't include them. diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index c68916ea62e9..dc4f7aa59d25 100644 --- a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts @@ -60,6 +60,9 @@ export interface AngularCompilerPluginOptions { mainPath?: string; skipCodeGeneration?: boolean; hostReplacementPaths?: { [path: string]: string }; + // TODO: remove singleFileIncludes for 2.0, this is just to support old projects that did not + // include 'polyfills.ts' in `tsconfig.spec.json'. + singleFileIncludes?: string[]; i18nInFile?: string; i18nInFormat?: string; i18nOutFile?: string; @@ -83,6 +86,7 @@ export class AngularCompilerPlugin implements Tapable { // TS compilation. private _compilerOptions: CompilerOptions; private _rootNames: string[]; + private _singleFileIncludes: string[] = []; private _program: (ts.Program | Program); private _compilerHost: WebpackCompilerHost & CompilerHost; private _moduleResolutionCache: ts.ModuleResolutionCache; @@ -157,8 +161,9 @@ export class AngularCompilerPlugin implements Tapable { basePath = path.resolve(process.cwd(), options.basePath); } - // TODO: check if we can get this from readConfiguration - this._basePath = basePath; + if (options.singleFileIncludes !== undefined) { + this._singleFileIncludes.push(...options.singleFileIncludes); + } // Parse the tsconfig contents. const config = readConfiguration(this._tsConfigPath); @@ -166,8 +171,9 @@ export class AngularCompilerPlugin implements Tapable { throw new Error(formatDiagnostics(config.errors)); } - this._rootNames = config.rootNames; + this._rootNames = config.rootNames.concat(...this._singleFileIncludes); this._compilerOptions = config.options; + this._basePath = config.options.basePath; // Overwrite outDir so we can find generated files next to their .ts origin in compilerHost. this._compilerOptions.outDir = ''; @@ -295,7 +301,8 @@ export class AngularCompilerPlugin implements Tapable { // Get the root files from the ts config. // When a new root name (like a lazy route) is added, it won't be available from // following imports on the existing files, so we need to get the new list of root files. - this._rootNames = readConfiguration(this._tsConfigPath).rootNames; + const config = readConfiguration(this._tsConfigPath); + this._rootNames = config.rootNames.concat(...this._singleFileIncludes); // Update the forked type checker with all changed compilation files. // This includes templates, that also need to be reloaded on the type checker. diff --git a/packages/@ngtools/webpack/src/refactor.ts b/packages/@ngtools/webpack/src/refactor.ts index 1d5d040b6375..91faa5e37464 100644 --- a/packages/@ngtools/webpack/src/refactor.ts +++ b/packages/@ngtools/webpack/src/refactor.ts @@ -16,6 +16,7 @@ const MagicString = require('magic-string'); * @param max The maximum number of items to return. * @return all nodes of kind, or [] if none is found */ +// TODO: replace this with collectDeepNodes and add limits to collectDeepNodes export function findAstNodes( node: ts.Node | null, sourceFile: ts.SourceFile, diff --git a/tests/e2e/setup/500-create-project.ts b/tests/e2e/setup/500-create-project.ts index a0ede37a6f01..b4aa8f108e99 100644 --- a/tests/e2e/setup/500-create-project.ts +++ b/tests/e2e/setup/500-create-project.ts @@ -52,20 +52,6 @@ export default function() { .then(() => updateTsConfig(json => { json['compilerOptions']['sourceRoot'] = '/'; })) - .then(() => { - if (argv.nightly) { - // ************************************************************************************* - // REMOVE THIS WITH UPDATED NG5 SCHEMATICS - // In ng5 we have to tell users users to update their tsconfig.json. - // `src/tsconfig.spec.json` needs to be updated with "polyfills.ts" on `include`. - // ************************************************************************************* - return updateJsonFile('src/tsconfig.spec.json', json => { - json['include'] = json['include'].concat('polyfills.ts'); - }) - // Ignore error if file doesn't exist. - .catch(() => { return; }); - } - }) .then(() => git('config', 'user.email', 'angular-core+e2e@google.com')) .then(() => git('config', 'user.name', 'Angular CLI E2e')) .then(() => git('config', 'commit.gpgSign', 'false')) diff --git a/tests/e2e/utils/project.ts b/tests/e2e/utils/project.ts index b8fc7d288a10..a11784cfbd12 100644 --- a/tests/e2e/utils/project.ts +++ b/tests/e2e/utils/project.ts @@ -43,20 +43,6 @@ export function createProject(name: string, ...args: string[]) { .then(() => useCIDefaults()) .then(() => argv['ng2'] ? useNg2() : Promise.resolve()) .then(() => argv.nightly || argv['ng-sha'] ? useSha() : Promise.resolve()) - .then(() => { - if (argv.nightly) { - // ************************************************************************************* - // REMOVE THIS WITH UPDATED NG5 SCHEMATICS - // In ng5 we have to tell users users to update their tsconfig.json. - // `src/tsconfig.spec.json` needs to be updated with "polyfills.ts" on `include`. - // ************************************************************************************* - return updateJsonFile('src/tsconfig.spec.json', json => { - json['include'] = json['include'].concat('polyfills.ts'); - }) - // Ignore error if file doesn't exist. - .catch(() => { return; }); - } - }) .then(() => console.log(`Project ${name} created... Installing npm.`)) .then(() => silentNpm('install')); }