From 072239ba0a36416893f73e6726daf4c05b29a93e Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Tue, 13 Aug 2024 16:35:08 +0100 Subject: [PATCH] fix: typesVersions not respecting outDir from tsconfig file (#1238) Fixes #1234 Also fixes an issue related the `.type-compat` output path that was discovered when adding tests. When getting `outdir` from the program, this might be either a relative or absolute path. Previously the code assumed it was always relative which is not correct. This led to absolute paths being used for the `.type-compat` dir and in the post processing; ultimately leading to a compilation failure. The fix is to ensure `outdir` & `.type-compat` are normalized to a relative path. --- 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/assembler.ts | 4 +- src/compiler.ts | 13 ++- src/downlevel-dts.ts | 20 ++-- src/helpers.ts | 15 +++ test/downlevel.test.ts | 215 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 255 insertions(+), 12 deletions(-) create mode 100644 test/downlevel.test.ts diff --git a/src/assembler.ts b/src/assembler.ts index 3a3b16e6..6402d91d 100644 --- a/src/assembler.ts +++ b/src/assembler.ts @@ -13,6 +13,7 @@ import { symbolIdentifier } from './common/symbol-id'; import { Directives } from './directives'; import { getReferencedDocParams, parseSymbolDocumentation, TypeSystemHints } from './docs'; import { Emitter } from './emitter'; +import { normalizeConfigPath } from './helpers'; import { JsiiDiagnostic } from './jsii-diagnostic'; import * as literate from './literate'; import * as bindings from './node-bindings'; @@ -102,7 +103,8 @@ export class Assembler implements Emitter { // If out-of-source build was configured (tsc's outDir and rootDir), the // main file's path needs to be re-rooted from the outDir into the rootDir. - const tscOutDir = program.getCompilerOptions().outDir; + // outDir might be also be absolute, so we need to normalize it. + const tscOutDir = normalizeConfigPath(projectInfo.projectRoot, program.getCompilerOptions().outDir); if (tscOutDir != null) { mainFile = path.relative(tscOutDir, mainFile); diff --git a/src/compiler.ts b/src/compiler.ts index ec5921cd..38a7d529 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -8,6 +8,7 @@ import { Assembler } from './assembler'; import { findDependencyDirectory } from './common/find-utils'; import { emitDownleveledDeclarations, TYPES_COMPAT } from './downlevel-dts'; import { Emitter } from './emitter'; +import { normalizeConfigPath } from './helpers'; import { JsiiDiagnostic } from './jsii-diagnostic'; import { ProjectInfo } from './project-info'; import { WARNINGSCODE_FILE_NAME } from './transforms/deprecation-warnings'; @@ -314,7 +315,12 @@ export class Compiler implements Emitter { } if (!hasErrors) { - emitDownleveledDeclarations(this.options.projectInfo); + emitDownleveledDeclarations( + this.projectRoot, + this.options.projectInfo.packageJson, + // outDir might be absolute. Need to normalize it. + normalizeConfigPath(this.projectRoot, this.tsconfig.compilerOptions.outDir), + ); } // Some extra validation on the config. @@ -359,6 +365,9 @@ export class Compiler implements Emitter { } const pi = this.options.projectInfo; + const configDir = path.dirname(this.configPath); + const absoluteTypesCompat = path.resolve(configDir, pi.tsc?.outDir ?? '.', TYPES_COMPAT); + const relativeTypesCompat = path.relative(configDir, absoluteTypesCompat); return { compilerOptions: { @@ -372,7 +381,7 @@ export class Compiler implements Emitter { include: [pi.tsc?.rootDir != null ? path.join(pi.tsc.rootDir, '**', '*.ts') : path.join('**', '*.ts')], exclude: [ 'node_modules', - pi.tsc?.outDir != null ? path.resolve(pi.tsc.outDir, TYPES_COMPAT) : TYPES_COMPAT, + relativeTypesCompat, ...(pi.excludeTypescript ?? []), ...(pi.tsc?.outDir != null && (pi.tsc?.rootDir == null || path.resolve(pi.tsc.outDir).startsWith(path.resolve(pi.tsc.rootDir) + path.sep)) diff --git a/src/downlevel-dts.ts b/src/downlevel-dts.ts index 929604ba..ef020b00 100644 --- a/src/downlevel-dts.ts +++ b/src/downlevel-dts.ts @@ -15,7 +15,7 @@ import { main as downlevel } from 'downlevel-dts'; import * as log4js from 'log4js'; import { SemVer } from 'semver'; import * as ts from 'typescript'; -import type { PackageJson, ProjectInfo } from './project-info'; +import type { PackageJson } from './project-info'; import type { Mutable } from './utils'; export const TYPES_COMPAT = '.types-compat'; @@ -39,14 +39,14 @@ const DOWNLEVEL_BREAKPOINTS: readonly SemVer[] = ['3.9'].map((ver) => new SemVer /** * Produces down-leveled declaration files to ensure compatibility with previous - * compiler releases (macthing TypeScript's `major.minor` versioning scheme). + * compiler releases (matching TypeScript's `major.minor` versioning scheme). * This is necessary in order to ensure a package change compiler release lines * does not force all it's consumers to do the same (and vice-versa). * * @returns the `typesVersions` object that should be recorded in `package.json` */ -export function emitDownleveledDeclarations({ packageJson, projectRoot, tsc }: ProjectInfo) { - const compatRoot = join(projectRoot, ...(tsc?.outDir != null ? [tsc?.outDir] : []), TYPES_COMPAT); +export function emitDownleveledDeclarations(projectRoot: string, packageJson: PackageJson, outDir?: string) { + const compatRoot = join(projectRoot, ...(outDir != null ? [outDir] : []), TYPES_COMPAT); rmSync(compatRoot, { force: true, recursive: true }); const rewrites = new Set<`${number}.${number}`>(); @@ -65,8 +65,8 @@ export function emitDownleveledDeclarations({ packageJson, projectRoot, tsc }: P const workdir = mkdtempSync(join(tmpdir(), `downlevel-dts-${breakpoint}-${basename(projectRoot)}-`)); try { downlevel(projectRoot, workdir, breakpoint.version); - const projectOutDir = tsc?.outDir != null ? join(projectRoot, tsc.outDir) : projectRoot; - const workOutDir = tsc?.outDir != null ? join(workdir, tsc.outDir) : workdir; + const projectOutDir = outDir != null ? join(projectRoot, outDir) : projectRoot; + const workOutDir = outDir != null ? join(workdir, outDir) : workdir; for (const dts of walkDirectory(workOutDir)) { const original = readFileSync(join(projectOutDir, dts), 'utf-8'); const downleveledPath = join(workOutDir, dts); @@ -106,8 +106,10 @@ export function emitDownleveledDeclarations({ packageJson, projectRoot, tsc }: P copyFileSync(downleveledPath, rewritten); } } + } catch (error) { + LOG.error(error); } finally { - // Clean up after outselves... + // Clean up after ourselves... rmSync(workdir, { force: true, recursive: true }); } } @@ -117,8 +119,8 @@ export function emitDownleveledDeclarations({ packageJson, projectRoot, tsc }: P for (const version of rewrites) { // Register the type redirect in the typesVersions configuration typesVersions ??= {}; - const from = [...(tsc?.outDir != null ? [tsc?.outDir] : []), '*'].join('/'); - const to = [...(tsc?.outDir != null ? [tsc?.outDir] : []), TYPES_COMPAT, `ts${version}`, '*'].join('/'); + const from = [...(outDir != null ? [outDir] : []), '*'].join('/'); + const to = [...(outDir != null ? [outDir] : []), TYPES_COMPAT, `ts${version}`, '*'].join('/'); // We put 2 candidate redirects (first match wins), so that it works for nested imports, too (see: https://github.com/microsoft/TypeScript/issues/43133) typesVersions[`<=${version}`] = { [from]: [to, `${to}/index.d.ts`] }; } diff --git a/src/helpers.ts b/src/helpers.ts index 26de7091..84ebf226 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -326,3 +326,18 @@ export class TestWorkspace { // Alias for backwards compatibility export type PackageInfo = PackageJson; + +/** + * TSConfig paths can either be relative to the project or absolute. + * This function normalizes paths to be relative to the provided root. + * After normalization, code using these paths can be much simpler. + * + * @param root the project root + * @param pathToNormalize the path to normalize, might be empty + */ +export function normalizeConfigPath(root: string, pathToNormalize?: string): string | undefined { + if (pathToNormalize == null || !path.isAbsolute(pathToNormalize)) { + return pathToNormalize; + } + return path.relative(root, pathToNormalize); +} diff --git a/test/downlevel.test.ts b/test/downlevel.test.ts new file mode 100644 index 00000000..81e0c373 --- /dev/null +++ b/test/downlevel.test.ts @@ -0,0 +1,215 @@ +import { mkdtempSync, writeFileSync, readFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Compiler } from '../src/compiler'; +import { TYPES_COMPAT } from '../src/downlevel-dts'; +import { PackageJson, ProjectInfo } from '../src/project-info'; + +describe('Compiler', () => { + describe('generated tsconfig', () => { + test('will downlevel code', () => { + // GIVEN + const sourceDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-generated-')); + _writeDownlevelableCode(sourceDir); + + // WHEN + const compiler = new Compiler({ + projectInfo: { + ..._makeProjectInfo(sourceDir, 'index.d.ts'), + }, + }); + const result = compiler.emit(); + + // THEN code compiles + expect(result.diagnostics).toHaveLength(0); + expect(result.emitSkipped).toBe(false); + // THEN code is downleveled + const downleveled = readFileSync(join(sourceDir, '.types-compat/ts3.9/index.d.ts'), 'utf-8'); + expect(downleveled).toMatchInlineSnapshot(` + "declare class MarkerA { + } + export type { MarkerA }; + " + `); + // THEN typeVersions are written + const packageJson = readPackageJson(sourceDir); + expect(packageJson.typesVersions).toMatchObject({ + '<=3.9': { + '*': ['.types-compat/ts3.9/*', '.types-compat/ts3.9/*/index.d.ts'], + }, + }); + // THEN + const tsconfig = JSON.parse(readFileSync(join(sourceDir, 'tsconfig.json'), 'utf-8')); + expect(tsconfig.exclude).toMatchInlineSnapshot(` + [ + "node_modules", + ".types-compat", + ] + `); + }); + + test('will downlevel code with outdir', () => { + // GIVEN + const outDir = 'jsii-outdir'; + const srcDir = 'jsii-srcdir'; + const projectDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-generated-')); + mkdirSync(join(projectDir, srcDir), { recursive: true }); + _writeDownlevelableCode(projectDir, srcDir); + + // WHEN + const compiler = new Compiler({ + projectInfo: { + ..._makeProjectInfo(projectDir, join(outDir, 'index.d.ts')), + tsc: { + outDir, + rootDir: srcDir, + }, + }, + }); + const result = compiler.emit(); + + // THEN code compiles + expect(result.diagnostics).toHaveLength(0); + expect(result.emitSkipped).toBe(false); + // THEN code is downleveled + const downleveled = readFileSync(join(projectDir, outDir, '.types-compat/ts3.9/index.d.ts'), 'utf-8'); + expect(downleveled).toMatchInlineSnapshot(` + "declare class MarkerA { + } + export type { MarkerA }; + " + `); + // THEN typeVersions are written + const packageJson = readPackageJson(projectDir); + expect(packageJson.typesVersions).toMatchObject({ + '<=3.9': { + 'jsii-outdir/*': ['jsii-outdir/.types-compat/ts3.9/*', 'jsii-outdir/.types-compat/ts3.9/*/index.d.ts'], + }, + }); + // THEN + const tsconfig = JSON.parse(readFileSync(join(projectDir, 'tsconfig.json'), 'utf-8')); + expect(tsconfig.exclude).toMatchInlineSnapshot(` + [ + "node_modules", + "jsii-outdir/.types-compat", + ] + `); + }); + }); + + describe('user-provided tsconfig', () => { + test('will downlevel code with outdir', () => { + // GIVEN + const outDir = 'jsii-outdir'; + const srcDir = 'jsii-srcdir'; + const projectDir = mkdtempSync(join(tmpdir(), 'jsii-compiler-user-tsconfig-')); + mkdirSync(join(projectDir, srcDir), { recursive: true }); + const tsconfigPath = 'tsconfig.dev.json'; + writeFileSync( + join(projectDir, tsconfigPath), + JSON.stringify( + tsconfigForNode18Strict({ + outDir, + rootDir: srcDir, + }), + null, + 2, + ), + ); + _writeDownlevelableCode(projectDir, srcDir); + + // WHEN + const compiler = new Compiler({ + projectInfo: _makeProjectInfo(projectDir, join(outDir, 'index.d.ts')), + typeScriptConfig: tsconfigPath, + }); + const result = compiler.emit(); + + // THEN code compiles + expect(result.diagnostics).toHaveLength(0); + expect(result.emitSkipped).toBe(false); + // THEN code is downleveled + const downleveled = readFileSync(join(projectDir, outDir, '.types-compat/ts3.9/index.d.ts'), 'utf-8'); + expect(downleveled).toMatchInlineSnapshot(` + "declare class MarkerA { + } + export type { MarkerA }; + " + `); + // THEN typeVersions are written + const packageJson = readPackageJson(projectDir); + expect(packageJson.typesVersions).toMatchObject({ + '<=3.9': { + 'jsii-outdir/*': ['jsii-outdir/.types-compat/ts3.9/*', 'jsii-outdir/.types-compat/ts3.9/*/index.d.ts'], + }, + }); + }); + }); +}); + +function readPackageJson(dir: string): PackageJson { + return JSON.parse(readFileSync(join(dir, 'package.json'), 'utf-8')); +} + +function _writeDownlevelableCode(projectDir: string, codeSubDir?: string) { + // Files in the project dir + writeFileSync(join(projectDir, 'README.md'), '# Test Package'); + writeFileSync(join(projectDir, 'package.json'), JSON.stringify({}, null, 2)); + + // Files in the code dir, e.g. `src` + const codeDir = codeSubDir ? join(projectDir, codeSubDir) : projectDir; + // See https://www.npmjs.com/package/downlevel-dts#type-modifiers-on-importexport-names-45 + writeFileSync(join(codeDir, 'index.ts'), 'class MarkerA {} export { type MarkerA }'); +} + +function _makeProjectInfo(sourceDir: string, types: string): ProjectInfo { + return { + projectRoot: sourceDir, + description: 'test', + homepage: 'https://github.com/aws/jsii-compiler', + packageJson: {}, + types, + main: types.replace(/(?:\.d)?\.ts(x?)/, '.js$1'), + name: 'jsii', // That's what package.json would tell if we look up... + version: '0.0.1', + jsiiVersionFormat: 'short', + license: 'Apache-2.0', + author: { name: 'John Doe', roles: ['author'] }, + repository: { type: 'git', url: 'https://github.com/aws/jsii.git' }, + dependencies: {}, + peerDependencies: {}, + dependencyClosure: [], + bundleDependencies: {}, + targets: {}, + excludeTypescript: [], + tsc: { + // NOTE: these are the default values jsii uses when none are provided in package.json. + inlineSourceMap: true, + inlineSources: true, + }, + }; +} + +/** + * An example of a user-provided config, based on the popular tsconfig/bases project & adjusted for the strict rule set + * @see https://github.com/tsconfig/bases/blob/main/bases/node18.json + */ +function tsconfigForNode18Strict(compilerOptions: any = {}) { + return { + compilerOptions: { + lib: ['es2022'], + module: 'node16', + target: 'es2022', + + strict: true, + esModuleInterop: true, + skipLibCheck: true, + noEmitOnError: true, + moduleResolution: 'node16', + declaration: true, + ...compilerOptions, + }, + exclude: ['node_modules', TYPES_COMPAT], + include: [join('**', '*.ts')], + }; +}