-
Notifications
You must be signed in to change notification settings - Fork 450
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
chore(lib): Standardize IResolvable usage #1299
chore(lib): Standardize IResolvable usage #1299
Conversation
Thanks for taking a look @DanielMSchmidt. @skorfmann @ansgarm any thoughts on the proposed changes? This may not make the next release, but I'd definitely like to see it in the following one. |
Thanks for the draft, @jsteinich 👍
The AWS adapter uses the output of a Terraform function here:
Hehe, nice idea. But, yeah, debatable 😄 Maybe there are other specifics of the current implementation used in the AWS adapter, but I've not found further ones and I could always resort to overrides should there be some 😄 |
3271416
to
1706331
Compare
3c9e0d2
to
609a946
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some stellar work @jsteinich 💯
packages/@cdktf/provider-generator/lib/get/generator/emitter/attributes-emitter.ts
Show resolved
Hide resolved
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. |
The goal of this PR is to establish consistent usage of
IReolvable
when working with attributes of Terraform elements by looking to the AWS CDK for guidance and making improvements where appropriate.Work already done:
IResolvable usage:
The general rule is include
IResolvable
when no token type is available and when not using custom structs.Changes from AWS CDK:
Don't use---UseIResolvable
for custom structs (labeled as 'object'). Since Terraform doesn't allow passing entire blocks, I'm not sure why we would need these to be resolvableIResolvable
when only an interface is generatednumber[]
. They are more common in Terraform and can combinestring[]
andnumber
token techniquesCreate a token type forboolean[]
. This one is debatable, but a token could be created using binary encodingRecord<string, *>
. Can use a special string token key in the map to indicate the entire map is a token. Only make for core types and expand others to useIResolvable
similar to AWS CDK since jsii requires exact typing.Other areas of consideration:
count
on resources being just a number? Probably yes since we can use a token and number translates to other languages better.IResolvable
? The AWS CDK would say no as it hurts other language support. Since chaining functions is more common in Terraform, we probably want to use it. Might be nice to make available for all, but could also do case by case analysis of which functions are likely to be chained. --- Match with other rules as much as possible.