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

feat(provider-generator): reference computed values nested in lists #1468

Merged
merged 5 commits into from
Jan 13, 2022

Conversation

DanielMSchmidt
Copy link
Contributor

@DanielMSchmidt DanielMSchmidt commented Jan 7, 2022

Resolves #1332

@DanielMSchmidt DanielMSchmidt force-pushed the issue-1332 branch 5 times, most recently from a1402dd to 46fca7b Compare January 7, 2022 18:28
@DanielMSchmidt DanielMSchmidt changed the title feat: expose computed values feat(provider-generator): reference computed values nested in lists Jan 7, 2022
Copy link
Collaborator

@jsteinich jsteinich left a comment

Choose a reason for hiding this comment

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

This change really only addresses 1 & 3 from #1332 (comment).
That's an improvement, but it would be nice to just finish it off finally.

Also, regarding number 3,

`public constructor(terraformResource: cdktf.ITerraformResource, terraformAttribute: string, isSingleItem: boolean)`
really should take IInterpolatingParent. Functionally it works, but we are masking the compile error with .

This does also generate some dangling types (since the 2nd case is not implemented).

@DanielMSchmidt
Copy link
Contributor Author

@jsteinich Number two is planned to be tackled by #993

Copy link
Collaborator

@jsteinich jsteinich left a comment

Choose a reason for hiding this comment

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

Just one remaining question. Otherwise it looks good.

This allows to test the required: false && optional: false usecase
Previously one could only reference one layer deep into a nested attribute, this lifts the restriction. In a world without JSII we would have written IInterpolatingParent = ITerraformResource | ComplexComputedList | ComplexComputedItem | ...
@DanielMSchmidt DanielMSchmidt merged commit 12cfdad into main Jan 13, 2022
@DanielMSchmidt DanielMSchmidt deleted the issue-1332 branch January 13, 2022 15:11
@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Gateway v2 Domain Name Resource: Missing Computed Attributes
3 participants