fix(jsii): --watch causes immediate failure

A change in the TypeScript compiler API around how the compiler host
reacts when invoked in `--watch` mode caused a failure in `jsii` due to
the method changing behaviors from not returning at all when watching to
instead returning an object that can be used to inspect/influence the
background watch process.

Changed the behavior of the `Compiler` interface so it now acts as an
`EventEmitter` when operating in `--watch` mode, and introduced a test
that confirms that operating with `--watch` results in the correct
behavior of watching for file changes and dynamically re-compiling.

Fixes #1149

packages/jsii/bin/jsii.ts

@@ -57,12 +57,17 @@ const warningTypes = Object.keys(enabledWarnings);
 
   const compiler = new Compiler({
     projectInfo,
-    watch: argv.watch,
     projectReferences: argv['project-references'],
     failOnWarnings: argv['fail-on-warnings'],
   });
 
-  return { projectRoot, emitResult: await compiler.emit() };
+  compiler.on('statusChanged', diagnostic => utils.logDiagnostic(diagnostic, projectRoot));
+
+  const result = argv.watch
+    // When watching, return a promise that never resolves... #magic
+    ? new Promise<never>((_, ko) => { compiler.watch().catch(ko); })
+    : compiler.emit();
+  return { projectRoot, emitResult: await result };
 })().then(({ projectRoot, emitResult }) => {
   for (const diagnostic of emitResult.diagnostics) {
     utils.logDiagnostic(diagnostic, projectRoot);
@@ -82,19 +87,20 @@ function _configureLog4js(verbosity: number) {
         type: 'stderr',
         layout: { type: 'colored' }
       },
-      diagnostics: {
+      [DIAGNOSTICS]: {
         type: 'stdout',
         layout: { type: 'messagePassThrough' }
       }
     },
     categories: {
       default: { appenders: ['console'], level: _logLevel() },
-      [DIAGNOSTICS]: { appenders: ['diagnostics'], level: _logLevel() }
+      // The diagnostics logger must be set to INFO or more verbose, or watch won't show important messages
+      [DIAGNOSTICS]: { appenders: ['diagnostics'], level: _logLevel(Math.max(verbosity, 1)) }
     }
   });
 
-  function _logLevel(): keyof log4js.Levels {
-    switch (verbosity) {
+  function _logLevel(verbosityLevel = verbosity): keyof log4js.Levels {
+    switch (verbosityLevel) {
       case 0: return 'WARN';
       case 1: return 'INFO';
       case 2: return 'DEBUG';

packages/jsii/lib/compiler.ts

@@ -1,11 +1,12 @@
 import * as Case from 'case';
 import * as colors from 'colors/safe';
+import * as events from 'events';
 import * as fs from 'fs-extra';
 import * as log4js from 'log4js';
 import * as path from 'path';
 import * as ts from 'typescript';
 import { Assembler } from './assembler';
-import { EmitResult, Emitter } from './emitter';
+import { EmitResult, Emitter, Diagnostic } from './emitter';
 import { ProjectInfo } from './project-info';
 import * as utils from './utils';
 
@@ -40,8 +41,6 @@ export const JSII_DIAGNOSTICS_CODE = 9999;
 export interface CompilerOptions {
   /** The information about the project to be built */
   projectInfo: ProjectInfo;
-  /** Whether the compiler should watch for changes or just compile once */
-  watch?: boolean;
   /** Whether to detect and generate TypeScript project references */
   projectReferences?: boolean;
   /** Whether to fail when a warning is emitted */
@@ -61,6 +60,7 @@ export class Compiler implements Emitter {
   private rootFiles: string[] = [];
   private readonly configPath: string;
   private readonly projectReferences: boolean;
+  private readonly eventEmitter = new events.EventEmitter();
 
   public constructor(private readonly options: CompilerOptions) {
     this.compilerHost = ts.createCompilerHost(COMPILER_OPTIONS);
@@ -72,28 +72,70 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Compiles the configured program.
-     *
-     * @param files can be specified to override the standard source code location logic. Useful for example when testing "negatives".
-     */
+   * Adds a listener for the 'result' event that is called with EmitResult.
+   *
+   * @param event    the 'result' event
+   * @param listener the listener to be called with the EmitResult.
+   */
+  public on(event: 'result', listener: (result: EmitResult) => any): this;
+  /**
+   * Adds a listener for the 'statusChanged' event that is called with a Diagnostic whenever the
+   * status of the current watch process changes.
+   *
+   * @param event    the 'statusChanged' event
+   * @param listener the listener to be called with the Diagnostic.
+   */
+  public on(event: 'statusChanged', listener: (message: Diagnostic) => any): this;
+  public on(event: string, listener: (...args: any[]) => any): this {
+    this.eventEmitter.on(event, listener);
+    return this;
+  }
+
+  /**
+   * Adds a once-only listener for the 'result' event that is called with EmitResult.
+   *
+   * @param event    the 'result' event
+   * @param listener the listener to be called with the EmitResult.
+   */
+  public once(event: 'result', listener: (result: EmitResult) => any): this;
+  /**
+   * Adds a once-only listener for the 'statusChanged' event that is called with a Diagnostic
+   * whenever the status of the current watch process changes.
+   *
+   * @param event    the 'statusChanged' event
+   * @param listener the listener to be called with the Diagnostic.
+   */
+  public once(event: 'statusChanged', listener: (message: Diagnostic) => any): this;
+  public once(event: string, listener: (...args: any[]) => any): this {
+    this.eventEmitter.once(event, listener);
+    return this;
+  }
+
+  /**
+   * Compiles the configured program.
+   *
+   * @param files can be specified to override the standard source code location logic. Useful for example when testing "negatives".
+   */
   public async emit(...files: string[]): Promise<EmitResult | never> {
     await this.buildTypeScriptConfig();
     await this.writeTypeScriptConfig();
     this.rootFiles = this.determineSources(files);
 
-    if (this.options.watch) {
-      if (files.length > 0) {
-        throw new Error('Files cannot be specified in watch mode!');
-      }
-      return this._startWatch();
-    }
     return this._buildOnce();
 
   }
 
+  public async watch(): Promise<Watch> {
+    await this.buildTypeScriptConfig();
+    await this.writeTypeScriptConfig();
+    this.rootFiles = this.determineSources([]);
+
+    return this._startWatch();
+  }
+
   /**
-     * Do a single build
-     */
+   * Do a single build
+   */
   private async _buildOnce(): Promise<EmitResult> {
     if (!this.compilerHost.getDefaultLibLocation) {
       throw new Error('No default library location was found on the TypeScript compiler host!');
@@ -114,9 +156,9 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Start a watch on the config that has been written to disk
-     */
-  private async _startWatch(): Promise<never> {
+   * Start a watch on the config that has been written to disk
+   */
+  private _startWatch(): Watch {
     const pi = this.options.projectInfo;
     const projectRoot = pi.projectRoot;
     const host = ts.createWatchCompilerHost(
@@ -126,7 +168,10 @@ export class Compiler implements Emitter {
         ...COMPILER_OPTIONS,
         noEmitOnError: false,
       },
-      { ...ts.sys, getCurrentDirectory() { return projectRoot; } }
+      { ...ts.sys, getCurrentDirectory() { return projectRoot; } },
+      ts.createSemanticDiagnosticsBuilderProgram,
+      undefined,
+      diag => this.eventEmitter.emit('statusChanged', diag)
     );
     if (!host.getDefaultLibLocation) {
       throw new Error('No default library location was found on the TypeScript compiler host!');
@@ -141,9 +186,7 @@ export class Compiler implements Emitter {
 
       if (orig) { orig.call(host, builderProgram); }
     };
-    ts.createWatchProgram(host);
-    // Previous call never returns
-    return Promise.reject(new Error('Unexpectedly returned from createWatchProgram'));
+    return ts.createWatchProgram(host);
   }
 
   private async _consumeProgram(program: ts.Program, stdlib: string): Promise<EmitResult> {
@@ -171,14 +214,16 @@ export class Compiler implements Emitter {
       LOG.error(`Error during type model analysis: ${e}`);
     }
 
-    return { emitSkipped: hasErrors, diagnostics, emittedFiles: emit.emittedFiles };
+    const result: EmitResult = { emitSkipped: hasErrors, diagnostics, emittedFiles: emit.emittedFiles };
+    this.eventEmitter.emit('result', result);
+    return result;
   }
 
   /**
-     * Build the TypeScript config object
-     *
-     * This is the object that will be written to disk.
-     */
+   * Build the TypeScript config object
+   *
+   * This is the object that will be written to disk.
+   */
   private async buildTypeScriptConfig() {
     let references: string[] | undefined;
     let composite: boolean | undefined;
@@ -212,10 +257,10 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Creates a `tsconfig.json` file to improve the IDE experience.
-     *
-     * @return the fully qualified path to the ``tsconfig.json`` file
-     */
+   * Creates a `tsconfig.json` file to improve the IDE experience.
+   *
+   * @return the fully qualified path to the ``tsconfig.json`` file
+   */
   private async writeTypeScriptConfig(): Promise<void> {
     const commentKey = '_generated_by_jsii_';
     const commentValue = 'Generated by jsii - safe to delete, and ideally should be in .gitignore';
@@ -233,15 +278,15 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Find all dependencies that look like TypeScript projects.
-     *
-     * Enumerate all dependencies, if they have a tsconfig.json file with
-     * "composite: true" we consider them project references.
-     *
-     * (Note: TypeScript seems to only correctly find transitive project references
-     * if there's an "index" tsconfig.json of all projects somewhere up the directory
-     * tree)
-     */
+   * Find all dependencies that look like TypeScript projects.
+   *
+   * Enumerate all dependencies, if they have a tsconfig.json file with
+   * "composite: true" we consider them project references.
+   *
+   * (Note: TypeScript seems to only correctly find transitive project references
+   * if there's an "index" tsconfig.json of all projects somewhere up the directory
+   * tree)
+   */
   private async findProjectReferences(): Promise<string[]> {
     const pkg = this.options.projectInfo.packageJson;
 
@@ -277,12 +322,12 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Find source files using the same mechanism that the TypeScript compiler itself uses.
-     *
-     * Respects includes/excludes/etc.
-     *
-     * This makes it so that running 'tsc' and running 'jsii' has the same behavior.
-     */
+   * Find source files using the same mechanism that the TypeScript compiler itself uses.
+   *
+   * Respects includes/excludes/etc.
+   *
+   * This makes it so that running 'tsc' and running 'jsii' has the same behavior.
+   */
   private determineSources(files: string[]): string[] {
     const ret = new Array<string>();
 
@@ -298,18 +343,18 @@ export class Compiler implements Emitter {
   }
 
   /**
-     * Resolve the given dependency name from the current package, and find the associated tsconfig.json location
-     *
-     * Because we have the following potential directory layout:
-     *
-     *   package/node_modules/some_dependency
-     *   package/tsconfig.json
-     *
-     * We resolve symlinks and only find a "TypeScript" dependency if doesn't have 'node_modules' in
-     * the path after resolving symlinks (i.e., if it's a peer package in the same monorepo).
-     *
-     * Returns undefined if no such tsconfig could be found.
-     */
+   * Resolve the given dependency name from the current package, and find the associated tsconfig.json location
+   *
+   * Because we have the following potential directory layout:
+   *
+   *   package/node_modules/some_dependency
+   *   package/tsconfig.json
+   *
+   * We resolve symlinks and only find a "TypeScript" dependency if doesn't have 'node_modules' in
+   * the path after resolving symlinks (i.e., if it's a peer package in the same monorepo).
+   *
+   * Returns undefined if no such tsconfig could be found.
+   */
   private async findMonorepoPeerTsconfig(depName: string): Promise<string | undefined> {
     const paths = nodeJsCompatibleSearchPaths(this.options.projectInfo.projectRoot);
 
@@ -329,6 +374,16 @@ export class Compiler implements Emitter {
   }
 }
 
+/**
+ * A handle to a watch-mode operation.
+ */
+export interface Watch {
+  /**
+   * Terminates the watch process.
+   */
+  close(): void;
+}
+
 function _pathOfLibraries(host: ts.CompilerHost | ts.WatchCompilerHost<any>): string[] {
   if (!COMPILER_OPTIONS.lib || COMPILER_OPTIONS.lib.length === 0) { return []; }
   const lib = host.getDefaultLibLocation?.();

packages/jsii/lib/helpers.ts

@@ -25,7 +25,7 @@ export async function sourceToAssemblyHelper(source: string, cb?: (obj: PackageI
   return inTempDir(async () => {
     const fileName = 'index.ts';
     await fs.writeFile(fileName, source, { encoding: 'utf-8' });
-    const compiler = new Compiler({ projectInfo: await makeProjectInfo(fileName, cb), watch: false });
+    const compiler = new Compiler({ projectInfo: await makeProjectInfo(fileName, cb) });
     const emitResult = await compiler.emit();
 
     const errors = emitResult.diagnostics.filter(d => d.category === DiagnosticCategory.Error);

packages/jsii/lib/project-info.ts

@@ -174,7 +174,7 @@ async function _loadDependencies(dependencies: { [name: string]: string | spec.P
     const pkg = _tryResolveAssembly(name, localPackage, searchPath);
     LOG.debug(`Resolved dependency ${name} to ${pkg}`);
     // eslint-disable-next-line no-await-in-loop
-    const assm = await loadAndValidateAssembly(pkg);
+    const assm = await loadAndValidateAssembly(pkg).catch(e => { throw new Error(`Error loading ${pkg}: ${e}`); });
     if (!version.intersects(new semver.Range(assm.version))) {
       throw new Error(`Declared dependency on version ${versionString} of ${name}, but version ${assm.version} was found`);
     }

packages/jsii/lib/utils.ts

@@ -1,4 +1,5 @@
 import * as log4js from 'log4js';
+import { cursorTo, clearScreenDown } from 'readline';
 import * as ts from 'typescript';
 import { DIAGNOSTICS } from './compiler';
 
@@ -19,6 +20,8 @@ export function diagnosticsLogger(logger: log4js.Logger, diagnostic: ts.Diagnost
       if (!logger.isWarnEnabled()) { return undefined; }
       return logger.warn.bind(logger);
     case ts.DiagnosticCategory.Message:
+      if (!logger.isInfoEnabled()) { return undefined; }
+      return logger.info.bind(logger);
     case ts.DiagnosticCategory.Suggestion:
     default:
       if (!logger.isDebugEnabled()) { return undefined; }
@@ -27,15 +30,26 @@ export function diagnosticsLogger(logger: log4js.Logger, diagnostic: ts.Diagnost
 }
 
 export function logDiagnostic(diagnostic: ts.Diagnostic, projectRoot: string) {
-  const formatDiagnosticsHost = {
+  if (process.stdout.isTTY && process.stderr.isTTY && diagnostic.code === 6032) {
+    // Clearing the screen if the code is that of "File change detected. Starting incremental compilation..."
+    for (const stream of [process.stdout, process.stderr]) {
+      stream.write('\n'.repeat(stream.rows ?? 0));
+      cursorTo(stream, 0, 0);
+      clearScreenDown(stream);
+    }
+  }
+
+  const formatDiagnosticsHost: ts.FormatDiagnosticsHost = {
     getCurrentDirectory: () => projectRoot,
-    getCanonicalFileName(fileName: string) { return fileName; },
-    getNewLine() { return '\n'; }
+    getCanonicalFileName: fileName => fileName,
+    getNewLine: () => ts.sys.newLine,
   };
 
-  const message = diagnostic.file
-    ? ts.formatDiagnosticsWithColorAndContext([diagnostic], formatDiagnosticsHost)
-    : ts.formatDiagnostics([diagnostic], formatDiagnosticsHost);
+  const message = diagnostic.category === ts.DiagnosticCategory.Message && typeof diagnostic.messageText === 'string'
+    ? diagnostic.messageText
+    : diagnostic.file
+      ? ts.formatDiagnosticsWithColorAndContext([diagnostic], formatDiagnosticsHost)
+      : ts.formatDiagnostics([diagnostic], formatDiagnosticsHost);
 
   const logFunc = diagnosticsLogger(log4js.getLogger(DIAGNOSTICS), diagnostic);
   if (!logFunc) { return; }