diff --git a/package.json b/package.json index 6dad27d90e51f..c79dbb9cfa2ff 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,9 @@ "nohoist": [ "**/jszip", "**/jszip/**", + "@aws-cdk/aws-codebuild/@aws-cdk/yaml-cfn", + "@aws-cdk/aws-codebuild/@aws-cdk/yaml-cfn/yaml", + "@aws-cdk/aws-codebuild/@aws-cdk/yaml-cfn/yaml/**", "@aws-cdk/aws-codepipeline-actions/case", "@aws-cdk/aws-codepipeline-actions/case/**", "@aws-cdk/aws-cognito/punycode", @@ -63,6 +66,9 @@ "@aws-cdk/cloud-assembly-schema/jsonschema/**", "@aws-cdk/cloud-assembly-schema/semver", "@aws-cdk/cloud-assembly-schema/semver/**", + "@aws-cdk/cloudformation-include/@aws-cdk/yaml-cfn", + "@aws-cdk/cloudformation-include/@aws-cdk/yaml-cfn/yaml", + "@aws-cdk/cloudformation-include/@aws-cdk/yaml-cfn/yaml/**", "@aws-cdk/core/@balena/dockerignore", "@aws-cdk/core/@balena/dockerignore/**", "@aws-cdk/core/fs-extra", @@ -75,6 +81,9 @@ "@aws-cdk/cx-api/semver/**", "@aws-cdk/yaml-cfn/yaml", "@aws-cdk/yaml-cfn/yaml/**", + "aws-cdk-lib/@aws-cdk/yaml-cfn", + "aws-cdk-lib/@aws-cdk/yaml-cfn/yaml", + "aws-cdk-lib/@aws-cdk/yaml-cfn/yaml/**", "aws-cdk-lib/@balena/dockerignore", "aws-cdk-lib/@balena/dockerignore/**", "aws-cdk-lib/case", @@ -93,6 +102,9 @@ "aws-cdk-lib/semver/**", "aws-cdk-lib/yaml", "aws-cdk-lib/yaml/**", + "monocdk/@aws-cdk/yaml-cfn", + "monocdk/@aws-cdk/yaml-cfn/yaml", + "monocdk/@aws-cdk/yaml-cfn/yaml/**", "monocdk/@balena/dockerignore", "monocdk/@balena/dockerignore/**", "monocdk/case", diff --git a/packages/@aws-cdk/aws-codebuild/package.json b/packages/@aws-cdk/aws-codebuild/package.json index e09f4f68e5264..95dba59271449 100644 --- a/packages/@aws-cdk/aws-codebuild/package.json +++ b/packages/@aws-cdk/aws-codebuild/package.json @@ -103,6 +103,9 @@ "@aws-cdk/yaml-cfn": "0.0.0", "constructs": "^3.2.0" }, + "bundledDependencies": [ + "@aws-cdk/yaml-cfn" + ], "homepage": "https://github.com/aws/aws-cdk", "peerDependencies": { "@aws-cdk/aws-cloudwatch": "0.0.0", @@ -119,7 +122,6 @@ "@aws-cdk/aws-secretsmanager": "0.0.0", "@aws-cdk/core": "0.0.0", "@aws-cdk/region-info": "0.0.0", - "@aws-cdk/yaml-cfn": "0.0.0", "constructs": "^3.2.0" }, "engines": { diff --git a/packages/@aws-cdk/cloudformation-include/package.json b/packages/@aws-cdk/cloudformation-include/package.json index c913b93bcdf41..c2ec62d8d0dc2 100644 --- a/packages/@aws-cdk/cloudformation-include/package.json +++ b/packages/@aws-cdk/cloudformation-include/package.json @@ -359,7 +359,6 @@ "@aws-cdk/aws-wafv2": "0.0.0", "@aws-cdk/aws-workspaces": "0.0.0", "@aws-cdk/core": "0.0.0", - "@aws-cdk/yaml-cfn": "0.0.0", "constructs": "^3.2.0" }, "devDependencies": { @@ -371,6 +370,9 @@ "pkglint": "0.0.0", "ts-jest": "^26.5.4" }, + "bundledDependencies": [ + "@aws-cdk/yaml-cfn" + ], "keywords": [ "aws", "cdk", diff --git a/packages/@aws-cdk/yaml-cfn/.gitignore b/packages/@aws-cdk/yaml-cfn/.gitignore index bb785cfb74f08..8b9c845e5d12a 100644 --- a/packages/@aws-cdk/yaml-cfn/.gitignore +++ b/packages/@aws-cdk/yaml-cfn/.gitignore @@ -3,7 +3,6 @@ *.d.ts node_modules dist -tsconfig.json .jsii .LAST_BUILD @@ -15,4 +14,4 @@ nyc.config.js !.eslintrc.js !jest.config.js -junit.xml \ No newline at end of file +junit.xml diff --git a/packages/@aws-cdk/yaml-cfn/package.json b/packages/@aws-cdk/yaml-cfn/package.json index 7fc68d44a23af..842455998dad5 100644 --- a/packages/@aws-cdk/yaml-cfn/package.json +++ b/packages/@aws-cdk/yaml-cfn/package.json @@ -23,32 +23,6 @@ "cloudformation", "yaml" ], - "jsii": { - "outdir": "dist", - "targets": { - "java": { - "package": "software.amazon.awscdk.yaml.cfn", - "maven": { - "groupId": "software.amazon.awscdk", - "artifactId": "cdk-yaml-cfn" - } - }, - "dotnet": { - "namespace": "Amazon.CDK.Yaml.Cfn", - "packageId": "Amazon.CDK.Yaml.Cfn", - "iconUrl": "https://raw.githubusercontent.com/aws/aws-cdk/master/logo/default-256-dark.png" - }, - "python": { - "distName": "aws-cdk.yaml-cfn", - "module": "aws_cdk.yaml_cfn", - "classifiers": [ - "Framework :: AWS CDK", - "Framework :: AWS CDK :: 1" - ] - } - }, - "projectReferences": true - }, "scripts": { "build": "cdk-build", "watch": "cdk-watch", diff --git a/packages/@aws-cdk/yaml-cfn/tsconfig.json b/packages/@aws-cdk/yaml-cfn/tsconfig.json new file mode 100644 index 0000000000000..5e75173fa8734 --- /dev/null +++ b/packages/@aws-cdk/yaml-cfn/tsconfig.json @@ -0,0 +1,24 @@ +{ + "compilerOptions": { + "target":"ES2018", + "module": "commonjs", + "lib": ["es2016", "es2017.object", "es2017.string"], + "declaration": true, + "composite": true, + "strict": true, + "noImplicitAny": true, + "strictNullChecks": true, + "noImplicitThis": true, + "alwaysStrict": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": false, + "inlineSourceMap": true, + "inlineSources": true, + "experimentalDecorators": true, + "strictPropertyInitialization":false + }, + "include": ["**/*.ts" ], + "exclude": ["node_modules"] +} diff --git a/packages/aws-cdk-lib/package.json b/packages/aws-cdk-lib/package.json index b0c8139bda0ed..97131ef9ff57f 100644 --- a/packages/aws-cdk-lib/package.json +++ b/packages/aws-cdk-lib/package.json @@ -77,6 +77,7 @@ }, "license": "Apache-2.0", "bundledDependencies": [ + "@aws-cdk/yaml-cfn", "@balena/dockerignore", "case", "fs-extra", @@ -88,6 +89,7 @@ "yaml" ], "dependencies": { + "@aws-cdk/yaml-cfn": "0.0.0", "@balena/dockerignore": "^1.0.2", "case": "1.6.3", "fs-extra": "^9.1.0", diff --git a/packages/decdk/test/schema.test.ts b/packages/decdk/test/schema.test.ts index 7b097dcc9fcbe..84888d94b365b 100644 --- a/packages/decdk/test/schema.test.ts +++ b/packages/decdk/test/schema.test.ts @@ -79,7 +79,7 @@ test('schemaForInterface: interface with primitives', async () => { * are propagated outwards. */ function spawn(command: string, options: SpawnOptions | undefined) { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const cp = spawnAsync(command, [], { stdio: 'inherit', ...options }); cp.on('error', reject); diff --git a/packages/monocdk/package.json b/packages/monocdk/package.json index b52c2bec922a5..f98dd437ffbf3 100644 --- a/packages/monocdk/package.json +++ b/packages/monocdk/package.json @@ -82,6 +82,7 @@ }, "license": "Apache-2.0", "bundledDependencies": [ + "@aws-cdk/yaml-cfn", "@balena/dockerignore", "case", "fs-extra", @@ -93,6 +94,7 @@ "yaml" ], "dependencies": { + "@aws-cdk/yaml-cfn": "0.0.0", "@balena/dockerignore": "^1.0.2", "case": "1.6.3", "fs-extra": "^9.1.0", diff --git a/tools/nodeunit-shim/index.ts b/tools/nodeunit-shim/index.ts index 8ba50bedefefd..1c4ee174ff229 100644 --- a/tools/nodeunit-shim/index.ts +++ b/tools/nodeunit-shim/index.ts @@ -81,7 +81,7 @@ export function nodeunitShim(exports: Record) { }); } else { // It's a test - test(testName, () => new Promise(ok => { + test(testName, () => new Promise(ok => { testObj(new Test(ok)); })); } diff --git a/tools/pkglint/bin/pkglint.ts b/tools/pkglint/bin/pkglint.ts index 0b5cd61ef1649..31cf7f5d2c74d 100644 --- a/tools/pkglint/bin/pkglint.ts +++ b/tools/pkglint/bin/pkglint.ts @@ -24,8 +24,16 @@ async function main(): Promise { const pkgs = findPackageJsons(argv.directory as string); - rules.forEach(rule => pkgs.filter(pkg => pkg.shouldApply(rule)).forEach(pkg => rule.prepare(pkg))); - rules.forEach(rule => pkgs.filter(pkg => pkg.shouldApply(rule)).forEach(pkg => rule.validate(pkg))); + for (const rule of rules) { + for (const pkg of pkgs.filter(pkg => pkg.shouldApply(rule))) { + rule.prepare(pkg); + } + } + for (const rule of rules) { + for (const pkg of pkgs.filter(pkg => pkg.shouldApply(rule))) { + await rule.validate(pkg); + } + } if (argv.fix) { pkgs.forEach(pkg => pkg.applyFixes()); diff --git a/tools/pkglint/lib/packagejson.ts b/tools/pkglint/lib/packagejson.ts index a59e8f1c6e307..7a7375fbe6fab 100644 --- a/tools/pkglint/lib/packagejson.ts +++ b/tools/pkglint/lib/packagejson.ts @@ -361,5 +361,5 @@ export abstract class ValidationRule { /** * Will be executed for every package definition once, should mutate the package object */ - public abstract validate(pkg: PackageJson): void; + public abstract validate(pkg: PackageJson): void | Promise; } diff --git a/tools/pkglint/lib/rules.ts b/tools/pkglint/lib/rules.ts index 1dd32c1c96392..b91e7954f352d 100644 --- a/tools/pkglint/lib/rules.ts +++ b/tools/pkglint/lib/rules.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as caseUtils from 'case'; +import * as fse from 'fs-extra'; import * as glob from 'glob'; import * as semver from 'semver'; import { LICENSE, NOTICE } from './licensing'; @@ -11,6 +12,7 @@ import { fileShouldBe, fileShouldBeginWith, fileShouldContain, fileShouldNotContain, findInnerPackages, + findPackageDir, monoRepoRoot, } from './util'; @@ -1370,25 +1372,42 @@ export class FastFailingBuildScripts extends ValidationRule { } } +/** + * For every bundled dependency, we need to make sure that package and all of its transitive dependencies are nohoisted + * + * Bundling literally works by including `/node_modules/` into + * the tarball when `npm pack` is run, and if that directory does not exist at + * that exact location (because it has been hoisted) then NPM shrugs its + * shoulders and the dependency will be missing from the distribution. + * + * -- + * + * We also must not forget to nohoist transitive dependencies. Strictly + * speaking, we need to only hoist transitive *runtime* dependencies (`dependencies`, not + * `devDependencies`). + * + * For 3rd party deps, there is no difference and we short-circuit by adding a + * catch-all glob (`/node_modules//**`), but for in-repo bundled + * dependencies, we DO need the `devDependencies` installed as per normal and + * only the transitive runtime dependencies nohoisted (recursively). + */ export class YarnNohoistBundledDependencies extends ValidationRule { public readonly name = 'yarn/nohoist-bundled-dependencies'; - public validate(pkg: PackageJson) { + public async validate(pkg: PackageJson) { const bundled: string[] = pkg.json.bundleDependencies || pkg.json.bundledDependencies || []; - if (bundled.length === 0) { return; } const repoPackageJson = path.resolve(__dirname, '../../../package.json'); + const nohoist = new Set(require(repoPackageJson).workspaces.nohoist); // eslint-disable-line @typescript-eslint/no-require-imports - const nohoist: string[] = require(repoPackageJson).workspaces.nohoist; // eslint-disable-line @typescript-eslint/no-require-imports + const expectedNoHoistEntries = new Array(); - const missing = new Array(); for (const dep of bundled) { - for (const entry of [`${pkg.packageName}/${dep}`, `${pkg.packageName}/${dep}/**`]) { - if (nohoist.indexOf(entry) >= 0) { continue; } - missing.push(entry); - } + await noHoistDependency(pkg.packageName, dep, pkg.packageRoot); } + const missing = expectedNoHoistEntries.filter(entry => !nohoist.has(entry)); + if (missing.length > 0) { pkg.report({ ruleName: this.name, @@ -1400,6 +1419,23 @@ export class YarnNohoistBundledDependencies extends ValidationRule { }, }); } + + async function noHoistDependency(parentPackageHierarchy: string, depName: string, parentPackageDir: string) { + expectedNoHoistEntries.push(`${parentPackageHierarchy}/${depName}`); + + const dependencyDir = await findPackageDir(depName, parentPackageDir); + if (!isMonoRepoPackageDir(dependencyDir)) { + // Not one of ours, so we can just ignore everything underneath as well + expectedNoHoistEntries.push(`${parentPackageHierarchy}/${depName}/**`); + return; + } + + // A monorepo package, recurse into dependencies (but not devDependencies) + const packageJson = await fse.readJson(path.join(dependencyDir, 'package.json')); + for (const dep of Object.keys(packageJson.dependencies ?? {})) { + await noHoistDependency(`${parentPackageHierarchy}/${depName}`, dep, dependencyDir); + } + } } } @@ -1669,3 +1705,14 @@ function cdkMajorVersion() { const releaseJson = require(`${__dirname}/../../../release.json`); return releaseJson.majorVersion as number; } + +/** + * Whether this is a package in the monorepo or not + * + * We're going to be cheeky and not do too much analysis, and say that + * a package that has `/node_modules/` in the directory name is NOT in the + * monorepo, otherwise it is. + */ +function isMonoRepoPackageDir(packageDir: string) { + return path.resolve(packageDir).indexOf(`${path.sep}node_modules${path.sep}`) === -1; +} \ No newline at end of file diff --git a/tools/pkglint/lib/util.ts b/tools/pkglint/lib/util.ts index 10b4415a6c3ca..fe56512113ec2 100644 --- a/tools/pkglint/lib/util.ts +++ b/tools/pkglint/lib/util.ts @@ -190,3 +190,39 @@ export function* findInnerPackages(dir: string): IterableIterator { yield* findInnerPackages(path.join(dir, fname)); } } + +/** + * Find package directory + * + * Do this by walking upwards in the directory tree until we find + * `/node_modules//package.json`. + * + * ------- + * + * Things that we tried but don't work: + * + * 1. require.resolve(`${depName}/package.json`, { paths: [rootDir] }); + * + * Breaks with ES Modules if `package.json` has not been exported, which is + * being enforced starting Node12. + * + * 2. findPackageJsonUpwardFrom(require.resolve(depName, { paths: [rootDir] })) + * + * Breaks if a built-in NodeJS package name conflicts with an NPM package name + * (in Node15 `string_decoder` is introduced...) + */ +export async function findPackageDir(depName: string, rootDir: string) { + let prevDir; + let dir = rootDir; + while (dir !== prevDir) { + const candidateDir = path.join(dir, 'node_modules', depName); + if (await new Promise(ok => fs.exists(path.join(candidateDir, 'package.json'), ok))) { + return new Promise((ok, ko) => fs.realpath(candidateDir, (err, result) => err ? ko(err) : ok(result))); + } + + prevDir = dir; + dir = path.dirname(dir); // dirname('/') -> '/', dirname('c:\\') -> 'c:\\' + } + + throw new Error(`Did not find '${depName}' upwards of '${rootDir}'`); +} \ No newline at end of file