RFC: 193 Fixing type unions

[RomainMuller]

• Tracking issue: Fixing of type unions #193
• Rendered version

---

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license

[mergify]

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

[skinny85]

I'm having a serious problem understanding this RFC.

Can we be more crisp on "this is the proposed solution"? With examples?

We have a separate section dealing with rationale & alternatives, but I feel like the entire doc is pretty much "we can do X; another option is Y", etc., and after reading it, I still can't explain what the proposed solution is.

Thanks!

Pull request has been modified.

[skinny85]

I am opposed to the current "Recommended Approach".

In my opinion, forcing casting things to any and using parameter overrides is too big of a drop in the customer experience of the CDK. One of the foundational promises that we make in the CDK is that everything that is achievable in CloudFormation is achievable also in the CDK (albeit, maybe only on the L1 level). Resorting to type hacks to implement CloudFormation idioms seems like is pretty much breaking that promise (yes, it's technically possible, but the experience is so bad, and so poorly discoverable, that I don't feel comfortable in making that change).

A quick search through internal Amazon code shows many cases in Java of using the low-level primitives like Fn::If for complex and boolean types (ping me if you want to see them). I would bet if I did a similar search for TypeScript, I would get ten times as many results. So this capability is actively being used by our customers.

It's also very important to our future plan with cloudformation-include. if we want to write methods like Bucket.fromCfnBucket(cfnBucket: CfnBucket): IBucket, we need type safety that the properties of the CfnBucket are actually what the types say they are, not some hidden IResolvables. If that's the case, writing those methods will be, for all practical intents and purposes, impossible.

I understand that this fixes some problems in JSII, but, in my opinion, this is putting JSII before the customer experience of CDK users, which is not the correct tradeoff.

I'm open to discussing the "Alternative Approach". I've also posted my own, second, alternative, but I'm not sure how feasible it is. Would love any feedback on that.

In summary, in its current form, I vote "No" on implementing this RFC. It's not the right thing to do. Apologies if that makes your life harder, but that is my opinion.

[RomainMuller]

@skinny85 thanks for your feedback! I'm providing some answers below...

---

In my opinion, forcing casting things to any and using parameter overrides is too big of a drop in the customer experience of the CDK. One of the foundational promises that we make in the CDK is that everything that is achievable in CloudFormation is achievable also in the CDK (albeit, maybe only on the L1 level). Resorting to type hacks to implement CloudFormation idioms seems like is pretty much breaking that promise (yes, it's technically possible, but the experience is so bad, and so poorly discoverable, that I don't feel comfortable in making that change).

I agree this provides arguments in favor of the argument that claims "TypeScript is the only first class citizen language in CDK", and so far was willing to take this as an interim step. I only reluctantly got to this recommendation from lack of evidence that a more complex solution would be necessary, and I'm absolutely willing to reconsider the recommendation as concerns are raised over the general safety/usability of the current proposal.

---

I would bet if I did a similar search for TypeScript, I would get ten times as many results. So this capability is actively being used by our customers.

Well TypeScript does not suffer from requiring as any usage, so the fact this feature is widely leveraged on the user side in TypeScript isn't much of an argument against the proposal. However I worry about the consequences of using this approach in the AWS Construct Library, as this could made it impossible to inspect property values (for example, in an Aspect, though a native method override) from Java and C# (since the static type would be declared to CfnBucket.LifecyclePolicyProperty, but the actual value returned would be of type IResolvable -- leading to a ClassCastException).

---

It's also very important to our future plan with cloudformation-include. if we want to write methods like Bucket.fromCfnBucket(cfnBucket: CfnBucket): IBucket, we need type safety that the properties of the CfnBucket are actually what the types say they are, not some hidden IResolvables. If that's the case, writing those methods will be, for all practical intents and purposes, impossible.

How is it different from the current situation where those methods are typed as unions of IResolvable and some complex type? If such a fromCfnBucket can be written against the current types, the exact same code can be leveraged with values shoe-horned using an as any. I agree this makes it more difficult to write such code, and it might be a lit more error prone... But impossible sounds like an overstatement.

---

I understand that this fixes some problems in JSII, but, in my opinion, this is putting JSII before the customer experience of CDK users, which is not the correct tradeoff.

This is not about fixing problems in jsii. The reality today is this is fixing problems in CDK due to bugs in jsii that are inherent to type unions, and which cannot be fixed in the current state of affairs.

---

I'm open to discussing the "Alternative Approach". I've also posted my own, second, alternative, but I'm not sure how feasible it is. Would love any feedback on that.

As I commented, this alternative is already discussed in the document, and likely would require a complete overhaul of the jsii compiler, which might not fit in our desired timeline (although if that is the right thing to do, then we probably should bend the timeline and not the other way around).

[nija-at]

Summary section should also briefly describe the proposal.

[nija-at]

detrimental to whose performance?

[nija-at]

Provide an example for collection of type unions.

It'll make the reading a little easier to follow than asking the user to imagine them 😊

[nija-at]

Fatal is quite harsh 😅

[nija-at]

Suggested change

	results in a degradation of ergonomics for statically typed languages, but not not imply a redution in available
	results in degradation of ergonomics for statically typed languages, but not imply a reduction in available

[nija-at]

How does this API ↓ change in this approach?

lifecycleConfiguration: CfnBucket.LifecycleConfigurationProperty | cdk.IResolvable | undefined;

Does it become any? (add to the doc)

[nija-at]

Are all union types bad or are there ones that we will keep around and well supported in the JSII?

For instance, I suppose we will want to keep string | undefined and such.

[RomainMuller]

| undefined is actually not a type union in jsii. It's only a null-able type.

[nija-at]

I'm curious to know why this type erasure is better than the existing type erasure.

This feels like the most important question of your recommended approach and worth presenting a deeper and drawing a more convincing picture.

[nija-at]

Given that this issue is only with L1s and not the L2s are easy to fix, how many different types (for lack of a better word) of type unions exist in our current L1s? i.e., are they all union-ed with IResolvable vs unions are across significantly different types (number | string, etc.) vs unions of scalar and vector (string | string[])?

Given CF has a certain level of types inherently, there can't be that many variations, right?

I'm wondering if we can also change our L1 generation so that this type erasure is not lost. Taking your example as an illustration -

lifecycleConfiguration: CfnBucket.LifecycleConfigurationProperty;
lifecycleConfigurationLazy: cdk.IResolvable;

[skinny85]

I would bet if I did a similar search for TypeScript, I would get ten times as many results. So this capability is actively being used by our customers.

Well TypeScript does not suffer from requiring as any usage, so the fact this feature is widely leveraged on the user side in TypeScript isn't much of an argument against the proposal.

Sorry, I don't understand this comment. In the same message, you also write:

If such a fromCfnBucket can be written against the current types, the exact same code can be leveraged with values shoe-horned using an as any.

Your current proposal contains the following TypeScript example:

Recommended Approach

Where TypeScript developers could use:

let cfnBucket: CfnBucket;
let value: IResolvable;
// ...
bucket.lifecycleConfiguration = value as any;

So TypeScript definitely seems to suffer from as any usage, or I'm really not understanding something fundamental about the proposal here.

[RomainMuller]

I would bet if I did a similar search for TypeScript, I would get ten times as many results. So this capability is actively being used by our customers.

Well TypeScript does not suffer from requiring as any usage, so the fact this feature is widely leveraged on the user side in TypeScript isn't much of an argument against the proposal.

Sorry, I don't understand this comment. In the same message, you also write:

If such a fromCfnBucket can be written against the current types, the exact same code can be leveraged with values shoe-horned using an as any.

Your current proposal contains the following TypeScript example:

Recommended Approach

Where TypeScript developers could use:

let cfnBucket: CfnBucket;
let value: IResolvable;
// ...
bucket.lifecycleConfiguration = value as any;

So TypeScript definitely seems to suffer from as any usage, or I'm really not understanding something fundamental about the proposal here.

What I mean is that the usage of as any is not "harmful" in TypeScript: you can use this language feature to cheat with the type system, in ways that are purely impossible in statically typed languages (there is no way I'll pass (Object)foo to a Java method that accepts a Bar-typed parameter).

However, I discovered scenarios that make certain usage patterns blow up in statically typed languages if something in TypeScript (or Javascript) used as any to force a value in... Since it makes that value impossible to read back from Java without getting a ClassCastException. I'm working on an updated version of the document.

Pull request has been modified.

[skinny85]

I absolutely love it! Awesome job @RomainMuller !

[eladb]

This is the jsii RELEASE NOTES, please also add the CDK's "BREAKING CHANGE" section for 2.x which describes for e.g. .NET and Java users what's changed for them (before/after)

Pull request has been modified.

[rix0rrr]

Overall I like the strategy, just some parts of the way the doc is written I take issue with.

Also, the question of type testing of object literals section needs to be either rounded out, or acknowledged that we're not going to address it. Potentially we can [start with] disallowing object literals from being sent out by JS where the type is declared as a union.

[rix0rrr]

Do you have another example of a language that does this besides Java?

This section feels overly generic if you're only talking about Java, and I don't know another one. Happy to be educated here!

[rix0rrr]

It would somewhat. We could (probably, need to think on this) the types so that they are ordered from most to least restrictive.

In that case, the first one that matches is [probably] the intended type, with the most permissive "anything goes" type coming last and not accidentally swallowing the real type.

[RomainMuller]

The issue is that in case two types only have optional properties, a value that has no properties matches both. If an API has a contract such as "if foo is bar, then baz is ", but the kernel elects the other empty-compatible candidate, then user will have a (bad) surprise.

It's not always possible to generate a dynamic proxy that represents "both" or "either" of those types (and the Kernel API definitely does not support such a wire type currently).

[rix0rrr]

We can disallow unions where it is impossible to discriminate between the types.

Anyway, I'd like to see more detail on this specific problem in the RFC as a whole. It's a little undertreated right now.

[rix0rrr]

I'd still like to see whether you consider this problem in scope or out of scope for the current RFC, and if in scope what the proposed handling of:

type PropsUnion = FooProps | BarProps;

Is going to be.

[RomainMuller]

This is documented... You'll get the union in the same way than you would in TypeScript, and you can either use a type inspection API (isFooProps(v: PropsUnion): v is FooProps) and you can "safely" force it, or you don't have this and you have to opportunistically force it... Either way you get similar ergonomics & safety to what you have in TypeScript in the same scenario.

[rix0rrr]

Simpler proposed rollout strategy:

• Rewrite all current code to use named unions (most of them are generated so shouldn't be too hard).
• Add a pacmak flag to generate v1 bindings treating the unions as "inline types", and v2 bindings generating the union classes.

[RomainMuller]

This was the proposal before moving to that. The inconvenient of the approach you are proposing is that it does not allow fixing the currently broken things by generating better type signatures in Java/C#. Those methods could be broken right now since they cannot currently be used from those languages.

[rix0rrr]

Can you give examples of things that are today outright broken though? The only change that's acceptable is things that simply cannot be used today (can be used but with ugly syntax is not good enough since presumably we'd be changing the semantics).

[RomainMuller]

Potentially we can [start with] disallowing object literals from being sent out by JS where the type is declared as a union.

@rix0rrr - this would be a breaking change, which cannot easily be guarded against at compile-time (so users in non-TypeScript would be the only ones enjoying the nasty surprise).

[rix0rrr]

Also need to add support to jsii-reflect for this.

[RomainMuller]

As far as I'm concerned this is an integral part of adding support in @jsii/spec. I don't mind clapping it out specifically.

[rix0rrr]

Still need to do something for Python. As specced, an object literal will contain the FQN for the union type, which Python will need to handle. Although if we change the spec I think we won't need to do more for Python than handle the union type in pacmak and ignore it :).

[RomainMuller]

Still need to do something for Python. As specced, an object literal will contain the FQN for the union type, which Python will need to handle

This is naturally required as new tests are added to validate those APIs can be used and behave in the expected way.

Although if we change the spec I think we won't need to do more for Python than handle the union type in pacmak and ignore it :)

This was my intention (aka de-sugar the type aliases in Python).

[rix0rrr]

the union type's fully qualified name if the value is [an object literal]

I don't think this will work, and break the wire protocol unnecessarily.

Looks like currently, the union types are a proposal that mostly affects statically typed host languages without native type union support. In the type:

type SomeUnion = string | Foo | IBar;

• A string would be serialized as { "jsii$native": "string", "value": "some_string" } (or whatever the serialization is), same as today.
• A Foo would be serialized as { "jsii$objid": "Foo@123" }, same as today

For both those cases, the only thing a language kernel would need to do is deserialize as usual, and then wrap the value in a SomeUnion instance.

If we were to switch object literals from:

{ "jsii$objid": "Object@123", "interfaces": ["IBar"] }

// to

{ "jsii$objid": "SomeUnion@123" }

This doesn't actually help the language kernel deserialize the right value. Worse, it now has broken the Python kernel, which used to be able to deserialize the value fine but now needs to grow support for union types (or know to ignore them, or something).

Union types should have zero effect on the wire protocol, only on the kernels on the respective sides.

[RomainMuller]

The point is to continue serializing definitely typed stuff in the same way they currently are, and to only use the union's FQN in the object ID when that is not possible (i.e: when the value returned is a struct or anonymous interface implementation).

I guess we could also return some opaque value (Object@####), however I'm not sure this will actually change anything much. I think using the union's FQN might be helpful in troubleshooting (when inspecting kernel traces).

In particular, if I obtain a Object@#### from a union-returning call, which I then "force" into a parameter typed as one of the union's candidates, the Kernel will probably fail to type-check this.

[rix0rrr]

If this is true for communication the one way, then it feels like it should be equally true the other way.

[RomainMuller]

They can be passed both ways.

The only call you cannot use with is create, which will never return a type union either.

[rix0rrr]

I'd still like to see whether you consider this problem in scope or out of scope for the current RFC, and if in scope what the proposed handling of:

type PropsUnion = FooProps | BarProps;

Is going to be.

[nija-at]

Approving to indicate that I like to see this proposal go forward.

Some comments below but mostly doc comments.

[nija-at]

The summary section should briefly describe the solution as well.

Currently, this section reads as an 'introduction' or 'background' which I believe it's not.

[nija-at]

These are release notes, so remove some of the conditionals and use more direct language.

Suggested change

	An important thing to keep in mind is that changing a type declaration to a _type union_ is a breaking change for those languages that do not natively
	support _type unions_ (while this is not the case when updating a parameter's type in **TypeScript**). The `jsii-diff` tool can be used to
	automatically check that you incurred no such breakage on an API that was declared `@stable`. If you need to change a `@stable` API to support _type
	unions_ when it previously accepted only a single type, the recommended approach is to deprecate the current `@stable` API and introduce a new one
	that accepts or returns the _type union_.
	Changing a type declaration to a _type union_ from a single type is a breaking change. To use type unions on APIs declared `@stable`,
	the existing API be deprecated and a new API that uses _type union_s be introduced.
	The `jsii-diff` tool can be used to automatically check that you incurred no such breakage on an API that was declared `@stable`.

[nija-at]

Something like that maybe?

I don't think I got it right either. Try re-phrasing to be more clear.

Suggested change

	APIs of the CDK for **Java** and **.NET** where the **TypeScript** version is declared using **type unions** (e.g:
	`CfnBucket.LoggingConfigurationProperty | IResolvable`) have been updated to offer better type fidelity compared to the **TypeScript** version.
	APIs of the CDK for **Java** and **.NET** where the **TypeScript** source declared **type unions** as its returned type (e.g:
	`CfnBucket.LoggingConfigurationProperty | IResolvable`) previously returned a generic low fidelity type `Object`.
	The fidelity of the return types are now improved and are comparable to the ones offered in **TypeScript**.

[nija-at]

"if they are not provided with an API allowing to validate whether some instance is of a candidate type or not" - finding it hard to follow this

Can you re-phrase or provide an example?

[nija-at]

Nice! 😊

[nija-at]

Would be nice to split these out into sub-sections. I can see at least three -

1. support in jsii
2. adoption to the cdk
3. migrate existing type unions

And maybe bake time?

[nija-at]

so one of them will work and the rest will throw InvalidCaseException, correct?

For the sake of completeness, I would suggest adding an additional code block under each language showing how a type union will be used in each language and what possibilities of return types would be (i.e., what exception/error will be thrown)

[nija-at]

Python missing?

[nija-at]

I suppose these are not the actual implementation of these methods?

I would assume there would be 3 private members stored for each of the types, and the getters or as*() APIs would check the corresponding member and either return or throw. Is that right?

[rix0rrr]

The naive implementation of this in C# would look like (passing a union type as a struct parameter, which is very common because all L1s have that):

new CfnUserPool(..., new CfnUserPoolProps {
  UserNameConfiguration = UserNameConfigurationUnion.fromUserNameConfiguration(new UsernameConfiguration { 
    Field = ...
  })
})

Which is not great. We'll have to look at each language feature and see how we can support this with minimal syntactic noise.

I'd like to see added to this RFC: examples in every language we are planning to generate union types for (Java, C#, Go), for the following 3 cases:

• Passing a union typed value as a positional argument
• Passing a union typed value into a Props type
• Reading a union typed value as a return type (<-- we've mostly been focusing on this so far I believe)

[eladb]

@RomainMuller what's the status here?