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

Replace ComplexComputedList with native array with a single ComplexComputedObject #993

Closed
DanielMSchmidt opened this issue Sep 10, 2021 · 3 comments · Fixed by #1499
Closed
Assignees
Labels
enhancement New feature or request feature/tokens priority/important-soon High priority, to be worked on as part of our current release or the following one. size/large estimated < 1 month
Milestone

Comments

@DanielMSchmidt
Copy link
Contributor

DanielMSchmidt commented Sep 10, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Currently we have a very limited way of working with lists as outputs of providers and the things we support make it clear that we don't deal with a native array but something else.
To make lists behave seamlessly we need to make them first class citizens by using native arrays.

To implement this properly we need to

  • return a list of Complex Elements from the getter, where
    • each native property is a token
    • each computed property is readonly
    • each complex property with computed attributes adds a put${propertyName} method to the element that takes an object with only the not-computed types as a property
  • Enhance the resolve workflow to to recognize complex lists and resolve them properly
  • Make the list 1 element long and throw an error if that element is used directly (i.e. through accessing the array)

This also enables us to advance the work of #925 and expose computed types as well. The current problem there is that the interface expects a getter to have the type DataKubernetesPersistentVolumeClaimSpecResource[], but in the current implementation it has (index: string) => DataKubernetesPersistentVolumeClaimSpecResources.

What this should enable

Currently inputs that are passed to a resource share the same type with references to that same attribute. For example, if we pass a string, we get back a string (which is an token encoded reference, but still appears to be a string). This is true for almost all types but not for lists. We want to achieve this for lists which contain objects (nested blocks) instead of plain values (lists of numbers or strings already return tokens encoded as a list of number / string containing the reference to that attribute).

Referencing list arguments also works in other languages than TS

When a list exists which can be set as input by the user, referencing that list does not work in other languages than TS (see: #1371). This allows the following to work:

todo: in Go referencing a list, passing it to e.g. Fn.length

Passing a reference to a list works without "fqn" hack

Example in HCL

data "aws_availability_zones" "azs" {
  state = "available"
}

resource "aws_subnet" "this" {
  # ...
  availability_zone_id = data.aws_availability_zones.azs.zone_ids[0]
}

This equivalent now works:

const azs = new DataAvailabilityZones(this, 'azs', { state: "available" })
new aws.Subnet(this, 'this', {
	availabilityZoneId: Fn.element(azs.zoneIds, 0);
	// Note: this would throw an error (as we cannot know this at synth time)
	// availabilityZoneId: azs.zoneIds[0]
})

Formerly this workaround was required (and did not work in other languages than TypeScript)

const azs = new DataAvailabilityZones(this, 'azs', { state: "available" })
new aws.Subnet(this, 'this', {
	availabilityZoneId: Fn.element(Fn.lookup(azs.fqn, 'zone_ids'), 0);
})

No more listAttribute(index) method

As passing a list reference into Fn.element() works now this type of method is not needed anymore. It was not very idiomatic anyway as it treated the output (reference) to an attribute of a resources as a different type than the input for said attribute.
Formerly listAttribute was a function (wat!?):

const item = resource.listAttribute('0'); // will be removed

Now it is a list, just like the related input:

const item = Fn.element(resource.listAttribute, 0);

References

@DanielMSchmidt DanielMSchmidt added enhancement New feature or request feature/tokens labels Sep 10, 2021
@DanielMSchmidt DanielMSchmidt added committed size/large estimated < 1 month labels Sep 21, 2021
@phinze phinze modified the milestones: 0.7, 0.8 (in planning) Oct 14, 2021
@DanielMSchmidt DanielMSchmidt added priority/important-soon High priority, to be worked on as part of our current release or the following one. and removed committed labels Nov 29, 2021
@ansgarm ansgarm assigned ansgarm and unassigned DanielMSchmidt Dec 14, 2021
@ansgarm ansgarm changed the title Replace ComplexComputedList with native array Replace ComplexComputedList with native array with a single ComplexObject Jan 10, 2022
@ansgarm ansgarm changed the title Replace ComplexComputedList with native array with a single ComplexObject Replace ComplexComputedList with native array with a single ComplexComputedObject Jan 10, 2022
@skorfmann
Copy link
Contributor

Example in HCL

data "aws_availability_zones" "azs" {
  state = "available"
}

resource "aws_subnet" "this" {
  # ...
  availability_zone_id = data.aws_availability_zones.azs.zone_ids[0]
}

Isn't this just a string list and therefore a string list token already?

@ansgarm
Copy link
Member

ansgarm commented Jan 10, 2022

Isn't this just a string list and therefore a string list token already?

Whoops, you're right @skorfmann. I'll find a better example that uses a list of objects.

@github-actions
Copy link
Contributor

I'm going to lock this issue 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 similar to this, 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 Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request feature/tokens priority/important-soon High priority, to be worked on as part of our current release or the following one. size/large estimated < 1 month
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants