feat(cdk): allow Tokens to be encoded as lists #1144
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add the ability for Tokens to be encoded into lists using the `token.toList()` method. This is useful for Tokens that intrinsically represent lists of strings, so we can pass them around in the type system as `string[]` (and interchange them with literal `string[]` values). The encoding is reversible just like string encodings are reversible. Contrary to strings encodings, concatenation operations are not allowed (they cannot be applied after decoding since CloudFormation does not have any list concatenation operators). This change does not change any CloudFormation resources yet to take advantage of the new encoding. Implements the most important remaining part of #744.
eladb
approved these changes
Nov 12, 2018
| // Optimization: if we can immediately resolve this, don't bother | ||
| // registering a Token. | ||
| if (valueType === 'string' || valueType === 'number' || valueType === 'boolean') { | ||
| return this.valueOrFunction.toString(); |
There was a problem hiding this comment.
This will return a string not string[]. Shouldn't that be an error?
There was a problem hiding this comment.
Oh yes. Copy pasta.
There was a problem hiding this comment.
Don't understand why TSC doesn't complain about that :(
| // | ||
| // arrays - resolve all values, remove undefined and remove empty arrays | ||
| // | ||
|
|
||
| if (Array.isArray(obj)) { | ||
| if (containsListToken(obj)) { |
There was a problem hiding this comment.
Something is odd here... Isn't a list token always size=1? Why are we checking if the array "contains a list token"? Is this for the use case where an array contains other arrays as elements (that is solved in the recursion).
There was a problem hiding this comment.
No, the issue is to detect the case where someone has done:
const xs = token.toList();
xs.push('hello');
// Or
const xs = ['hello'].concat(token.toList());In that case, we want to detect that the token as constructed was meant to be a Token but is now illegal because of the concatenation.
4 tasks
eladb
pushed a commit
that referenced
this pull request
Jan 8, 2019
Now that we can represent attributes as native strings and string lists, this covers the majority of resource attributes in CloudFormation. This change: * String attributes are represented as `string` (like before). * String list attribute are now represented as `string[]`. * Any other attribute types are represented as `cdk.Token`. Attributes that are represented as Tokens as of this change: * amazonmq has a bunch of `Integer` attributes (will be solved by #1455) * iot1click has a bunch of `Boolean` attributes * cloudformation has a JSON attribute * That's all. A few improvements to cfn2ts: * Auto-detect cfn2ts scope from package.json so it is more self-contained and doesn't rely on cdk-build-tools to run. * Added a "cfn2ts" npm script to all modules so it is now possible to regenerate all L1 via "lerna run cfn2ts". * Removed the premature optimization for avoiding code regeneration (it saved about 0.5ms). Fixes #1406 BREAKING CHANGE: any `CfnXxx` resource attributes that represented a list of strings are now typed as `string[]`s (via #1144). Attributes that represent strings, are still typed as `string` (#712) and all other attribute types are represented as `cdk.Token`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add the ability for Tokens to be encoded into lists using the
token.toList()method.This is useful for Tokens that intrinsically represent lists of strings,
so we can pass them around in the type system as
string[](andinterchange them with literal
string[]values).The encoding is reversible just like string encodings are reversible.
Contrary to strings encodings, concatenation operations are not
allowed (they cannot be applied after decoding since CloudFormation
does not have any list concatenation operators).
This change does not change any CloudFormation resources yet to
take advantage of the new encoding.
Implements the most important remaining part of #744.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.