aws-cdk-rfcs icon indicating copy to clipboard operation
aws-cdk-rfcs copied to clipboard

RFC: 193 Fixing type unions

Open RomainMuller opened this issue 3 years ago • 7 comments


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

RomainMuller avatar Jul 07 '20 11:07 RomainMuller

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

mergify[bot] avatar Jul 07 '20 11:07 mergify[bot]

@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).

RomainMuller avatar Jul 14 '20 11:07 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.

skinny85 avatar Jul 14 '20 16:07 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.

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.

RomainMuller avatar Jul 16 '20 08:07 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).

RomainMuller avatar Aug 03 '20 10:08 RomainMuller

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)

rix0rrr avatar Aug 17 '20 08:08 rix0rrr

@RomainMuller what's the status here?

eladb avatar Mar 10 '21 11:03 eladb