Skip to content

Commit

Permalink
feat(jsii): make source map behavior fully configurable (#3558)
Browse files Browse the repository at this point in the history
Instead of enabling declarations maps everywhere as was done in #3521,
allow customers to define their desired source-map related configuration
in the `jsii.tsc` stanza of `package.json`.

This change in stance is motivated by how introduction of declarations
map causes broad asset hash changes in consumer code, which effectively
breaks many snapshot-based regression tests, and this feature should
hence be opt-in.



---

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 committed Jun 1, 2022
1 parent c40f26c commit 06d9a39
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 27 deletions.
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Expand Up @@ -9,6 +9,7 @@ on:

env:
DOTNET_NOLOGO: true
NODE_OPTIONS: --max-old-space-size=4096

# This workflows currently has the following jobs:
# - build : Builds the source tree as-is
Expand Down
Expand Up @@ -186,8 +186,14 @@ are set in the `jsii.tsc` section of the `package.json` file, but use the same n
- `rootDir` - specifies the root directory that contains all of the `.ts` source files. This is used in conjunction with
`outDir`, to control the directory structure that gets generated.
- `forceConsistentCasingInFileNames` - if `true`, will make the `TypeScript` compiler care about the casing of files
specified in `import` statements. This is helpful if you're developing on a filesystem that is case-insensitive
specified in `import` statements. This is helpful if you're developing on a filesystem that is case-insensitive
(Mac/Win), but building/deploying on a filesystem that is case-sensitive (Linux).
- `declarationMap`, `inlineSourceMap`, `inlineSources`, and `sourceMap` allow confifuring the source map generation.
This option can be useful to finely control your local development experience (for example, by enabling
`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 }`

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

[`tsconfig.json`]: https://www.typescriptlang.org/docs/handbook/tsconfig-json.html
Expand Down
6 changes: 3 additions & 3 deletions packages/@jsii/check-node/src/index.test.ts
Expand Up @@ -10,10 +10,10 @@ test('tested node releases are correctly registered & supported', () => {
});

// This test is there to ensure house-keeping happens when it should. If we are
// testing a given node release, it must not have been EOL for over 30 days.
test('tested node release have not ben EOL for more than 30 days', () => {
// testing a given node release, it must not have been EOL for over 60 days.
test('tested node release have not ben EOL for more than 60 days', () => {
const { nodeRelease } = NodeRelease.forThisRuntime();
expect(nodeRelease?.endOfLifeDate?.getTime()).toBeGreaterThan(
Date.now() - 30 * 86_400_000,
Date.now() - 60 * 86_400_000,
);
});
3 changes: 2 additions & 1 deletion packages/@scope/jsii-calc-lib/package.json
Expand Up @@ -69,7 +69,8 @@
}
},
"tsc": {
"outDir": "build"
"outDir": "build",
"sourceMap": false
},
"versionFormat": "short",
"metadata": {
Expand Down
3 changes: 0 additions & 3 deletions packages/jsii/lib/compiler.ts
Expand Up @@ -16,11 +16,8 @@ const BASE_COMPILER_OPTIONS: ts.CompilerOptions = {
alwaysStrict: true,
charset: 'utf8',
declaration: true,
declarationMap: true,
experimentalDecorators: true,
incremental: true,
inlineSourceMap: true,
inlineSources: true,
lib: ['lib.es2019.d.ts'],
module: ts.ModuleKind.CommonJS,
newLine: ts.NewLineKind.LineFeed,
Expand Down
51 changes: 46 additions & 5 deletions packages/jsii/lib/project-info.ts
Expand Up @@ -13,11 +13,21 @@ const spdx: Set<string> = require('spdx-license-list/simple');

const LOG = log4js.getLogger('jsii/package-info');

export interface TSCompilerOptions {
readonly outDir?: string;
readonly rootDir?: string;
readonly forceConsistentCasingInFileNames?: boolean;
}
export type TSCompilerOptions = Partial<
Pick<
ts.CompilerOptions,
// Directory preferences
| 'outDir'
| 'rootDir'
// Style preferences
| 'forceConsistentCasingInFileNames'
// Source map preferences
| 'declarationMap'
| 'inlineSourceMap'
| 'inlineSources'
| 'sourceMap'
>
>;

export interface ProjectInfo {
readonly projectRoot: string;
Expand Down Expand Up @@ -207,6 +217,9 @@ export function loadProjectInfo(projectRoot: string): ProjectInfoResult {
tsc: {
outDir: pkg.jsii?.tsc?.outDir,
rootDir: pkg.jsii?.tsc?.rootDir,
forceConsistentCasingInFileNames:
pkg.jsii?.tsc?.forceConsistentCasingInFileNames,
..._sourceMapPreferences(pkg.jsii?.tsc),
},
bin: pkg.bin,
exports: pkg.exports,
Expand All @@ -228,6 +241,34 @@ function _guessRepositoryType(url: string): string {
);
}

function _sourceMapPreferences({
declarationMap,
inlineSourceMap,
inlineSources,
sourceMap,
}: TSCompilerOptions = {}) {
// If none of the options are specified, use the default configuration from jsii <= 1.58.0, which
// means inline source maps with embedded source information.
if (
declarationMap == null &&
inlineSourceMap == null &&
inlineSources == null &&
sourceMap == null
) {
declarationMap = false;
inlineSourceMap = true;
inlineSources = true;
sourceMap = undefined;
}

return {
declarationMap,
inlineSourceMap,
inlineSources,
sourceMap,
};
}

interface DependencyInfo {
readonly assembly: spec.Assembly;
readonly resolvedDependencies: Record<string, string>;
Expand Down
11 changes: 9 additions & 2 deletions packages/jsii/test/compiler.test.ts
Expand Up @@ -149,7 +149,7 @@ describe(Compiler, () => {
}
});

test('emits declaration map', () => {
test('emits declaration map when feature is enabled', () => {
const sourceDir = mkdtempSync(join(tmpdir(), 'jsii-tmpdir'));

try {
Expand All @@ -158,6 +158,9 @@ describe(Compiler, () => {
const compiler = new Compiler({
projectInfo: {
..._makeProjectInfo(sourceDir, 'index.d.ts'),
tsc: {
declarationMap: true,
},
},
generateTypeScriptConfig: 'tsconfig.jsii.json',
});
Expand Down Expand Up @@ -191,6 +194,11 @@ function _makeProjectInfo(sourceDir: string, types: string): ProjectInfo {
bundleDependencies: {},
targets: {},
excludeTypescript: [],
tsc: {
// NOTE: these are the default values jsii uses when none are provided in package.json.
inlineSourceMap: true,
inlineSources: true,
},
};
}

Expand All @@ -203,7 +211,6 @@ function expectedTypeScriptConfig() {
charset: 'utf8',
composite: false,
declaration: true,
declarationMap: true,
experimentalDecorators: true,
incremental: true,
inlineSourceMap: true,
Expand Down
24 changes: 12 additions & 12 deletions packages/jsii/test/deprecated-remover.test.ts
Expand Up @@ -113,12 +113,12 @@ test('produces correct output', () => {
export * from './retained';
export * from './enums';
export { GrandChild, Retained } from './mixed';
//# sourceMappingURL=index.d.ts.map//////////////////
//////////////////
///////////////////////
/// deprecated.d.ts ///
//# sourceMappingURL=deprecated.d.ts.map///////////////////////
///////////////////////
/////////////////////
Expand All @@ -127,15 +127,15 @@ test('produces correct output', () => {
}
export declare class RetainedClass {
}
//# sourceMappingURL=retained.d.ts.map/////////////////////
/////////////////////
//////////////////
/// enums.d.ts ///
export declare enum SomeEnum {
VALUE_RETAINED = 0
}
//# sourceMappingURL=enums.d.ts.map//////////////////
//////////////////
//////////////////
Expand All @@ -148,7 +148,7 @@ test('produces correct output', () => {
export declare class GrandChild extends Retained implements retained_1.IRetainedInterface {
retainedMethod(): void;
}
//# sourceMappingURL=mixed.d.ts.map//////////////////
//////////////////
"
`);
});
Expand Down Expand Up @@ -177,12 +177,12 @@ test('cross-file deprecated heritage', () => {
import './deprecated';
export interface INotDeprecated {
}
//# sourceMappingURL=index.d.ts.map//////////////////
//////////////////
///////////////////////
/// deprecated.d.ts ///
//# sourceMappingURL=deprecated.d.ts.map///////////////////////
///////////////////////
"
`);
});
Expand Down Expand Up @@ -452,15 +452,15 @@ describe('stripDeprecatedAllowList', () => {
export * from './retained';
export * from './enums';
export { Deprecated, GrandChild, Retained } from './mixed';
//# sourceMappingURL=index.d.ts.map//////////////////
//////////////////
///////////////////////
/// deprecated.d.ts ///
/** @deprecated stripped */
export declare class DeprecatedClass {
}
//# sourceMappingURL=deprecated.d.ts.map///////////////////////
///////////////////////
/////////////////////
Expand All @@ -469,7 +469,7 @@ describe('stripDeprecatedAllowList', () => {
}
export declare class RetainedClass {
}
//# sourceMappingURL=retained.d.ts.map/////////////////////
/////////////////////
//////////////////
Expand All @@ -482,7 +482,7 @@ describe('stripDeprecatedAllowList', () => {
VALUE_ONE = 0,
VALUE_TWO = 1
}
//# sourceMappingURL=enums.d.ts.map//////////////////
//////////////////
//////////////////
Expand All @@ -499,7 +499,7 @@ describe('stripDeprecatedAllowList', () => {
export declare class GrandChild extends Deprecated {
retainedMethod(): void;
}
//# sourceMappingURL=mixed.d.ts.map//////////////////
//////////////////
"
`);
});
Expand Down

0 comments on commit 06d9a39

Please sign in to comment.