Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect and correct "incorrect" use of new FnJoin #516

Merged
merged 3 commits into from
Aug 7, 2018
Merged

Detect and correct "incorrect" use of new FnJoin #516

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92d00ae
Detect and correct "incorrect" use of `new FnJoin`
RomainMuller Aug 7, 2018
723d55e
Adhere to the Fn::Join syntax instead of trying to rescue improper us…
RomainMuller Aug 7, 2018
94142cb
Correct a few incorrect uses.
RomainMuller Aug 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/@aws-cdk/cdk/lib/cloudformation/fn.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ export class FnJoin extends Fn {
* @param listOfValues The list of values you want combined.
*/
constructor(delimiter: string, ...listOfValues: any[]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I rather we align the signature here to CloudFormation instead of be "smart" about it. FnConcat is a nice sugar on top that has a variadic signature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also missing test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Can do that then.

/*
* People often incorrectly use as ``new FnJoin('.', ['stuff', 'to', 'join'])``. Detect this usage pattern and
* correct the invokation so it produces the expected outcome instead of bailing out in error in CloudFormation.
*
* @see https://github.com/awslabs/aws-cdk/issues/512
*/
if (listOfValues.length === 1 && Array.isArray(listOfValues[0])) {
listOfValues = listOfValues[0];
}
if (listOfValues.length === 0) {
throw new Error(`FnJoin requires at least one value to be provided`);
}
super('Fn::Join', [ delimiter, listOfValues ]);
}
}
Expand Down