From dc2bf08f6ee7c8ad7b235a2d03ad344e9b45ed69 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 1 Oct 2018 14:53:50 +0100 Subject: [PATCH 1/5] feat(@ngtools/webpack): support custom logging --- .../webpack/src/angular_compiler_plugin.ts | 47 ++++++++++++++++--- packages/ngtools/webpack/src/type_checker.ts | 40 ++++------------ .../webpack/src/type_checker_messages.ts | 45 ++++++++++++++++++ .../webpack/src/type_checker_worker.ts | 6 ++- 4 files changed, 99 insertions(+), 39 deletions(-) create mode 100644 packages/ngtools/webpack/src/type_checker_messages.ts diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 011fad1684f3..c25ff7563b28 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -5,7 +5,16 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import { Path, dirname, getSystemPath, normalize, resolve, virtualFs } from '@angular-devkit/core'; +import { + Path, + dirname, + getSystemPath, + logging, + normalize, + resolve, + virtualFs, +} from '@angular-devkit/core'; +import { createConsoleLogger } from '@angular-devkit/core/node'; import { CompilerHost, CompilerOptions, @@ -45,7 +54,15 @@ import { replaceServerBootstrap, } from './transformers'; import { collectDeepNodes } from './transformers/ast_helpers'; -import { AUTO_START_ARG, InitMessage, UpdateMessage } from './type_checker'; +import { + AUTO_START_ARG, +} from './type_checker'; +import { + InitMessage, + LogMessage, + MESSAGE_KIND, + UpdateMessage, +} from './type_checker_messages'; import { VirtualFileSystemDecorator, VirtualWatchFileSystemDecorator, @@ -59,7 +76,7 @@ import { WebpackInputHost } from './webpack-input-host'; const treeKill = require('tree-kill'); -export interface ContextElementDependency {} +export interface ContextElementDependency { } export interface ContextElementDependencyConstructor { new(modulePath: string, name: string): ContextElementDependency; @@ -85,6 +102,7 @@ export interface AngularCompilerPluginOptions { missingTranslation?: string; platform?: PLATFORM; nameLazyFiles?: boolean; + logger?: logging.Logger; // added to the list of lazy routes additionalLazyModules?: { [module: string]: string }; @@ -141,6 +159,9 @@ export class AngularCompilerPlugin { private _typeCheckerProcess: ChildProcess | null; private _forkedTypeCheckerInitialized = false; + // Logging. + private _logger: logging.Logger; + private get _ngCompilerSupportsNewApi() { if (this._JitMode) { return false; @@ -179,6 +200,8 @@ export class AngularCompilerPlugin { private _setupOptions(options: AngularCompilerPluginOptions) { time('AngularCompilerPlugin._setupOptions'); + this._logger = options.logger || createConsoleLogger(); + // Fill in the missing options. if (!options.hasOwnProperty('tsConfigPath')) { throw new Error('Must specify "tsConfigPath" in the configuration of @ngtools/webpack.'); @@ -415,7 +438,7 @@ export class AngularCompilerPlugin { }), // TODO: fix compiler-cli typings; entryModule should not be string, but also optional. // tslint:disable-next-line:no-non-null-assertion - entryModule: this._entryModule !, + entryModule: this._entryModule!, }); timeEnd('AngularCompilerPlugin._getLazyRoutesFromNgtools'); @@ -542,6 +565,18 @@ export class AngularCompilerPlugin { forkArgs, forkOptions); + // Handle child messages. + this._typeCheckerProcess.on('message', message => { + switch (message.kind) { + case MESSAGE_KIND.Log: + const logMessage = message as LogMessage; + this._logger.log(logMessage.level, logMessage.message); + break; + default: + throw new Error(`TypeChecker: Unexpected message received: ${message}.`); + } + }); + // Handle child process exit. this._typeCheckerProcess.once('exit', (_, signal) => { this._typeCheckerProcess = null; @@ -748,10 +783,10 @@ export class AngularCompilerPlugin { const name = request.request; const issuer = request.contextInfo.issuer; if (name.endsWith('.ts') || name.endsWith('.tsx') - || (issuer && /\.ts|ngfactory\.js$/.test(issuer))) { + || (issuer && /\.ts|ngfactory\.js$/.test(issuer))) { try { await this.done; - } catch {} + } catch { } } } diff --git a/packages/ngtools/webpack/src/type_checker.ts b/packages/ngtools/webpack/src/type_checker.ts index 7e134c9d3b81..57f18222b84c 100644 --- a/packages/ngtools/webpack/src/type_checker.ts +++ b/packages/ngtools/webpack/src/type_checker.ts @@ -5,7 +5,6 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import { terminal } from '@angular-devkit/core'; import { NodeJsSyncHost } from '@angular-devkit/core/node'; import { CompilerHost, @@ -19,37 +18,10 @@ import * as ts from 'typescript'; import { time, timeEnd } from './benchmark'; import { WebpackCompilerHost } from './compiler_host'; import { CancellationToken, gatherDiagnostics } from './gather_diagnostics'; +import { LogMessage, TypeCheckerMessage } from './type_checker_messages'; // This file should run in a child process with the AUTO_START_ARG argument - - -export enum MESSAGE_KIND { - Init, - Update, -} - -export abstract class TypeCheckerMessage { - constructor(public kind: MESSAGE_KIND) { } -} - -export class InitMessage extends TypeCheckerMessage { - constructor( - public compilerOptions: ts.CompilerOptions, - public basePath: string, - public jitMode: boolean, - public rootNames: string[], - ) { - super(MESSAGE_KIND.Init); - } -} - -export class UpdateMessage extends TypeCheckerMessage { - constructor(public rootNames: string[], public changedCompilationFiles: string[]) { - super(MESSAGE_KIND.Update); - } -} - export const AUTO_START_ARG = '9d93e901-158a-4cf9-ba1b-2f0582ffcfeb'; export class TypeChecker { @@ -124,7 +96,7 @@ export class TypeChecker { if (errors.length > 0) { const message = formatDiagnostics(errors); - console.error(terminal.bold(terminal.red('ERROR in ' + message))); + this.sendMessage(new LogMessage('error', 'ERROR in ' + message)); } else { // Reset the changed file tracker only if there are no errors. this._compilerHost.resetChangedFileTracker(); @@ -132,11 +104,17 @@ export class TypeChecker { if (warnings.length > 0) { const message = formatDiagnostics(warnings); - console.error(terminal.bold(terminal.yellow('WARNING in ' + message))); + this.sendMessage(new LogMessage('warn', 'WARNING in ' + message)); } } } + private sendMessage(msg: TypeCheckerMessage) { + if (process.send) { + process.send(msg); + } + } + public update(rootNames: string[], changedCompilationFiles: string[], cancellationToken: CancellationToken) { this._update(rootNames, changedCompilationFiles); diff --git a/packages/ngtools/webpack/src/type_checker_messages.ts b/packages/ngtools/webpack/src/type_checker_messages.ts new file mode 100644 index 000000000000..4fc0a65c9ba7 --- /dev/null +++ b/packages/ngtools/webpack/src/type_checker_messages.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import { logging } from '@angular-devkit/core'; +import * as ts from 'typescript'; + + +export enum MESSAGE_KIND { + Init, + Update, + Log, +} + +export abstract class TypeCheckerMessage { + constructor(public kind: MESSAGE_KIND) { } +} + +export class InitMessage extends TypeCheckerMessage { + constructor( + public compilerOptions: ts.CompilerOptions, + public basePath: string, + public jitMode: boolean, + public rootNames: string[], + public hostReplacementPaths: { [path: string]: string }, + ) { + super(MESSAGE_KIND.Init); + } +} + +export class UpdateMessage extends TypeCheckerMessage { + constructor(public rootNames: string[], public changedCompilationFiles: string[]) { + super(MESSAGE_KIND.Update); + } +} + +export class LogMessage extends TypeCheckerMessage { + constructor(public level: logging.LogLevel, public message: string) { + super(MESSAGE_KIND.Log); + } +} + diff --git a/packages/ngtools/webpack/src/type_checker_worker.ts b/packages/ngtools/webpack/src/type_checker_worker.ts index 777477292357..5cbab0379de6 100644 --- a/packages/ngtools/webpack/src/type_checker_worker.ts +++ b/packages/ngtools/webpack/src/type_checker_worker.ts @@ -10,12 +10,14 @@ import { time, timeEnd } from './benchmark'; import { CancellationToken } from './gather_diagnostics'; import { AUTO_START_ARG, + TypeChecker, +} from './type_checker'; +import { InitMessage, MESSAGE_KIND, - TypeChecker, TypeCheckerMessage, UpdateMessage, -} from './type_checker'; +} from './type_checker_messages'; let typeChecker: TypeChecker; let lastCancellationToken: CancellationToken; From de2632fad1ffeb0e7cb8771fc3d2cc5f72e9a2a5 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 1 Oct 2018 14:54:35 +0100 Subject: [PATCH 2/5] feat(@angular-devkit/build-angular): pass logger to ngtools/webpack --- .../src/angular-cli-files/models/build-options.ts | 5 +++-- .../angular-cli-files/models/webpack-configs/typescript.ts | 1 + packages/angular_devkit/build_angular/src/browser/index.ts | 1 + .../angular_devkit/build_angular/src/extract-i18n/index.ts | 1 + packages/angular_devkit/build_angular/src/karma/index.ts | 1 + packages/angular_devkit/build_angular/src/server/index.ts | 1 + 6 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts index 3ee86311eacc..5a33f1e32913 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/build-options.ts @@ -8,8 +8,8 @@ // TODO: cleanup this file, it's copied as is from Angular CLI. -// tslint:disable-next-line:no-implicit-dependencies -import * as ts from 'typescript'; +import { logging } from '@angular-devkit/core'; +import * as ts from 'typescript'; // tslint:disable-line:no-implicit-dependencies import { AssetPatternObject, Budget, @@ -74,6 +74,7 @@ export interface WebpackTestOptions extends BuildOptions { export interface WebpackConfigOptions { root: string; + logger: logging.Logger; projectRoot: string; sourceRoot?: string; buildOptions: T; diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts index 82bd2e7ae38f..f939ca054f30 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/typescript.ts @@ -81,6 +81,7 @@ function _createAotPlugin( nameLazyFiles: buildOptions.namedChunks, forkTypeChecker: buildOptions.forkTypeChecker, contextElementDependencyConstructor: require('webpack/lib/dependencies/ContextElementDependency'), + logger: wco.logger, ...options, }; return new AngularCompilerPlugin(pluginOptions); diff --git a/packages/angular_devkit/build_angular/src/browser/index.ts b/packages/angular_devkit/build_angular/src/browser/index.ts index 39cb7eb43819..ab49981a9f0c 100644 --- a/packages/angular_devkit/build_angular/src/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/browser/index.ts @@ -132,6 +132,7 @@ export class BrowserBuilder implements Builder { wco = { root: getSystemPath(root), + logger: this.context.logger, projectRoot: getSystemPath(projectRoot), buildOptions: options, tsConfig, diff --git a/packages/angular_devkit/build_angular/src/extract-i18n/index.ts b/packages/angular_devkit/build_angular/src/extract-i18n/index.ts index 34d327fedf7e..06fdc798ed4b 100644 --- a/packages/angular_devkit/build_angular/src/extract-i18n/index.ts +++ b/packages/angular_devkit/build_angular/src/extract-i18n/index.ts @@ -118,6 +118,7 @@ export class ExtractI18nBuilder implements Builder { wco = { root: getSystemPath(root), + logger: this.context.logger, projectRoot: getSystemPath(projectRoot), // TODO: use only this.options, it contains all flags and configs items already. buildOptions: options, diff --git a/packages/angular_devkit/build_angular/src/karma/index.ts b/packages/angular_devkit/build_angular/src/karma/index.ts index 3d1b066d9f1b..5b97d5eb8c9c 100644 --- a/packages/angular_devkit/build_angular/src/karma/index.ts +++ b/packages/angular_devkit/build_angular/src/karma/index.ts @@ -139,6 +139,7 @@ export class KarmaBuilder implements Builder { wco = { root: getSystemPath(root), + logger: this.context.logger, projectRoot: getSystemPath(projectRoot), sourceRoot: sourceRoot && getSystemPath(sourceRoot), // TODO: use only this.options, it contains all flags and configs items already. diff --git a/packages/angular_devkit/build_angular/src/server/index.ts b/packages/angular_devkit/build_angular/src/server/index.ts index 590030022bdb..3874885dc77f 100644 --- a/packages/angular_devkit/build_angular/src/server/index.ts +++ b/packages/angular_devkit/build_angular/src/server/index.ts @@ -83,6 +83,7 @@ export class ServerBuilder implements Builder { wco = { root: getSystemPath(root), + logger: this.context.logger, projectRoot: getSystemPath(projectRoot), // TODO: use only this.options, it contains all flags and configs items already. buildOptions: { From daea31f4bef787b539f0607c37cf7ba141993ef4 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 1 Oct 2018 15:14:59 +0100 Subject: [PATCH 3/5] feat(@ngtools/webpack): support hostReplacementPaths Fix #12197 --- .../webpack/src/angular_compiler_plugin.ts | 7 ++++++- packages/ngtools/webpack/src/type_checker.ts | 16 +++++++++++++++- .../ngtools/webpack/src/type_checker_worker.ts | 1 + 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index c25ff7563b28..9a2edcf7e154 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -602,8 +602,13 @@ export class AngularCompilerPlugin { private _updateForkedTypeChecker(rootNames: string[], changedCompilationFiles: string[]) { if (this._typeCheckerProcess) { if (!this._forkedTypeCheckerInitialized) { + let hostReplacementPaths = {}; + if (this._options.hostReplacementPaths + && typeof this._options.hostReplacementPaths != 'function') { + hostReplacementPaths = this._options.hostReplacementPaths; + } this._typeCheckerProcess.send(new InitMessage(this._compilerOptions, this._basePath, - this._JitMode, this._rootNames)); + this._JitMode, this._rootNames, hostReplacementPaths)); this._forkedTypeCheckerInitialized = true; } this._typeCheckerProcess.send(new UpdateMessage(rootNames, changedCompilationFiles)); diff --git a/packages/ngtools/webpack/src/type_checker.ts b/packages/ngtools/webpack/src/type_checker.ts index 57f18222b84c..17503fb1b18d 100644 --- a/packages/ngtools/webpack/src/type_checker.ts +++ b/packages/ngtools/webpack/src/type_checker.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import { normalize, resolve, virtualFs } from '@angular-devkit/core'; import { NodeJsSyncHost } from '@angular-devkit/core/node'; import { CompilerHost, @@ -33,12 +34,25 @@ export class TypeChecker { _basePath: string, private _JitMode: boolean, private _rootNames: string[], + hostReplacementPaths: { [path: string]: string }, ) { time('TypeChecker.constructor'); + const host = new virtualFs.AliasHost(new NodeJsSyncHost()); + + // Add file replacements. + for (const from in hostReplacementPaths) { + const normalizedFrom = resolve(normalize(_basePath), normalize(from)); + const normalizedWith = resolve( + normalize(_basePath), + normalize(hostReplacementPaths[from]), + ); + host.aliases.set(normalizedFrom, normalizedWith); + } + const compilerHost = new WebpackCompilerHost( _compilerOptions, _basePath, - new NodeJsSyncHost(), + host, ); // 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 diff --git a/packages/ngtools/webpack/src/type_checker_worker.ts b/packages/ngtools/webpack/src/type_checker_worker.ts index 5cbab0379de6..f04abd01ea1d 100644 --- a/packages/ngtools/webpack/src/type_checker_worker.ts +++ b/packages/ngtools/webpack/src/type_checker_worker.ts @@ -34,6 +34,7 @@ if (process.argv.indexOf(AUTO_START_ARG) >= 0) { initMessage.basePath, initMessage.jitMode, initMessage.rootNames, + initMessage.hostReplacementPaths, ); break; case MESSAGE_KIND.Update: From 8aa83153682a0c784d23396a40025729773ab865 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 1 Oct 2018 15:15:47 +0100 Subject: [PATCH 4/5] test(@angular-devkit/build-angular): add test for type checker replacements --- .../test/browser/replacements_spec_large.ts | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/packages/angular_devkit/build_angular/test/browser/replacements_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/replacements_spec_large.ts index e73e56aec1d4..e1fda961ae9b 100644 --- a/packages/angular_devkit/build_angular/test/browser/replacements_spec_large.ts +++ b/packages/angular_devkit/build_angular/test/browser/replacements_spec_large.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import { runTargetSpec } from '@angular-devkit/architect/testing'; +import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing'; import { join, normalize, virtualFs } from '@angular-devkit/core'; -import { takeWhile, tap } from 'rxjs/operators'; +import { of, race } from 'rxjs'; +import { delay, filter, map, take, takeUntil, takeWhile, tap } from 'rxjs/operators'; import { browserTargetSpec, host } from '../utils'; @@ -154,4 +155,53 @@ describe('Browser Builder file replacements', () => { ); }); + it('file replacements work with forked type checker on watch mode', async () => { + host.writeMultipleFiles({ + 'src/file-replaced.ts': 'export var obj = { one: 1, two: 2 };', + 'src/file.ts': `export var obj = { one: 1 };`, + 'src/main.ts': ` + import { obj } from './file'; + console.log(obj.two); + `, + }); + + const overrides = { + fileReplacements: [{ + replace: normalize('/src/file.ts'), + with: normalize('/src/file-replaced.ts'), + }], + watch: true, + }; + + const unexpectedError = `Property 'two' does not exist on type '{ one: number; }'`; + const expectedError = `Property 'prop' does not exist on type '{}'`; + const logger = new TestLogger('rebuild-type-errors'); + + // Race between a timeout and the expected log entry. + const stop$ = race( + of(null).pipe(delay(DefaultTimeout * 2 / 3)), + logger.pipe( + filter(entry => entry.message.includes(expectedError)), + map(entry => entry.message), + take(1), + ), + ); + + let errorAdded = false; + runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout, logger).pipe( + tap((buildEvent) => expect(buildEvent.success).toBe(true, 'build should succeed')), + tap(() => { + // Introduce a known type error to detect in the logger filter. + if (!errorAdded) { + host.appendToFile('src/main.ts', 'console.log({}.prop);'); + errorAdded = true; + } + }), + takeUntil(stop$), + ).subscribe(); + + const res = await stop$.toPromise(); + expect(res).not.toBe(null, 'Test timed out.'); + expect(res).not.toContain(unexpectedError); + }); }); From f778a876d4d35ab79a016030402125de856b4441 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 2 Oct 2018 18:00:05 +0100 Subject: [PATCH 5/5] fix(@angular-devkit/build-angular): workaround karma issue See https://github.com/karma-runner/karma/issues/3154 --- packages/angular_devkit/build_angular/src/karma/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/angular_devkit/build_angular/src/karma/index.ts b/packages/angular_devkit/build_angular/src/karma/index.ts index 5b97d5eb8c9c..96c58a1d93f3 100644 --- a/packages/angular_devkit/build_angular/src/karma/index.ts +++ b/packages/angular_devkit/build_angular/src/karma/index.ts @@ -89,6 +89,10 @@ export class KarmaBuilder implements Builder { // Pass onto Karma to emit BuildEvents. successCb: () => obs.next({ success: true }), failureCb: () => obs.next({ success: false }), + // Workaround for https://github.com/karma-runner/karma/issues/3154 + // When this workaround is removed, user projects need to be updated to use a Karma + // version that has a fix for this issue. + toJSON: () => { }, }; // TODO: inside the configs, always use the project root and not the workspace root.