Improved typing for union types

[thegedge]

What does this PR do and why?

After 9 or 10 types in a union, we devolve into an IAnyType. This isn't particularly useful, and potentially even dangerous (in a typechecking sense, given the behaviour of any).

This PR makes union types become proper TS sum types for any number of member types given to the union. It also results in types that are much simpler to manage, since we don't have to "unroll" for each possible number of types. We've also brought similar improvements to types.enumeration.

Steps to validate locally

I added some new tests for union and enumeration types, and verified that the tests fail before this change and pass after.

[thegedge]

Although I can't quite explain it, this appears to also resolve #1525

On main:

On this branch:

[EDIT]
Same for #1664

Haven't dug deeper, but I'm wondering if it'll also knock off #1833

[coolsoftwaretyler]

Hey @thegedge - thank you again, so much, for this phenomenal work. I'm stoked to see how many issues it may resolve.

I left a few comments here. I have some minor suggestions and questions. Nothing blocking, so please feel free to either make those changes, or ask me to do it myself. I appreciate all the time you've already put into this.

As you mentioned, there are a few changes here that some folks may consider breaking. If anyone else sees this PR and has concerns, I'd love to hear that. We will bring it up at the Maintainer's meeting on March 7th. I expect we will release a pre-release version for folks to try out, but probably take our time with releasing it, to make sure we don't cause a lot of downstream churn for folks

[coolsoftwaretyler]

Although I can't quite explain it, this appears to also resolve #1525

On main:

On this branch:

[EDIT] Same for #1664

Haven't dug deeper, but I'm wondering if it'll also knock off #1833

Hey @thegedge - I can't verify this fix. I'm seeing the issue still in a test in bcac3c0. How did you get the passing screenshot?

[coolsoftwaretyler]

c2b47ee confirms the fix for #1664.

[coolsoftwaretyler]

47c0702 replicates #1833 - so I don't think this fixes that quite yet either, unless I've got something borked locally as in this comment

[coolsoftwaretyler]

Merge conflict came from #2152 since we're editing __tests__/core/type-system.test.ts in both. Let's resolve some of the other items before we resolve the conflicts. Should be able to pull from both current and incoming on that one, if I'm not mistaken.

[thegedge]

Oh yeah, great call on regression tests for the issues we fixed!

I'll double check on #1525 this evening

[coolsoftwaretyler]

@thegedge - I added a test for 1525, and it's failing in CI

We definitely don't need to block on this, but since it looks like you've seen the fix for 1525 at some point, would be cool to try and verify it.

Let me know whatcha think!

[coolsoftwaretyler]

I will remove that failing test and plan to merge this in sometime over the weekend unless I hear otherwise.

[thegedge]

Hey, @coolsoftwaretyler. Thanks for 👀. We have a big launch week coming up so I've been super busy and neglected to check in. If I can't get to it this evening, let's ignore and follow up with #1525 later.

[coolsoftwaretyler]

Perfect, thank you!

[thegedge]

Amended your test commit, was just missing an important check from the issue to ensure narrowing happens. In this PR, it passes!

🙇 for adding all these tests. I'll keep that in mind for future work.

[coolsoftwaretyler]

Phenomenal!

I'll merge and then put out a pre-release build to npm, along with an RFC about how strict we want to be around semantic versioning here.