Skip to content

Commit

Permalink
feat(jsii): Enforce use of peerDependencies (#421)
Browse files Browse the repository at this point in the history
Any direct dependency to another jsii module must be recorded as a
peerDependency as well, guaranteeing a consistent behavior of
dependencies across all possible jsii runtimes.

BREAKING CHANGE: All direct dependencies must be duplicated in
                 peerDependencies unless they are in bundledDependencies.

Fixes #361
  • Loading branch information
RomainMuller authored Apr 2, 2019
1 parent 42dece1 commit e72fea5
Show file tree
Hide file tree
Showing 24 changed files with 177 additions and 150 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
node_modules/
.BUILD_COMPLETED
lerna-debug.log
tsconfig.tsbuildinfo
.DS_Store
.idea
.vs
Expand Down
4 changes: 2 additions & 2 deletions packages/codemaker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"main": "lib/index.js",
"types": "lib/index.d.ts",
"scripts": {
"build": "tsc",
"watch": "tsc -w",
"build": "tsc --build",
"watch": "tsc --build -w",
"test": "nodeunit test/test.*.js",
"package": "rm -fr dist/js && mkdir -p dist/js && mv $(npm pack) dist/js"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/codemaker/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"noUnusedLocals": true, /* Report errors on unused locals. */
"noUnusedParameters": true, /* Report errors on unused parameters. */
"noImplicitReturns": true, /* Report error when not all code paths in function return a value. */
"noFallthroughCasesInSwitch": true /* Report errors for fallthrough cases in switch statement. */
"noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */
"composite": true
}
}
2 changes: 1 addition & 1 deletion packages/jsii-dotnet-jsonmodel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"types": "lib/index.d.ts",
"scripts": {
"gen": "/bin/bash ./generate.sh",
"build": "tsc && /bin/bash ./build.sh",
"build": "tsc --build && /bin/bash ./build.sh",
"test": "/bin/bash ./test.sh",
"package": "package-dotnet"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-dotnet-runtime-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"private": true,
"scripts": {
"gen": "/bin/bash ./generate.sh",
"build": "npm run gen && tsc && /bin/bash ./build.sh",
"build": "npm run gen && tsc --build && /bin/bash ./build.sh",
"test": "/bin/bash ./test.sh"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-dotnet-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"types": "lib/index.d.ts",
"scripts": {
"gen": "/bin/bash ./generate.sh",
"build": "npm run gen && tsc && /bin/bash ./build.sh",
"build": "npm run gen && tsc --build && /bin/bash ./build.sh",
"test": "/bin/bash ./test.sh",
"package": "package-dotnet"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-java-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"private": true,
"scripts": {
"gen": "/bin/bash ./generate.sh",
"build": "tsc && npm run gen && cd project && mvn deploy -D altDeploymentRepository=local::default::file://${PWD}/../maven-repo",
"build": "tsc --build && npm run gen && cd project && mvn deploy -D altDeploymentRepository=local::default::file://${PWD}/../maven-repo",
"test": "echo 'Tests are run as part of the build target'",
"package": "package-java"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-kernel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"main": "lib/index.js",
"types": "lib/index.d.ts",
"scripts": {
"build": "tsc && tslint -p .",
"watch": "tsc -w",
"build": "tsc --build && tslint -p .",
"watch": "tsc --build -w",
"lint": "tslint -p . --force",
"test": "nodeunit test/test.*.js",
"package": "package-js"
Expand Down
9 changes: 7 additions & 2 deletions packages/jsii-kernel/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@
"inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

/* Experimental Options */
"experimentalDecorators": true /* Enables experimental support for ES7 decorators. */
"experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */
// "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */
}
"composite": true
},
"include": ["**/*.ts"],
"references": [{
"path": "../jsii-spec"
}]
}
4 changes: 2 additions & 2 deletions packages/jsii-pacmak/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"types": "lib/index.d.ts",
"scripts": {
"gen": "/bin/bash generate.sh",
"build": "npm run gen && tsc && chmod +x bin/jsii-pacmak && tslint -p .",
"watch": "tsc -w",
"build": "npm run gen && tsc --build && chmod +x bin/jsii-pacmak && tslint -p .",
"watch": "tsc --build -w",
"lint": "tslint -p . --force",
"test": "/bin/bash test/diff-test.sh && /bin/bash test/build-test.sh",
"package": "package-js"
Expand Down
13 changes: 11 additions & 2 deletions packages/jsii-pacmak/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,16 @@
"inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

/* Experimental Options */
"experimentalDecorators": true /* Enables experimental support for ES7 decorators. */
"experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */
// "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */
}
"composite": true
},
"include": [
"**/*.ts"
],
"references": [{
"path": "../jsii-spec"
}, {
"path": "../codemaker"
}]
}
4 changes: 2 additions & 2 deletions packages/jsii-reflect/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"jsii-tree": "bin/jsii-tree"
},
"scripts": {
"build": "tsc && chmod +x bin/jsii-tree",
"watch": "tsc -w",
"build": "tsc --build && chmod +x bin/jsii-tree",
"watch": "tsc --build -w",
"test": "jest",
"package": "package-js"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"jsii-runtime": "bin/jsii-runtime"
},
"scripts": {
"build": "tsc && chmod +x bin/jsii-runtime && /bin/bash ./bundle.sh",
"watch": "tsc -w",
"build": "tsc --build && chmod +x bin/jsii-runtime && /bin/bash ./bundle.sh",
"watch": "tsc --build -w",
"test": "/bin/bash test/playback-test.sh",
"package": "package-js"
},
Expand Down
11 changes: 6 additions & 5 deletions packages/jsii-spec/lib/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,18 @@ export interface PackageVersion {
dependencies?: { [assembly: string]: PackageVersion };

/**
* Indicates if this dependency is a peer dependency or a normal dependency.
* Indicates if this dependency is a direct (peer) dependency or a
* transitive dependency.
*
* Peer dependencies are expected to be explicitly defined by the user of
* this library instead of brought in as transitive dependencies.
*
* jsii enforces that if this module exports a type from a dependency, this
* dependency must be defined as a peer and not as a normal dependency.
* Otherwise, it would be impossible to safely use two versions of this
* dependency in a closure.
* jsii enforces that any direct dependency on another jsii module is also
* defined as a peerDependency. Otherwise, it would be impossible to safely
* use two versions of this dependency in a closure.
*
* @see https://github.com/awslabs/aws-cdk/issues/979
* @see https://github.com/awslabs/jsii/issues/361
* @see https://nodejs.org/en/blog/npm/peer-dependencies/
*/
peer?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-spec/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"main": "lib/index.js",
"types": "lib/index.d.ts",
"scripts": {
"watch": "tsc -w",
"build": "tsc && bash generate-json-schema.sh",
"build": "tsc --build && bash generate-json-schema.sh",
"watch": "tsc --build -w",
"test": "nodeunit test/test.*.js",
"package": "package-js"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/jsii-spec/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
// "inlineSources": false, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

/* Experimental Options */
"experimentalDecorators": true /* Enables experimental support for ES7 decorators. */
"experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */
// "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */
"composite": true
}
}
3 changes: 0 additions & 3 deletions packages/jsii/bin/jsii-fix-peers

This file was deleted.

95 changes: 0 additions & 95 deletions packages/jsii/bin/jsii-fix-peers.ts

This file was deleted.

14 changes: 11 additions & 3 deletions packages/jsii/bin/jsii.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,16 @@ import { VERSION } from '../lib/version';
.env('JSII')
.option('watch', { alias: 'w', type: 'boolean', desc: 'Watch for file changes and recompile automatically' })
.option('verbose', { alias: 'v', type: 'count', desc: 'Increase the verbosity of output', global: true })
// tslint:disable-next-line:max-line-length
.option('project-references', { alias: 'r', type: 'boolean', desc: 'Generate TypeScript project references (also [package.json].jsii.projectReferences)' })
.option('project-references', {
alias: 'r',
type: 'boolean',
desc: 'Generate TypeScript project references (also [package.json].jsii.projectReferences)'
})
.option('fix-peer-dependencies', {
type: 'boolean',
default: true,
desc: 'Automatically add missing entries in the peerDependencies section of package.json'
})
.help()
.version(VERSION)
.argv;
Expand All @@ -23,7 +31,7 @@ import { VERSION } from '../lib/version';
const projectRoot = path.normalize(path.resolve(process.cwd(), argv._[0] || '.'));

const compiler = new Compiler({
projectInfo: await loadProjectInfo(projectRoot),
projectInfo: await loadProjectInfo(projectRoot, { fixPeerDependencies: argv['fix-peer-dependencies'] }),
watch: argv.watch,
projectReferences: argv['project-references']
});
Expand Down
42 changes: 37 additions & 5 deletions packages/jsii/lib/project-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,55 @@ export interface ProjectInfo {
readonly projectReferences?: boolean;
}

export async function loadProjectInfo(projectRoot: string): Promise<ProjectInfo> {
const pkg = require(path.join(projectRoot, 'package.json'));
export async function loadProjectInfo(projectRoot: string, { fixPeerDependencies }: { fixPeerDependencies: boolean }): Promise<ProjectInfo> {
const packageJsonPath = path.join(projectRoot, 'package.json');
const pkg = require(packageJsonPath);

const bundleDependencies: { [name: string]: string } = {};
(pkg.bundleDependencies || pkg.bundledDependencies || []).forEach((name: string) => {
const version = pkg.dependencies && pkg.dependencies[name];
if (!version) {
throw new Error(`The "package.json" has "${name}" in "bundleDependencies", but it is not declared in "dependencies"`);
throw new Error(`The "package.json" file has "${name}" in "bundleDependencies", but it is not declared in "dependencies"`);
}

if (pkg.peerDependencies && name in pkg.peerDependencies) {
throw new Error(`The "package.json" has "${name}" in "bundleDependencies", and also in "peerDependencies"`);
throw new Error(`The "package.json" file has "${name}" in "bundleDependencies", and also in "peerDependencies"`);
}

bundleDependencies[name] = version;
});

let addedPeerDependency = false;
Object.entries(pkg.dependencies || {}).forEach(([name, version]) => {
if (name in bundleDependencies) {
return;
}
pkg.peerDependencies = pkg.peerDependencies || {};
const peerVersion = pkg.peerDependencies[name];
if (peerVersion === version) {
return;
}
if (!fixPeerDependencies) {
if (peerVersion) {
// tslint:disable-next-line: max-line-length
throw new Error(`The "package.json" file has different version requirements for "${name}" in "dependencies" (${version}) versus "peerDependencies" (${peerVersion})`);
}
throw new Error(`The "package.json" file has "${name}" in "dependencies", but not in "peerDependencies"`);
}
if (peerVersion) {
LOG.warn(`Changing "peerDependency" on "${name}" from "${peerVersion}" to ${version}`);
} else {
LOG.warn(`Recording missing "peerDependency" on "${name}" at ${version}`);
}
pkg.peerDependencies[name] = version;
addedPeerDependency = true;
});
// Re-write "package.json" if we fixed up "peerDependencies" and were told to automatically fix.
// Yes, we should never have addedPeerDependencies if not fixPeerDependency, but I still check again.
if (addedPeerDependency && fixPeerDependencies) {
await fs.writeJson(packageJsonPath, pkg, { encoding: 'utf8', spaces: 2 });
}

const transitiveAssemblies: { [name: string]: spec.Assembly } = {};
const dependencies =
await _loadDependencies(pkg.dependencies, projectRoot, transitiveAssemblies, new Set<string>(Object.keys(bundleDependencies)));
Expand Down Expand Up @@ -109,7 +141,7 @@ function _guessRepositoryType(url: string): string {
throw new Error(`The "package.json" file must specify the "repository.type" attribute (could not guess from ${url})`);
}

async function _loadDependencies(dependencies: { [name: string]: string | spec.PackageVersion } | undefined,
async function _loadDependencies(dependencies: { [name: string]: string | spec.PackageVersion } | undefined,
searchPath: string,
transitiveAssemblies: { [name: string]: spec.Assembly },
bundled = new Set<string>()): Promise<spec.Assembly[]> {
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
"jsii-fix-peers": "bin/jsii-fix-peers"
},
"scripts": {
"build": "cp ../../README.md . && bash ./generate.sh && tsc",
"watch": "bash ./generate.sh && tsc -w",
"build": "cp ../../README.md . && bash ./generate.sh && tsc --build",
"watch": "bash ./generate.sh && tsc --build -w",
"test": "nyc nodeunit test/test.*.js",
"package": "package-js"
},
Expand Down
Loading

0 comments on commit e72fea5

Please sign in to comment.