Skip to content

Commit

Permalink
chore: stop aliasing typescript in dependencies (#3623)
Browse files Browse the repository at this point in the history
Aliasing `typescript` to `typescript-3.9` results in occasionally
surprising hoisting behavior, in particular using `npm 18`. In
particular, if two distinct  versions of `typescript` are installed,
aliasing allows both to be hoisted at the top-level, which results in
ambiguity as to which of them gets to have the `node_modules/.bin/tsc`
symlink.

This PR stops aliasing `typescript` to `typescript-3.9` and does the
necessary work to ensure builds continue to operate smoothly, menaing
it replaced TypeScript configuration files for jest with plain ESM
configuration files including typed JSDoc comments (for IDE supprot to
continue working as before), as `jest` otherwise uses `ts-node` to
transform the configuration files, and  `ts-node` uses the "most local"
`typescript` library to perform this parsing (and unfortunately,
`typescript@3.9` does not support the `ES2020` target we are using).


This also required allowing `jsii.tsc.types` to be specified in the
`package.json` file of jsii projects, as otherwise the TypeScript
compiler attempts to load `@types/*` packages that are not compatible
with `typescript@3.9` (for example, `@synclair/typebox` requires
`typescript@>=4.5`). This setting is anyway generally useful in
complex monorepo situations.

---

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
RomainMuller authored Jun 28, 2022
1 parent 623c0c1 commit 6d9dda5
Show file tree
Hide file tree
Showing 81 changed files with 155 additions and 119 deletions.
5 changes: 3 additions & 2 deletions eslint-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
env:
es2020: true
jest: true
node: true

Expand All @@ -10,9 +11,9 @@ plugins:

parser: '@typescript-eslint/parser'
parserOptions:
ecmaVersion: 2018
ecmaVersion: 2020
impliedStrict: true
sourceType: module
sourceType: script
project: ./**/tsconfig.json

extends:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ are set in the `jsii.tsc` section of the `package.json` file, but use the same n
`declarationMap`), or to optimize the emitted code size (by disabling source maps entirely).
+ if any of these options is specified, the source map configuration will exactly match what is being provided here
+ If none are specified, the default settings will be used: `#!ts { inlineSourceMap: true, inlineSources: true }`
- `types` allows limiting which visible type libraries get loaded in the global scope by the typescript compiler. By
default, all visible `@types/*` packages will be loaded, which can be undesirable (in particular in monorepos, where
some type libraries are not compatible with the TypeScript compiler version that `jsii` uses). The value specified
here will be forwarded as-is to the TypeScript compiler.

Refer to the [TypeScript compiler options reference][ts-options] for more information about those options.

Expand Down
23 changes: 12 additions & 11 deletions jest.config.ts → jest.config.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Config } from '@jest/types';
import jest from '@jest/types';
import { defaults } from 'jest-config';
import { cpus } from 'os';
import { env } from 'process';
Expand All @@ -10,8 +10,10 @@ import { env } from 'process';
* be overridden (for example, the coverage threshold), then a new
* `jest.config.ts` file should be created that imports from this one and
* modifies just what needs to be modified, typically using `overriddenConfig`.
*
* @type {jest.Config.InitialOptions}
*/
const config: Config.InitialOptions = {
const config = {
...defaults,

collectCoverage: true,
Expand Down Expand Up @@ -41,32 +43,31 @@ const config: Config.InitialOptions = {
* operation works deeply on objects, but overrides that are not objects (e.g:
* arrays, strings, ...) simply replace the original value.
*
* @param overrides values to be used for overriding the orignal configuration.
* @param {jest.Config.InitialOptions} overrides values to be used for overriding the orignal configuration.
*
* @return {jest.Config.InitialOptions}
*/
export function overriddenConfig(overrides: Config.InitialOptions) {
export function overriddenConfig(overrides) {
return merge(config, overrides);

function merge<T>(original: T, override: T): T {
function merge(original, override) {
if (typeof original === 'object') {
// Arrays are objects, too!
if (Array.isArray(override)) {
return override;
}

const result: any = {};
const result = {};
const allKeys = new Set([
...Object.keys(original),
...Object.keys(override),
]);

// TypeScript appears to choke if we do the "as any" in the same
// expression as the key access, so we delcare surrogate varibales...
const originalAsAny = original as any;
const overrideAsAny = override as any;

for (const key of Array.from(allKeys).sort()) {
const originalValue: unknown = originalAsAny[key];
const overrideValue: unknown = overrideAsAny[key];
const originalValue = original[key];
const overrideValue = override[key];
if (originalValue == null) {
result[key] = overrideValue;
} else if (overrideValue == null) {
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@
],
"nohoist": [
"**/@fixtures/jsii-calc-bundled",
"**/@fixtures/jsii-calc-bundled/**"
"**/@fixtures/jsii-calc-bundled/**",
"**/typescript"
]
},
"resolutions": {
"@types/prettier": "2.6.0"
"minipass": "3.2.1"
},
"resolutions_info": {
"minipass": "https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/60901"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Compiler } from 'jsii/lib/compiler';
import { loadProjectInfo } from 'jsii/lib/project-info';
import * as path from 'node:path';
import * as process from 'node:process';
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import type { Context } from '.';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'fs-extra';
import * as path from 'node:path';
import * as process from 'node:process';
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import type { Context } from '.';

Expand Down
2 changes: 1 addition & 1 deletion packages/@jsii/benchmarks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"jsii": "^0.0.0",
"npm": "^8.12.1",
"tar": "^6.1.11",
"typescript-3.9": "npm:typescript@~3.9.10",
"typescript": "~3.9.10",
"yargs": "^16.2.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@jsii/benchmarks/scripts/snapshot-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as glob from 'glob';
import * as os from 'os';
import * as path from 'path';
import * as tar from 'tar';
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { cdkTagv2_21_1, cdkv2_21_1 } from '../lib/constants';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../../jest.config';
import { overriddenConfig } from '../../../jest.config.mjs';

export default overriddenConfig({
coveragePathIgnorePatterns: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../../jest.config';
import { overriddenConfig } from '../../../jest.config.mjs';

export default overriddenConfig({
// We don't need coverage for the integration tests
Expand Down
3 changes: 3 additions & 0 deletions packages/@jsii/kernel/jest.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import config from '../../../jest.config.mjs';

export default config;
3 changes: 0 additions & 3 deletions packages/@jsii/kernel/jest.config.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../../jest.config';
import { overriddenConfig } from '../../../jest.config.mjs';

export default overriddenConfig({
coveragePathIgnorePatterns: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../../jest.config';
import { overriddenConfig } from '../../../jest.config.mjs';

export default overriddenConfig({
coverageThreshold: {
Expand Down
3 changes: 2 additions & 1 deletion packages/@scope/jsii-calc-base-of-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
},
"tsc": {
"outDir": "./build",
"rootDir": "."
"rootDir": ".",
"types": []
},
"versionFormat": "short",
"metadata": {
Expand Down
3 changes: 3 additions & 0 deletions packages/@scope/jsii-calc-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@
"module": "scope.jsii_calc_base"
}
},
"tsc": {
"types": []
},
"versionFormat": "short"
}
}
3 changes: 2 additions & 1 deletion packages/@scope/jsii-calc-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@
},
"tsc": {
"outDir": "build",
"sourceMap": false
"sourceMap": false,
"types": []
},
"versionFormat": "short",
"metadata": {
Expand Down
3 changes: 3 additions & 0 deletions packages/codemaker/jest.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import config from '../../jest.config.mjs';

export default config;
3 changes: 0 additions & 3 deletions packages/codemaker/jest.config.ts

This file was deleted.

5 changes: 5 additions & 0 deletions packages/jsii-calc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
]
}
},
"tsc": {
"types": [
"node"
]
},
"metadata": {
"jsii:boolean": true,
"jsii:number": 1337,
Expand Down
3 changes: 3 additions & 0 deletions packages/jsii-config/jest.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import config from '../../jest.config.mjs';

export default config;
3 changes: 0 additions & 3 deletions packages/jsii-config/jest.config.ts

This file was deleted.

3 changes: 3 additions & 0 deletions packages/jsii-diff/jest.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import config from '../../jest.config.mjs';

export default config;
3 changes: 0 additions & 3 deletions packages/jsii-diff/jest.config.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../jest.config';
import { overriddenConfig } from '../../jest.config.mjs';

export default overriddenConfig({
coveragePathIgnorePatterns: ['/node_modules/', '<rootDir>/test'],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { overriddenConfig } from '../../jest.config';
import { overriddenConfig } from '../../jest.config.mjs';

export default overriddenConfig({
coverageThreshold: {
Expand Down
6 changes: 6 additions & 0 deletions packages/jsii-rosetta/jest.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createRequire } from 'node:module';
import { overriddenConfig } from '../../jest.config.mjs';

export default overriddenConfig({
setupFiles: [createRequire(import.meta.url).resolve('./jestsetup.js')],
});
7 changes: 0 additions & 7 deletions packages/jsii-rosetta/jest.config.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/fixtures.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import { createSourceFile, ScriptKind, ScriptTarget, SyntaxKind } from 'typescript-3.9';
import { createSourceFile, ScriptKind, ScriptTarget, SyntaxKind } from 'typescript';

import { TypeScriptSnippet, SnippetParameters, ApiLocation } from './snippet';

Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/jsii/jsii-types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { inferredTypeOfExpression, BuiltInType, builtInTypeName, mapElementType } from '../typescript/types';
import { hasAnyFlag, analyzeStructType, JsiiSymbol } from './jsii-utils';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/jsii/jsii-utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as spec from '@jsii/spec';
import { symbolIdentifier } from 'jsii';
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { AstRenderer } from '../renderer';
import { typeContainsUndefined } from '../typescript/types';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/csharp.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { determineJsiiType, JsiiType, ObjectLiteralStruct } from '../jsii/jsii-types';
import { JsiiSymbol, simpleName, namespaceName } from '../jsii/jsii-utils';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/default.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { analyzeObjectLiteral, ObjectLiteralStruct } from '../jsii/jsii-types';
import { isNamedLikeStruct, isJsiiProtocolType } from '../jsii/jsii-utils';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/go.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// import { JsiiSymbol, simpleName, namespaceName } from '../jsii/jsii-utils';
// import { jsiiTargetParameter } from '../jsii/packages';
import { AssertionError } from 'assert';
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { analyzeObjectLiteral, determineJsiiType, JsiiType, ObjectLiteralStruct } from '../jsii/jsii-types';
import { OTree } from '../o-tree';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/java.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { determineJsiiType, JsiiType, analyzeObjectLiteral, ObjectLiteralStruct } from '../jsii/jsii-types';
import { JsiiSymbol, simpleName, namespaceName } from '../jsii/jsii-utils';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/python.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { determineJsiiType, JsiiType, ObjectLiteralStruct } from '../jsii/jsii-types';
import {
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/record-references.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { lookupJsiiSymbol } from '../jsii/jsii-utils';
import { TargetLanguage } from '../languages/target-language';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/visualize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { OTree } from '../o-tree';
import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class ReplaceTypeScriptTransform extends ReplaceCodeTransform {
public constructor(api: ApiLocation, strict: boolean, replacer: TypeScriptReplacer) {
super((block, line) => {
const languageParts = block.language ? block.language.split(' ') : [];
if (languageParts[0] !== 'typescript-3.9' && languageParts[0] !== 'ts') {
if (languageParts[0] !== 'typescript' && languageParts[0] !== 'ts') {
return block;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/renderer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { TargetLanguage } from './languages';
import { NO_SYNTAX, OTree, UnknownSyntax, Span } from './o-tree';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/rosetta-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export class RosettaTabletReader {

const translated = this.translateSnippet(snippet, targetLang);

return translated ?? { language: 'typescript-3.9', source: example };
return translated ?? { language: 'typescript', source: example };
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/tablets/tablets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ export class TranslatedSnippet {
public get originalSource(): Translation {
return {
source: this.snippet.translations[ORIGINAL_SNIPPET_KEY].source,
language: 'typescript-3.9',
language: 'typescript',
didCompile: this.snippet.didCompile,
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/translate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';
import { inspect } from 'util';

import { TARGET_LANGUAGES, TargetLanguage } from './languages';
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/typescript/ast-utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { AstRenderer } from '../renderer';

Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/typescript/imports.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { JsiiSymbol, parentSymbol, lookupJsiiSymbolFromNode } from '../jsii/jsii-utils';
import { AstRenderer } from '../renderer';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

import { Spans } from './visible-spans';

Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/typescript/ts-compiler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ts from 'typescript-3.9';
import * as ts from 'typescript';

export class TypeScriptCompiler {
private readonly realHost = ts.createCompilerHost(STANDARD_COMPILER_OPTIONS, true);
Expand Down
Loading

0 comments on commit 6d9dda5

Please sign in to comment.