Skip to content

Commit

Permalink
fix: typesVersions not respecting outDir from tsconfig file (#1238)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mrgrain authored Aug 13, 2024
1 parent f5073a5 commit 072239b
Show file tree
Hide file tree
Showing 5 changed files with 255 additions and 12 deletions.
4 changes: 3 additions & 1 deletion src/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down
13 changes: 11 additions & 2 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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: {
Expand All @@ -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))
Expand Down
20 changes: 11 additions & 9 deletions src/downlevel-dts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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}`>();
Expand All @@ -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);
Expand Down Expand Up @@ -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 });
}
}
Expand All @@ -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`] };
}
Expand Down
15 changes: 15 additions & 0 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
215 changes: 215 additions & 0 deletions test/downlevel.test.ts
Original file line number Diff line number Diff line change
@@ -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')],
};
}

0 comments on commit 072239b

Please sign in to comment.