Skip to content

Commit

Permalink
chore(yargs-gen): use lodash.clonedeep instead of structured clone (#…
Browse files Browse the repository at this point in the history
…32537)

This will make it easier to run the build on node 16.

Adjacent changes:

pkglint had a bug that made it fail for packages with a `.` in their name. Instead now allow to pass the json path directly as an array.

Move inline function extraction completely into config and not part of yargs-gen.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mrgrain authored Dec 16, 2024
1 parent bf026bd commit f42e2cc
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 12 deletions.
4 changes: 3 additions & 1 deletion packages/aws-cdk/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,11 @@ export class DynamicValue {
}

public static fromInline(f: () => any): DynamicResult {
const ARROW = '=>';
const body = f.toString();
return {
dynamicType: 'function',
dynamicValue: f.toString(),
dynamicValue: body.substring(body.indexOf(ARROW) + ARROW.length).trim(),
};
}
}
7 changes: 4 additions & 3 deletions packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
},
"scripts": {
"build": "cdk-build",
"yargs-gen": "yarn ts-node --preferTsExts scripts/yargs-gen.ts",
"yargs-gen": "ts-node --preferTsExts scripts/yargs-gen.ts",
"watch": "cdk-watch",
"lint": "cdk-lint",
"pkglint": "pkglint -f",
Expand Down Expand Up @@ -75,8 +75,8 @@
"@types/fs-extra": "^9.0.13",
"@types/glob": "^7.2.0",
"@types/jest": "^29.5.12",
"@types/node": "^18.18.14",
"@types/mockery": "^1.4.33",
"@types/node": "^18.18.14",
"@types/promptly": "^3.0.5",
"@types/semver": "^7.5.8",
"@types/sinon": "^9.0.11",
Expand All @@ -97,6 +97,7 @@
"nock": "^13.5.5",
"sinon": "^9.2.4",
"ts-jest": "^29.2.5",
"ts-node": "^10.9.2",
"ts-mock-imports": "^1.3.16",
"xml-js": "^1.6.11"
},
Expand Down Expand Up @@ -128,12 +129,12 @@
"@jsii/check-node": "1.104.0",
"@smithy/middleware-endpoint": "3.1.4",
"@smithy/node-http-handler": "3.2.4",
"@smithy/property-provider": "3.1.10",
"@smithy/shared-ini-file-loader": "3.1.8",
"@smithy/types": "3.5.0",
"@smithy/util-retry": "3.0.7",
"@smithy/util-stream": "3.1.9",
"@smithy/util-waiter": "3.1.6",
"@smithy/property-provider": "3.1.10",
"archiver": "^5.3.2",
"camelcase": "^6.3.0",
"cdk-assets": "^3.0.0-rc.48",
Expand Down
4 changes: 2 additions & 2 deletions tools/@aws-cdk/pkglint/lib/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,7 @@ export class AllVersionsTheSame extends ValidationRule {

private validateDep(pkg: PackageJson, depField: string, dep: string) {
if (dep in this.ourPackages) {
expectJSON(this.name, pkg, depField + '.' + dep, this.ourPackages[dep]);
expectJSON(this.name, pkg, [depField, dep], this.ourPackages[dep]);
return;
}

Expand All @@ -1380,7 +1380,7 @@ export class AllVersionsTheSame extends ValidationRule {

const versions = this.usedDeps[dep];
versions.sort((a, b) => b.count - a.count);
expectJSON(this.name, pkg, depField + '.' + dep, versions[0].version);
expectJSON(this.name, pkg, [depField, dep], versions[0].version);
}
}

Expand Down
4 changes: 2 additions & 2 deletions tools/@aws-cdk/pkglint/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import { PackageJson, PKGLINT_IGNORES } from './packagejson';
export function expectJSON(
ruleName: string,
pkg: PackageJson,
jsonPath: string,
jsonPath: string | string[],
expected: any,
ignore?: RegExp,
caseInsensitive: boolean = false,
regexMatch: boolean = false,
) {
const parts = jsonPath.split('.');
const parts = Array.isArray(jsonPath) ? jsonPath : jsonPath.split('.');
const actual = deepGet(pkg.json, parts);
if (checkEquality()) {
pkg.report({
Expand Down
11 changes: 7 additions & 4 deletions tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { EsLintRules } from '@cdklabs/typewriter/lib/eslint-rules';
import * as prettier from 'prettier';
import { CliConfig, CliOption, YargsOption } from './yargs-types';

// to import lodash.clonedeep properly, we would need to set esModuleInterop: true
// however that setting does not work in the CLI, so we fudge it.
// eslint-disable-next-line @typescript-eslint/no-require-imports
const cloneDeep = require('lodash.clonedeep');

export async function renderYargs(config: CliConfig): Promise<string> {
const scope = new Module('aws-cdk');

Expand Down Expand Up @@ -106,7 +111,7 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: CliOpt
let optionsExpr = prefix;
for (const option of Object.keys(options)) {
const theOption: CliOption = options[option];
const optionProps: YargsOption = structuredClone(theOption);
const optionProps: YargsOption = cloneDeep(theOption);
const optionArgs: { [key: string]: Expression } = {};

// Array defaults
Expand All @@ -121,9 +126,7 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: CliOpt
optionArgs[optionProp] = code.expr.ident(optionValue.dynamicValue);
} else if (optionValue && optionValue.dynamicType === 'function') {
const inlineFunction: string = optionValue.dynamicValue;
const NUMBER_OF_SPACES_BETWEEN_ARROW_AND_CODE = 3;
// this only works with arrow functions, like () =>
optionArgs[optionProp] = code.expr.directCode(inlineFunction.substring(inlineFunction.indexOf('=>') + NUMBER_OF_SPACES_BETWEEN_ARROW_AND_CODE));
optionArgs[optionProp] = code.expr.directCode(inlineFunction);
} else {
optionArgs[optionProp] = lit(optionValue);
}
Expand Down
2 changes: 2 additions & 0 deletions tools/@aws-cdk/yargs-gen/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
"license": "Apache-2.0",
"dependencies": {
"@cdklabs/typewriter": "^0.0.4",
"lodash.clonedeep": "^4.5.0",
"prettier": "^2.8.8"
},
"devDependencies": {
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^29.5.12",
"@types/lodash.clonedeep": "^4.5.0",
"@types/node": "^18",
"jest": "^29.7.0"
},
Expand Down
17 changes: 17 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7927,6 +7927,18 @@
resolved "https://registry.npmjs.org/@types/license-checker/-/license-checker-25.0.6.tgz#c346285ee7e42bac58a4922059453f50a5d4175d"
integrity sha512-ju/75+YPkNE5vX1iPer+qtI1eI/LqJVYZgOsmSHI1iiEM1bQL5Gh1lEvyjR9T7ZXVE1FwJa2doWJEEmPNwbZkw==

"@types/lodash.clonedeep@^4.5.0":
version "4.5.9"
resolved "https://registry.npmjs.org/@types/lodash.clonedeep/-/lodash.clonedeep-4.5.9.tgz#ea48276c7cc18d080e00bb56cf965bcceb3f0fc1"
integrity sha512-19429mWC+FyaAhOLzsS8kZUsI+/GmBAQ0HFiCPsKGU+7pBXOQWhyrY6xNNDwUSX8SMZMJvuFVMF9O5dQOlQK9Q==
dependencies:
"@types/lodash" "*"

"@types/lodash@*":
version "4.17.13"
resolved "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.13.tgz#786e2d67cfd95e32862143abe7463a7f90c300eb"
integrity sha512-lfx+dftrEZcdBPczf9d0Qv0x+j/rfNCMuC6OcfXmO8gkfeNAY88PgKUbvG56whcN23gc27yenwF6oJZXGFpYxg==

"@types/lodash@^4.17.12":
version "4.17.12"
resolved "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.12.tgz#25d71312bf66512105d71e55d42e22c36bcfc689"
Expand Down Expand Up @@ -14259,6 +14271,11 @@ locate-path@^6.0.0:
dependencies:
p-locate "^5.0.0"

lodash.clonedeep@^4.5.0:
version "4.5.0"
resolved "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef"
integrity sha512-H5ZhCF25riFd9uB5UCkVKo61m3S/xZk1x4wA6yp/L3RFP6Z/eHH1ymQcGLo7J3GMPfm0V/7m1tryHuGVxpqEBQ==

lodash.defaults@^4.2.0:
version "4.2.0"
resolved "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz#d09178716ffea4dde9e5fb7b37f6f0802274580c"
Expand Down

0 comments on commit f42e2cc

Please sign in to comment.