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.

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

Numbers that need stringification fail for Tokens #3769

Closed
rix0rrr opened this issue Aug 23, 2019 · 2 comments
Closed

Numbers that need stringification fail for Tokens #3769

rix0rrr opened this issue Aug 23, 2019 · 2 comments
Assignees
Labels
@aws-cdk/core Related to core CDK functionality feature-request A feature should be added or improved.

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2019

As seen in #3758:

Sometimes CFN models properties as strings which are clearly intended to be numbers, and are modeled in CDK as numbers. If we immediately stringify these numbers in the constructor and the user happened to pass a Token, the template will end up containing something like "-1.8881545897087673e+289".

The solution is to stringify sometimes lazily, with a function like this:

function stringifyNumber(x: number) {
  if (Token.isUnresolved(x)) {
    return Lazy.stringValue({ produce: context => `${context.resolve(x)}` });
  } else {
    return `${x}`;
  }
}

This feature should probably live in the standard library, as many construct libraries are likely to need it.

An alternative approach would be spidering all strings for substrings that look like they might be stringified number Tokens, but this seems error-prone and expensive (computation-wise). For example, it would depend on the double stringification behavior of all JSII languages.

@rix0rrr rix0rrr added @aws-cdk/core Related to core CDK functionality feature-request A feature should be added or improved. labels Aug 23, 2019
@jogold
Copy link
Contributor

jogold commented Aug 23, 2019

For reference, something similar was already discussed in #2846.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Dec 2, 2019

Has been added to core library as Tokenization.stringifyNumber().

@rix0rrr rix0rrr closed this as completed Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants