feat: Extend support for Terraform expressions #1131
Conversation
…erator), property access (e.g. chaining x[1]["name"]) and operation expressions such as the unary ones (!, -) and binary ones (e.g. arithmetics, equality checks, boolean operators also allows marking lazy values as inner values, which allows for proper token resolution (these features are required for the aws adapter)
Exactly, it is intended for mapping of intrinsic functions from CloudFormation in the AWS Adapter, like e.g.: https://github.com/hashicorp/cdktf-aws-cdk/blob/44fe1dfd6c058ec804379cf526940bad3eece638/src/aws-adapter.ts#L410-L419. |
| @@ -22,7 +22,7 @@ export class TerraformDataSource | |||
| // TerraformMetaArguments | |||
|
|
|||
| public dependsOn?: string[]; | |||
| public count?: number; | |||
| public count?: number | IResolvable; | |||
There was a problem hiding this comment.
What's the motivation for this? Numbers are generally encoded as tokens.
There was a problem hiding this comment.
That is required for CloudFormation Conditions support. Conditions can contain different intrinsic functions which I translate to Terraform functions which can return IResolvables instead of number encoded Tokens.
There was a problem hiding this comment.
This is the line that requires it: https://github.com/hashicorp/cdktf-aws-cdk/blob/f60d52b2e6074d6be14e67e127ae957d181548f9/src/aws-adapter.ts#L150
There was a problem hiding this comment.
Couldn't you wrap it in Token.asNumber?
There was a problem hiding this comment.
I did not try that 😅 Any reason to do it that way and switch back to only allowing number?
There was a problem hiding this comment.
IResolvable is more difficult to use; especially in other languages. We haven't quite gotten there yet, but ideally we have consistent rules for when to use tokens encoded in native types and when to use IResolvable.
It's a bit unfortunate that at least the operators are done in a way that will make it difficult to make user facing in the future, but still probably for the best right now. |
Yeah, we might want to put some thought into what to expose and why (i.e. which building patterns / best practices we might provoke in doing so). |
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Required for https://github.com/hashicorp/cdktf-aws-cdk (see also this PR)