aws-cdk-rfcs
aws-cdk-rfcs copied to clipboard
RFC: 193 Fixing type unions
- Tracking issue: #193
- Rendered version
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.
@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 likeBucket.fromCfnBucket(cfnBucket: CfnBucket): IBucket
, we need type safety that the properties of theCfnBucket
are actually what the types say they are, not some hiddenIResolvable
s. 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).
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 anas 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.
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 anas 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.
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).
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)
@RomainMuller what's the status here?