Skip to content

Commit

Permalink
fix(codebuild): module fails to load with error "Cannot use import st…
Browse files Browse the repository at this point in the history
…atement outside a module"

The bundling done in #13699 is somehow only putting the TypeScript files in the resulting NPM bundle,
and thus causing the above error (from Node trying to execute TypeScript).

This reverts commit 41a2b2e from PR #13699.
  • Loading branch information
skinny85 committed Mar 25, 2021
1 parent 28ba8b4 commit b1ffd33
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 140 deletions.
12 changes: 0 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@
"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",
Expand All @@ -66,9 +63,6 @@
"@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",
Expand All @@ -81,9 +75,6 @@
"@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",
Expand All @@ -102,9 +93,6 @@
"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",
Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-codebuild/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,6 @@
"@aws-cdk/yaml-cfn": "0.0.0",
"constructs": "^3.3.69"
},
"bundledDependencies": [
"@aws-cdk/yaml-cfn"
],
"homepage": "https://github.com/aws/aws-cdk",
"peerDependencies": {
"@aws-cdk/aws-cloudwatch": "0.0.0",
Expand All @@ -122,6 +119,7 @@
"@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.3.69"
},
"engines": {
Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/cloudformation-include/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@
"@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.3.69"
},
"devDependencies": {
Expand All @@ -374,9 +375,6 @@
"pkglint": "0.0.0",
"ts-jest": "^26.5.4"
},
"bundledDependencies": [
"@aws-cdk/yaml-cfn"
],
"keywords": [
"aws",
"cdk",
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/yaml-cfn/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*.d.ts
node_modules
dist
tsconfig.json
.jsii

.LAST_BUILD
Expand All @@ -14,4 +15,4 @@ nyc.config.js
!.eslintrc.js
!jest.config.js

junit.xml
junit.xml
26 changes: 26 additions & 0 deletions packages/@aws-cdk/yaml-cfn/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,32 @@
"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",
Expand Down
24 changes: 0 additions & 24 deletions packages/@aws-cdk/yaml-cfn/tsconfig.json

This file was deleted.

2 changes: 0 additions & 2 deletions packages/aws-cdk-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
},
"license": "Apache-2.0",
"bundledDependencies": [
"@aws-cdk/yaml-cfn",
"@balena/dockerignore",
"case",
"fs-extra",
Expand All @@ -93,7 +92,6 @@
"yaml"
],
"dependencies": {
"@aws-cdk/yaml-cfn": "0.0.0",
"@balena/dockerignore": "^1.0.2",
"case": "1.6.3",
"fs-extra": "^9.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/decdk/test/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ test('schemaForInterface: interface with primitives', async () => {
* are propagated outwards.
*/
function spawn(command: string, options: SpawnOptions | undefined) {
return new Promise<void>((resolve, reject) => {
return new Promise((resolve, reject) => {
const cp = spawnAsync(command, [], { stdio: 'inherit', ...options });

cp.on('error', reject);
Expand Down
2 changes: 0 additions & 2 deletions packages/monocdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@
},
"license": "Apache-2.0",
"bundledDependencies": [
"@aws-cdk/yaml-cfn",
"@balena/dockerignore",
"case",
"fs-extra",
Expand All @@ -99,7 +98,6 @@
"yaml"
],
"dependencies": {
"@aws-cdk/yaml-cfn": "0.0.0",
"@balena/dockerignore": "^1.0.2",
"case": "1.6.3",
"fs-extra": "^9.1.0",
Expand Down
2 changes: 1 addition & 1 deletion tools/nodeunit-shim/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function nodeunitShim(exports: Record<string, any>) {
});
} else {
// It's a test
test(testName, () => new Promise<void>(ok => {
test(testName, () => new Promise(ok => {
testObj(new Test(ok));
}));
}
Expand Down
12 changes: 2 additions & 10 deletions tools/pkglint/bin/pkglint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,8 @@ async function main(): Promise<void> {

const pkgs = findPackageJsons(argv.directory as string);

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

if (argv.fix) {
pkgs.forEach(pkg => pkg.applyFixes());
Expand Down
2 changes: 1 addition & 1 deletion tools/pkglint/lib/packagejson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 | Promise<void>;
public abstract validate(pkg: PackageJson): void;
}
52 changes: 8 additions & 44 deletions tools/pkglint/lib/rules.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
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';
Expand All @@ -12,7 +11,6 @@ import {
fileShouldBe, fileShouldBeginWith, fileShouldContain,
fileShouldNotContain,
findInnerPackages,
findPackageDir,
monoRepoRoot,
} from './util';

Expand Down Expand Up @@ -1373,42 +1371,25 @@ 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 async validate(pkg: PackageJson) {
public 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 expectedNoHoistEntries = new Array<string>();
const nohoist: string[] = require(repoPackageJson).workspaces.nohoist; // eslint-disable-line @typescript-eslint/no-require-imports

const missing = new Array<string>();
for (const dep of bundled) {
await noHoistDependency(pkg.packageName, dep, pkg.packageRoot);
for (const entry of [`${pkg.packageName}/${dep}`, `${pkg.packageName}/${dep}/**`]) {
if (nohoist.indexOf(entry) >= 0) { continue; }
missing.push(entry);
}
}

const missing = expectedNoHoistEntries.filter(entry => !nohoist.has(entry));

if (missing.length > 0) {
pkg.report({
ruleName: this.name,
Expand All @@ -1420,23 +1401,6 @@ 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);
}
}
}
}

Expand Down
36 changes: 0 additions & 36 deletions tools/pkglint/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,39 +190,3 @@ 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}'`);
}

0 comments on commit b1ffd33

Please sign in to comment.