Skip to content

Commit 378384a

Browse files
authored
chore(spec2cdk): refactor to use hand-written helpers for arrays (#36256)
### Reason for this change The previously generated code for getting the values of the `IxxRef` in arrays was not readable, as a for loop to mutate the array in place and comma syntax were used to return the mutated array in a single line. **Before**: ``` cdk.withResolved(props.${name}, (props.xx?.forEach((item: T | string, i: number, arr: Array<T | string>) => { arr[i] = (item as T)?.xxxRef?.xx ?? cdk.ensureStringOrUndefined(item, "xxx", "lambda.T | string"); }), props.xxx as Array<string>)); ``` **After**: ``` cdk.mapArrayInPlace(props.layers, (item: IxxxRef | string) => (item as IxxxRef)?.xxxRef?.xxxArn ?? cdk.ensureStringOrUndefined(item, "xxx", "lambda.IxxxRef | string")) ``` ### Description of changes Added a handwritten function to do the in place in mapping in a generic way, preserving type safety. ### Description of how you validated changes - This is a refactoring, unit tests for this are already in: https://github.com/aws/aws-cdk/blob/3e048c145cb801f31cf74716aacd8fa07bd004f8/packages/aws-cdk-lib/aws-lambda/test/function.test.ts#L5246 ### 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*
1 parent d8f0995 commit 378384a

File tree

4 files changed

+50
-14
lines changed

4 files changed

+50
-14
lines changed

packages/aws-cdk-lib/core/lib/runtime.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Construct } from 'constructs';
22
import { ValidationError } from './errors';
3+
import { withResolved } from './token';
34

45
// ----------------------------------------------------------------------
56
// PROPERTY MAPPERS
@@ -430,3 +431,23 @@ export function ensureStringOrUndefined(value: any, propName: string, possibleTy
430431
}
431432
return value;
432433
}
434+
435+
/**
436+
* Map the elements of an array in place, preserving the original array reference.
437+
*
438+
* If `arr` is defined and resolved, each element is replaced with the result of `mapper`
439+
* and the same array is returned, typed as `U[]`.
440+
* If `arr` is undefined, returns undefined.
441+
*
442+
*/
443+
export function mapArrayInPlace<T, U extends T>(arr: T[], mapper: (item: T) => U): U[];
444+
export function mapArrayInPlace<T, U extends T>(arr: T[] | undefined, mapper: (item: T) => U): U[] | undefined;
445+
export function mapArrayInPlace<T, U extends T>(arr: T[] | undefined, mapper: (item: T) => U): U[] | undefined {
446+
if (!arr) return undefined;
447+
withResolved(arr, () => {
448+
for (let i = 0; i < arr.length; i++) {
449+
arr[i] = mapper(arr[i]);
450+
}
451+
});
452+
return arr as unknown as U[];
453+
}

tools/@aws-cdk/spec2cdk/lib/cdk/cdk.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export class CdkCore extends ExternalModule {
3434
public readonly unionMapper = makeCallableExpr(this, 'unionMapper');
3535
public readonly requireProperty = makeCallableExpr(this, 'requireProperty');
3636
public readonly isResolvableObject = makeCallableExpr(this, 'isResolvableObject');
37+
public readonly mapArrayInPlace = makeCallableExpr(this, 'mapArrayInPlace');
3738

3839
public readonly ValidationResult = $T(Type.fromName(this, 'ValidationResult'));
3940
public readonly VALIDATION_SUCCESS = makeCallableExpr(this, 'VALIDATION_SUCCESS');

tools/@aws-cdk/spec2cdk/lib/cdk/resolver-builder.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { DefinitionReference, Property } from '@aws-cdk/service-spec-types';
2-
import { expr, Expression, Module, Type } from '@cdklabs/typewriter';
2+
import { CallableDeclaration, expr, Expression, Lambda, Module, Parameter, Type } from '@cdklabs/typewriter';
33
import { CDK_CORE } from './cdk';
44
import { RelationshipDecider, Relationship, GENERATE_RELATIONSHIPS_ON_TYPES } from './relationship-decider';
55
import { NON_RESOLVABLE_PROPERTY_NAMES } from './tagging';
@@ -78,13 +78,13 @@ export class ResolverBuilder {
7878
: Type.distinctUnionOf(resolvableType, ...newTypes);
7979

8080
const typeDisplayNames = [
81-
...relationships.map(r => r.typeDisplayName),
81+
...[...new Set(relationships.map(r => r.typeDisplayName))],
8282
resolvableType.arrayOfType?.toString() ?? resolvableType.toString(),
8383
].join(' | ');
8484

8585
// Generates code like:
8686
// For single value T | string : (props.xx as IxxxRef)?.xxxRef?.xxxArn ?? cdk.ensureStringOrUndefined(props.xxx, "xxx", "iam.IxxxRef | string");
87-
// For array <T | string>[]: (props.xx?.forEach((item: T | string, i: number, arr: Array<T | string>) => { arr[i] = (item as T)?.xxxRef?.xx ?? cdk.ensureStringOrUndefined(item, "xxx", "lambda.T | string"); }), props.xxx as Array<string>);
87+
// For array <T | string>[]: cdk.mapArrayInPlace(props.layers, (item: IxxxRef | string) => (item as IxxxRef)?.xxxRef?.xxxArn ?? cdk.ensureStringOrUndefined(item, "xxx", "lambda.IxxxRef | string"))
8888
// Ensures that arn properties always appear first in the chain as they are more general
8989
const arnRels = relationships.filter(r => r.propName.toLowerCase().endsWith('arn'));
9090
const otherRels = relationships.filter(r => !r.propName.toLowerCase().endsWith('arn'));
@@ -94,11 +94,10 @@ export class ResolverBuilder {
9494
.map(r => `(${itemName} as ${r.referenceType})?.${r.referenceName}?.${r.propName}`),
9595
`cdk.ensureStringOrUndefined(${itemName}, "${name}", "${typeDisplayNames}")`,
9696
].join(' ?? ');
97-
const resolver = (_: Expression) => {
98-
if (resolvableType.arrayOfType) {
99-
return expr.directCode(
100-
`(cdk.withResolved(props.${name}, () => props.${name}?.forEach((item: ${propType.arrayOfType!.toString()}, i: number, arr: ${propType.toString()}) => { arr[i] = ${buildChain('item')}; })), props.${name} as ${resolvableType.toString()})`,
101-
);
97+
const resolver = (props: Expression) => {
98+
if (propType.arrayOfType) {
99+
const mapper = this.createMapperLambda(propType.arrayOfType, expr.directCode(buildChain('item')));
100+
return CDK_CORE.mapArrayInPlace.call(expr.get(props, name), mapper);
102101
} else {
103102
return expr.directCode(buildChain(`props.${name}`));
104103
}
@@ -120,20 +119,35 @@ export class ResolverBuilder {
120119

121120
const resolver = (props: Expression) => {
122121
const propValue = expr.get(props, name);
123-
const isArray = baseType.arrayOfType !== undefined;
122+
// Prop can be of type IResolvable | Array<IResolvable | CfnProperty>
123+
const arrayType = resolvableType.unionOfTypes?.find(t => t.arrayOfType)?.arrayOfType;
124124

125-
const flattenCall = isArray
126-
? expr.directCode(`props.${name}.forEach((item: any, i: number, arr: any[]) => { arr[i] = ${functionName}(item) }), props.${name}`)
127-
: expr.ident(functionName).call(propValue);
125+
let flattenCall;
126+
if (arrayType) {
127+
const mapper = this.createMapperLambda(arrayType, expr.ident(functionName).call(expr.ident('item')));
128+
flattenCall = CDK_CORE.mapArrayInPlace.call(propValue, mapper);
129+
} else {
130+
flattenCall = expr.ident(functionName).call(propValue);
131+
}
128132

129133
const condition = optional
130134
? expr.cond(expr.not(propValue)).then(expr.UNDEFINED).else(flattenCall)
131135
: flattenCall;
132136

133-
return isArray
137+
return arrayType
134138
? expr.cond(CDK_CORE.isResolvableObject(propValue)).then(propValue).else(condition)
135139
: condition;
136140
};
137141
return { name, propType: resolvableType, resolvableType, baseType, resolver };
138142
}
143+
144+
/**
145+
* Creates an arrow function `(item: T) => body` for use with array mapping.
146+
* Parameter requires a CallableDeclaration scope, but Lambda is an anonymous
147+
* expression so we provide a placeholder.
148+
*/
149+
private createMapperLambda(itemType: Type, body: Expression): Lambda {
150+
const lambdaScope: CallableDeclaration = { parameters: [], returnType: Type.VOID, name: 'LambdaScope' };
151+
return new Lambda([new Parameter(lambdaScope, { name: 'item', type: itemType })], body);
152+
}
139153
}

tools/@aws-cdk/spec2cdk/test/relationships.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,6 @@ test('relationship have arns appear first in the constructor chain', () => {
321321

322322
const rendered = renderer.render(module);
323323

324-
const chain = 'this.roleArn = (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? (props.roleArn as IRoleRef)?.roleRef?.roleName ?? (props.roleArn as IRoleRef)?.roleRef?.otherPrimaryId ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | iam.IRoleRef | iam.IRoleRef | string")';
324+
const chain = 'this.roleArn = (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? (props.roleArn as IRoleRef)?.roleRef?.roleName ?? (props.roleArn as IRoleRef)?.roleRef?.otherPrimaryId ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string")';
325325
expect(rendered).toContain(chain);
326326
});

0 commit comments

Comments
 (0)