From 26a3d968fb50f564fc34cf468dff44734fe69957 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Fri, 13 Dec 2024 12:01:16 +0100 Subject: [PATCH] fix: `lib` setting from custom config is ignored (#1576) When a user provides a custom `tsconfig.json` file, they have the ability to override the `lib` setting, which provides a number of libraries that should be loaded at compile time. This config setting is not passed directly into the TypeScript compiler. Instead, it is resolved and translated into a list of `.d.ts` file that are passed as `rootNames` to the compiler, along with the actual source files. In that resolution, we failed to look at the `lib` setting the user provided, and instead always used the default config. Fix by looking at the user-provided config, falling back to the default config only if the user config doesn't have a `lib` setting (mirroring the behavior of tsc). Fixes aws/jsii#4706. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- src/compiler.ts | 20 +++++++------ test/compiler.test.ts | 68 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index 723dff69..f51e21b9 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -253,7 +253,7 @@ export class Compiler implements Emitter { const tsconf = this.tsconfig!; const prog = ts.createIncrementalProgram({ - rootNames: this.rootFiles.concat(_pathOfLibraries(this.compilerHost)), + rootNames: this.rootFiles.concat(_pathOfLibraries(tsconf.compilerOptions, this.compilerHost)), options: tsconf.compilerOptions, // Make the references absolute for the compiler projectReferences: tsconf.references?.map((ref) => ({ @@ -585,17 +585,19 @@ export interface NonBlockingWatchOptions { readonly compilationComplete: (emitResult: ts.EmitResult) => void; } -function _pathOfLibraries(host: ts.CompilerHost | ts.WatchCompilerHost): string[] { - if (!BASE_COMPILER_OPTIONS.lib || BASE_COMPILER_OPTIONS.lib.length === 0) { +function _pathOfLibraries(options: ts.CompilerOptions, host: ts.CompilerHost | ts.WatchCompilerHost): string[] { + // Prefer user libraries, falling back to a library based on the target if not supplied by the user. + // This matches tsc behavior. + const libs = options.lib ?? [ts.getDefaultLibFileName(options)] ?? []; + if (libs.length === 0) { return []; } - const lib = host.getDefaultLibLocation?.(); - if (!lib) { - throw new Error( - `Compiler host doesn't have a default library directory available for ${BASE_COMPILER_OPTIONS.lib.join(', ')}`, - ); + + const libDir = host.getDefaultLibLocation?.(); + if (!libDir) { + throw new Error(`Compiler host doesn't have a default library directory available for ${libs.join(', ')}`); } - return BASE_COMPILER_OPTIONS.lib.map((name) => path.join(lib, name)); + return libs.map((name) => path.join(libDir, name)); } function parseConfigHostFromCompilerHost(host: ts.CompilerHost): ts.ParseConfigHost { diff --git a/test/compiler.test.ts b/test/compiler.test.ts index d257a28a..a21eb281 100644 --- a/test/compiler.test.ts +++ b/test/compiler.test.ts @@ -2,12 +2,21 @@ import { mkdirSync, existsSync, mkdtempSync, rmSync, writeFileSync, readFileSync import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { loadAssemblyFromPath, SPEC_FILE_NAME, SPEC_FILE_NAME_COMPRESSED } from '@jsii/spec'; +import * as ts from 'typescript'; import { compile, Lock } from './fixtures'; import { Compiler } from '../src/compiler'; import { ProjectInfo } from '../src/project-info'; import { TypeScriptConfigValidationRuleSet } from '../src/tsconfig'; import { TypeScriptConfigValidator } from '../src/tsconfig/tsconfig-validator'; +// This is necessary to be able to jest.spyOn to functions in the 'ts' module. Replace the read-only +// object descriptors with a plain object. +jest.mock('typescript', () => ({ ...jest.requireActual('typescript') })); + +beforeEach(() => { + jest.restoreAllMocks(); +}); + describe(Compiler, () => { describe('generated tsconfig', () => { test('default is tsconfig.json', () => { @@ -226,9 +235,17 @@ describe(Compiler, () => { }); describe('user-provided tsconfig', () => { + let sourceDir: string; + const tsconfigPath = 'tsconfig.dev.json'; + beforeEach(() => { + sourceDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-user-tsconfig-')); + }); + + afterEach(() => { + rmSync(sourceDir, { force: true, recursive: true }); + }); + test('will use user-provided config', () => { - const sourceDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-user-tsconfig-')); - const tsconfigPath = 'tsconfig.dev.json'; writeFileSync(join(sourceDir, tsconfigPath), JSON.stringify(tsconfigForNode18Strict(), null, 2)); writeFileSync(join(sourceDir, 'index.ts'), 'export class MarkerA {}'); @@ -244,9 +261,7 @@ describe(Compiler, () => { }); test('use user-provided config uses include and exclude', () => { - const sourceDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-user-tsconfig-')); mkdirSync(join(sourceDir, 'sub')); - const tsconfigPath = 'tsconfig.dev.json'; writeFileSync( join(sourceDir, tsconfigPath), JSON.stringify( @@ -281,9 +296,48 @@ describe(Compiler, () => { expect(result.emitSkipped).toBe(false); }); + test('respect "lib" setting from user-provided config', () => { + const tsconfig = tsconfigForNode18Strict(); + tsconfig.compilerOptions.lib = ['Decorators.Legacy']; // Something very nonstandard + writeFileSync(join(sourceDir, tsconfigPath), JSON.stringify(tsconfig, null, 2)); + + const createIncrementalProgram = jest.spyOn(ts, 'createIncrementalProgram'); + + const compiler = new Compiler({ + projectInfo: _makeProjectInfo(sourceDir, 'index.d.ts'), + typeScriptConfig: tsconfigPath, + }); + compiler.emit(); + + expect(createIncrementalProgram).toHaveBeenCalledWith( + expect.objectContaining({ + rootNames: expect.arrayContaining([expect.stringContaining('lib.decorators.legacy.d.ts')]), + }), + ); + }); + + test('missing "lib" setting is based on compilation target', () => { + const tsconfig = tsconfigForNode18Strict(); + tsconfig.compilerOptions.target = 'es6'; + delete tsconfig.compilerOptions.lib; + writeFileSync(join(sourceDir, tsconfigPath), JSON.stringify(tsconfig, null, 2)); + + const createIncrementalProgram = jest.spyOn(ts, 'createIncrementalProgram'); + + const compiler = new Compiler({ + projectInfo: _makeProjectInfo(sourceDir, 'index.d.ts'), + typeScriptConfig: tsconfigPath, + }); + compiler.emit(); + + expect(createIncrementalProgram).toHaveBeenCalledWith( + expect.objectContaining({ + rootNames: expect.arrayContaining([expect.stringContaining('lib.es6.d.ts')]), + }), + ); + }); + test('"watch" mode', async () => { - const sourceDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-watch-mode-')); - const tsconfigPath = 'tsconfig.dev.json'; writeFileSync(join(sourceDir, tsconfigPath), JSON.stringify(tsconfigForNode18Strict(), null, 2)); try { @@ -464,7 +518,7 @@ function expectedTypeScriptConfig() { function tsconfigForNode18Strict() { return { compilerOptions: { - lib: ['es2022'], + lib: ['es2022'] as string[] | undefined, module: 'node16', target: 'es2022',