chore: un-jsii and bundle @aws-cdk/yaml-cfn (aws#13699)

The `yaml-cfn` library is used as an implementation detail of
`cloudformation-include`, `aws-cdk`, `decdk`, and the monopackages.

Un-jsii it and bundle it into the framework packages that need it.

Does not need to be accessed over jsii and does not need to be exposed
in the monopackages.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

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",

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": {

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",

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
+junit.xml

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",

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"]
+}

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",

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<void>((resolve, reject) => {
     const cp = spawnAsync(command, [], { stdio: 'inherit', ...options });
 
     cp.on('error', reject);

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",

tools/nodeunit-shim/index.ts

@@ -81,7 +81,7 @@ export function nodeunitShim(exports: Record<string, any>) {
       });
     } else {
       // It's a test
-      test(testName, () => new Promise(ok => {
+      test(testName, () => new Promise<void>(ok => {
         testObj(new Test(ok));
       }));
     }

tools/pkglint/bin/pkglint.ts

@@ -24,8 +24,16 @@ async function main(): Promise<void> {
 
   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());

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<void>;
 }

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 `<package>/node_modules/<dep>` 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 (`<package>/node_modules/<dep>/**`), 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<string>(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<string>();
 
-    const missing = new Array<string>();
     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;
+}

tools/pkglint/lib/util.ts

@@ -190,3 +190,39 @@ export function* findInnerPackages(dir: string): IterableIterator<string> {
     yield* findInnerPackages(path.join(dir, fname));
   }
 }
+
+/**
+ * Find package directory
+ *
+ * Do this by walking upwards in the directory tree until we find
+ * `<dir>/node_modules/<package>/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<string>((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}'`);
+}