From d32cf839fabbc534fa88ca49494a051a76b9c73d Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Mon, 6 Jun 2022 20:24:20 -0400 Subject: [PATCH] refactor: simplify hosts to directly assign tsModule.sys where possible (#349) - no need to duplicate types this way, which can and have changed over time -- it's always the same typings this way - also reorganize `host.ts` to have similar categories of functions near each other, instead of a mix of functions wherever - similar to how I organized the tests for `host` as well - shrink the code a bit this way too - add a comment about `getDefaultLibFileName`'s confusing naming pointing to the TS issues about how this is an old mistake but changing it now would be breaking - this is also how the TS Wiki recommends setting up hosts: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#incremental-build-support-using-the-language-services - NOTE: because of how `tsproxy` works to support alternate TS implementations, this does require that `tsModule` _exists_ at the time of instantiation, i.e. that `setTypescriptModule` has already been called - for `host.ts`, `LanguageServiceHost` is only instantiated after `setTypescriptModule`, but for `diagnostics-format-host.ts`, it is immediately instantiated (at the bottom of the file), hence why `getCurrentDirectory` can't just be assigned to `tsModule.sys` - there is a way to fix this, but the refactoring is more complex as it would require creating in `index.ts` and then passing it as an argument -- would want to refactor more at that point too, so leaving that out for now in this otherwise small, isolated refactor - for a different, but related reason, the `host.trace` tests have to mock `console` instead of just `console.log`, since `trace` would just be set to the old, unmocked `console.log` otherwise - as it's assigned directly to `console.log` now --- __tests__/host.spec.ts | 6 ++- src/diagnostics-format-host.ts | 11 +---- src/host.ts | 82 +++++++--------------------------- 3 files changed, 24 insertions(+), 75 deletions(-) diff --git a/__tests__/host.spec.ts b/__tests__/host.spec.ts index 7248f034..8d16ef6f 100644 --- a/__tests__/host.spec.ts +++ b/__tests__/host.spec.ts @@ -9,6 +9,11 @@ import { LanguageServiceHost } from "../src/host"; setTypescriptModule(ts); +// mock for host.trace +(global as any).console = { + log: jest.fn(), +}; + const defaultConfig = { fileNames: [], errors: [], options: {} }; const unaryFunc = "const unary = (x: string): string => x.reverse()"; @@ -72,7 +77,6 @@ test("LanguageServiceHost", async () => { expect(host.getTypeRootsVersion()).toEqual(0); // mock out trace - console.log = jest.fn(); host.trace('test log'); expect(console.log).toHaveBeenCalledWith('test log'); }); diff --git a/src/diagnostics-format-host.ts b/src/diagnostics-format-host.ts index c8d39bbd..1e56bf39 100644 --- a/src/diagnostics-format-host.ts +++ b/src/diagnostics-format-host.ts @@ -10,15 +10,8 @@ export class FormatHost implements tsTypes.FormatDiagnosticsHost return tsModule.sys.getCurrentDirectory(); } - public getCanonicalFileName(fileName: string): string - { - return path.normalize(fileName); - } - - public getNewLine(): string - { - return tsModule.sys.newLine; - } + public getCanonicalFileName = path.normalize; + public getNewLine = () => tsModule.sys.newLine; } export const formatHost = new FormatHost(); diff --git a/src/host.ts b/src/host.ts index 484959d0..8df432d4 100644 --- a/src/host.ts +++ b/src/host.ts @@ -6,16 +6,14 @@ import { TransformerFactoryCreator } from "./ioptions"; export class LanguageServiceHost implements tsTypes.LanguageServiceHost { - private cwd: string; private snapshots: { [fileName: string]: tsTypes.IScriptSnapshot } = {}; private versions: { [fileName: string]: number } = {}; private service?: tsTypes.LanguageService; private fileNames: Set; - constructor(private parsedConfig: tsTypes.ParsedCommandLine, private transformers: TransformerFactoryCreator[], cwd: string) + constructor(private parsedConfig: tsTypes.ParsedCommandLine, private transformers: TransformerFactoryCreator[], private cwd: string) { this.fileNames = new Set(parsedConfig.fileNames); - this.cwd = cwd; } public reset() @@ -58,10 +56,7 @@ export class LanguageServiceHost implements tsTypes.LanguageServiceHost return undefined; } - public getCurrentDirectory() - { - return this.cwd; - } + public getScriptFileNames = () => Array.from(this.fileNames.values()); public getScriptVersion(fileName: string) { @@ -70,61 +65,6 @@ export class LanguageServiceHost implements tsTypes.LanguageServiceHost return (this.versions[fileName] || 0).toString(); } - public getScriptFileNames() - { - return Array.from(this.fileNames.values()); - } - - public getCompilationSettings(): tsTypes.CompilerOptions - { - return this.parsedConfig.options; - } - - public getDefaultLibFileName(opts: tsTypes.CompilerOptions) - { - return tsModule.getDefaultLibFilePath(opts); - } - - public useCaseSensitiveFileNames(): boolean - { - return tsModule.sys.useCaseSensitiveFileNames; - } - - public readDirectory(path: string, extensions?: string[], exclude?: string[], include?: string[]): string[] - { - return tsModule.sys.readDirectory(path, extensions, exclude, include); - } - - public readFile(path: string, encoding?: string): string | undefined - { - return tsModule.sys.readFile(path, encoding); - } - - public fileExists(path: string): boolean - { - return tsModule.sys.fileExists(path); - } - - public realpath(path: string): string - { - return tsModule.sys.realpath!(path); // this exists in the default implementation: https://github.com/microsoft/TypeScript/blob/ab2523bbe0352d4486f67b73473d2143ad64d03d/src/compiler/sys.ts#L1288 - } - - public getTypeRootsVersion(): number - { - return 0; - } - - public directoryExists(directoryName: string): boolean - { - return tsModule.sys.directoryExists(directoryName); - } - - public getDirectories(directoryName: string): string[] - { - return tsModule.sys.getDirectories(directoryName); - } - public getCustomTransformers(): tsTypes.CustomTransformers | undefined { if (this.service === undefined || this.transformers === undefined || this.transformers.length === 0) @@ -151,7 +91,19 @@ export class LanguageServiceHost implements tsTypes.LanguageServiceHost return transformer; } - public trace(line: string) { - console.log(line) - } + public getCompilationSettings = () => this.parsedConfig.options; + public getTypeRootsVersion = () => 0; + public getCurrentDirectory = () => this.cwd; + + public useCaseSensitiveFileNames = () => tsModule.sys.useCaseSensitiveFileNames; + public getDefaultLibFileName = tsModule.getDefaultLibFilePath; // confusing naming: https://github.com/microsoft/TypeScript/issues/35318 + + public readDirectory = tsModule.sys.readDirectory; + public readFile = tsModule.sys.readFile; + public fileExists = tsModule.sys.fileExists; + public directoryExists = tsModule.sys.directoryExists; + public getDirectories = tsModule.sys.getDirectories; + public realpath = tsModule.sys.realpath!; // this exists in the default implementation: https://github.com/microsoft/TypeScript/blob/ab2523bbe0352d4486f67b73473d2143ad64d03d/src/compiler/sys.ts#L1288 + + public trace = console.log; }