From e9a8f9f705fdd79697602bd4e132369d5bf3bdde Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 26 Sep 2020 22:14:10 +0200 Subject: [PATCH] fix(compiler-cli): enable @types discovery in incremental rebuilds (#39011) Prior to this fix, incremental rebuilds could fail to type check due to missing ambient types from auto-discovered declaration files in @types directories, or type roots in general. This was caused by the intermediary `ts.Program` that is created for template type checking, for which a `ts.CompilerHost` was used which did not implement the optional `directoryExists` methods. As a result, auto-discovery of types would not be working correctly, and this would retain into the `ts.Program` that would be created for an incremental rebuild. This commit fixes the issue by forcing the custom `ts.CompilerHost` used for type checking to properly delegate into the original `ts.CompilerHost`, even for optional methods. This is accomplished using a base class `DelegatingCompilerHost` which is typed in such a way that newly introduced `ts.CompilerHost` methods must be accounted for. Fixes #38979 PR Close #39011 --- .../src/ngtsc/typecheck/src/host.ts | 96 ++++++++++--------- .../test/ngtsc/incremental_spec.ts | 21 ++++ 2 files changed, 73 insertions(+), 44 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts index 57a74db930..d7283890c1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts @@ -10,11 +10,59 @@ import * as ts from 'typescript'; import {copyFileShimData, ShimReferenceTagger} from '../../shims'; +/** + * Represents the `ts.CompilerHost` interface, with a transformation applied that turns all + * methods (even optional ones) into required fields (which may be `undefined`, if the method was + * optional). + */ +export type RequiredCompilerHostDelegations = { + [M in keyof Required]: ts.CompilerHost[M]; +}; + +/** + * Delegates all methods of `ts.CompilerHost` to a delegate, with the exception of + * `getSourceFile`, `fileExists` and `writeFile` which are implemented in `TypeCheckProgramHost`. + * + * If a new method is added to `ts.CompilerHost` which is not delegated, a type error will be + * generated for this class. + */ +export class DelegatingCompilerHost implements + Omit { + constructor(protected delegate: ts.CompilerHost) {} + + private delegateMethod(name: M): ts.CompilerHost[M] { + return this.delegate[name] !== undefined ? (this.delegate[name] as any).bind(this.delegate) : + undefined; + } + + // Excluded are 'getSourceFile', 'fileExists' and 'writeFile', which are actually implemented by + // `TypeCheckProgramHost` below. + createHash = this.delegateMethod('createHash'); + directoryExists = this.delegateMethod('directoryExists'); + getCancellationToken = this.delegateMethod('getCancellationToken'); + getCanonicalFileName = this.delegateMethod('getCanonicalFileName'); + getCurrentDirectory = this.delegateMethod('getCurrentDirectory'); + getDefaultLibFileName = this.delegateMethod('getDefaultLibFileName'); + getDefaultLibLocation = this.delegateMethod('getDefaultLibLocation'); + getDirectories = this.delegateMethod('getDirectories'); + getEnvironmentVariable = this.delegateMethod('getEnvironmentVariable'); + getNewLine = this.delegateMethod('getNewLine'); + getParsedCommandLine = this.delegateMethod('getParsedCommandLine'); + getSourceFileByPath = this.delegateMethod('getSourceFileByPath'); + readDirectory = this.delegateMethod('readDirectory'); + readFile = this.delegateMethod('readFile'); + realpath = this.delegateMethod('realpath'); + resolveModuleNames = this.delegateMethod('resolveModuleNames'); + resolveTypeReferenceDirectives = this.delegateMethod('resolveTypeReferenceDirectives'); + trace = this.delegateMethod('trace'); + useCaseSensitiveFileNames = this.delegateMethod('useCaseSensitiveFileNames'); +} + /** * A `ts.CompilerHost` which augments source files with type checking code from a * `TypeCheckContext`. */ -export class TypeCheckProgramHost implements ts.CompilerHost { +export class TypeCheckProgramHost extends DelegatingCompilerHost { /** * Map of source file names to `ts.SourceFile` instances. */ @@ -32,20 +80,11 @@ export class TypeCheckProgramHost implements ts.CompilerHost { */ private shimTagger = new ShimReferenceTagger(this.shimExtensionPrefixes); - readonly resolveModuleNames?: ts.CompilerHost['resolveModuleNames']; - constructor( sfMap: Map, private originalProgram: ts.Program, - private delegate: ts.CompilerHost, private shimExtensionPrefixes: string[]) { + delegate: ts.CompilerHost, private shimExtensionPrefixes: string[]) { + super(delegate); this.sfMap = sfMap; - - if (delegate.getDirectories !== undefined) { - this.getDirectories = (path: string) => delegate.getDirectories!(path); - } - - if (delegate.resolveModuleNames !== undefined) { - this.resolveModuleNames = delegate.resolveModuleNames; - } } getSourceFile( @@ -88,42 +127,11 @@ export class TypeCheckProgramHost implements ts.CompilerHost { this.shimTagger.finalize(); } - // The rest of the methods simply delegate to the underlying `ts.CompilerHost`. - - getDefaultLibFileName(options: ts.CompilerOptions): string { - return this.delegate.getDefaultLibFileName(options); - } - - writeFile( - fileName: string, data: string, writeByteOrderMark: boolean, - onError: ((message: string) => void)|undefined, - sourceFiles: ReadonlyArray|undefined): void { + writeFile(): never { throw new Error(`TypeCheckProgramHost should never write files`); } - getCurrentDirectory(): string { - return this.delegate.getCurrentDirectory(); - } - - getDirectories?: (path: string) => string[]; - - getCanonicalFileName(fileName: string): string { - return this.delegate.getCanonicalFileName(fileName); - } - - useCaseSensitiveFileNames(): boolean { - return this.delegate.useCaseSensitiveFileNames(); - } - - getNewLine(): string { - return this.delegate.getNewLine(); - } - fileExists(fileName: string): boolean { return this.sfMap.has(fileName) || this.delegate.fileExists(fileName); } - - readFile(fileName: string): string|undefined { - return this.delegate.readFile(fileName); - } } diff --git a/packages/compiler-cli/test/ngtsc/incremental_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_spec.ts index ee637eecd7..a7fd2024f9 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_spec.ts +++ b/packages/compiler-cli/test/ngtsc/incremental_spec.ts @@ -442,6 +442,27 @@ runInEachFileSystem(() => { // https://github.com/angular/angular/issues/30079), this would have crashed. }); + // https://github.com/angular/angular/issues/38979 + it('should retain ambient types provided by auto-discovered @types', () => { + // This test verifies that ambient types declared in node_modules/@types are still available + // in incremental compilations. In the below code, the usage of `require` should be valid + // in the original program and the incremental program. + env.tsconfig({fullTemplateTypeCheck: true}); + env.write('node_modules/@types/node/index.d.ts', 'declare var require: any;'); + env.write('main.ts', ` + import {Component} from '@angular/core'; + + require('path'); + + @Component({template: ''}) + export class MyComponent {} + `); + env.driveMain(); + env.invalidateCachedFile('main.ts'); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + // https://github.com/angular/angular/pull/26036 it('should handle redirected source files', () => { env.tsconfig({fullTemplateTypeCheck: true});