GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Relate source types covered by a target discriminated union by rbuckton · Pull Request #30779 · microsoft/TypeScript

Conversation

@rbuckton

@rbuckton Ron Buckton (rbuckton) commented Apr 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This PR effects how we relate a source "Object" type to a target union type when:

  1. The source is not relatable to a single type in the target union.
  2. The target union has discriminant properties.
  3. The source type would be sufficiently covered by all constituents of the union.

For example, today if we were to change IteratorResult to be a discriminated union, the following example would no longer compile:

type IteratorResult<T, TReturn = T> =
  | { done: false, value: T }
  | { done: true, value: TReturn };

class NumberIterator {
  next() {
    return { done: false, value: 1 }
  }
}

let x: Iterator<number> = new NumberIterator();
// error TS2322: Type 'NumberIterator' is not assignable to type 'IteratorResult<number>'.
//   Type '{ done: boolean, value: number }' is not assignable to type '{ done: true, value: number }'.
//     ...

With this change, we will perform the following steps after all other relationship checks fail:

If 'source' is an Object type and 'target' is a Union type with discriminant properties:

  1. Generate a matrix of combinations of discriminant properties and types 'source' can satisfy.
    1. If the number of combinations is above a fixed limit (currently 25), we consider the comparison too complex and determine that 'source' is not related to the 'target'.
  2. Filter 'target' to the subset of constituents whose discriminants exist in the matrix.
    1. If any combination of types in the matrix does not have a match in 'target', we determine 'source' is not related to 'target'.
  3. For each matching constituent of 'target', we relate the remaining non-discriminant properties of 'source' to 'target'.
    1. If any remaining properties are not related, we determine 'source' is not related to 'target'
    2. Otherwise, 'source' is sufficiently covered by all constituents of 'target' and they are related.

This means that now, for the example above, we perform the following steps:

type S = { done: boolean, value: number }
type T = { done: false, value: number } // T0
       | { done: true, value: number }  // T1
  1. Generate the a matrix of combinations:
    1. Determine the discriminants d of S with respect to T: ["done"]
    2. Determine the discriminant types dt of d: [[true, false]]
    3. Generate the matrix of combinations c: [[true], [false]]
  2. Filter T to the matching discriminants cm of c: [T0, T1]
    1. Determine that cm has at least one match for each combination in c.
  3. Relate the remaining non-discriminants (["value"]) of S and cm.
    1. Determine that S["value"] is related to both T0["value"] and T1["value"], therefore S is related to T.

Further examples can be found in assignmentCompatWithDiscriminatedUnion.ts

Related: #2983
Fixes: #14865
Fixes: #30871
Fixes: #30170
Fixes: #12052
Fixes: #18421
Fixes: #15907
Fixes: #20889

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

niieani, bradenhs, adidahiya, sutarmin, and gitowiec reacted with thumbs up emoji
All reactions
  • 👍 5 reactions
@rbuckton
Ron Buckton (rbuckton) requested review from Daniel Rosenwasser (DanielRosenwasser), Ryan Cavanaugh (RyanCavanaugh), Anders Hejlsberg (ahejlsberg), Nathan Shively-Sanders (sandersn) and Wesley Wigham (weswigham) April 5, 2019 19:19
@rbuckton

Ron Buckton (rbuckton) commented Apr 5, 2019

Copy link
Copy Markdown
Contributor Author

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ron Buckton (@rbuckton), I've started to run the Definitely Typed test suite on this PR at 645853a. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ron Buckton (@rbuckton), I've started to run the extended test suite on this PR at 645853a. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rbuckton

Ron Buckton (rbuckton) commented Apr 6, 2019

Copy link
Copy Markdown
Contributor Author

TypeScript Bot (@typescript-bot) perf test

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 6, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ron Buckton (@rbuckton), I've started to run the perf test suite on this PR at c712918. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@microsoft Microsoft (microsoft) deleted a comment from TypeScript Bot (typescript-bot) Apr 6, 2019
@microsoft Microsoft (microsoft) deleted a comment from TypeScript Bot (typescript-bot) Apr 6, 2019
@rbuckton

Ron Buckton (rbuckton) commented Apr 6, 2019

Copy link
Copy Markdown
Contributor Author

Pardon the dust, tinkering with the on-demand benchmark script to not generate a massive wall of text when posting the results.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rbuckton Ron Buckton (rbuckton) mentioned this pull request Apr 6, 2019
Improve type checking and inference for Generators and Async Generators #30790
Merged
weswigham
Wesley Wigham (weswigham) approved these changes Apr 6, 2019
View reviewed changes
@jack-williams Jack Williams (jack-williams) mentioned this pull request Apr 11, 2019
Indexed Access Types that should be the same as a given types are somehow different than that type, under strictNullChecks #30871
Closed
@weswigham Wesley Wigham (weswigham) mentioned this pull request Apr 12, 2019
1-tuple of union should be the same as union of 1-tuples #30895
Closed
5 tasks done
@rbuckton
Ron Buckton (rbuckton) force-pushed the relateDiscriminants branch 2 times, most recently from 04d985e to 8321adf Compare April 18, 2019 22:53
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Apr 19, 2019

Copy link
Copy Markdown
Member

Possibly fixes? #30170 #12052 #18421 #12052 #15907

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Apr 19, 2019

Copy link
Copy Markdown
Member

Also #20889

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rbuckton
Ron Buckton (rbuckton) force-pushed the relateDiscriminants branch from 8321adf to 332ea0a Compare April 19, 2019 19:41
@rbuckton

Ron Buckton (rbuckton) commented Apr 19, 2019

Copy link
Copy Markdown
Contributor Author

Ryan Cavanaugh (@RyanCavanaugh): I've added tests and verified that this does indeed fix those issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rbuckton

Ron Buckton (rbuckton) commented Apr 19, 2019

Copy link
Copy Markdown
Contributor Author

Anders Hejlsberg (@ahejlsberg): Can you take a look?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ahejlsberg
Anders Hejlsberg (ahejlsberg) approved these changes Apr 29, 2019
View reviewed changes

@ahejlsberg Anders Hejlsberg (ahejlsberg) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd inline the constant instead of shouting all caps.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@rbuckton
Ron Buckton (rbuckton) force-pushed the relateDiscriminants branch from 332ea0a to db7f701 Compare April 29, 2019 21:59
@rbuckton
Relate a source type that is sufficiently covered by a target discrim…
26fd6da
…inated union
@rbuckton
Ron Buckton (rbuckton) force-pushed the relateDiscriminants branch from db7f701 to 26fd6da Compare April 29, 2019 22:46
@rbuckton
Ron Buckton (rbuckton) merged commit 2d8527f into master Apr 29, 2019
@rbuckton
Ron Buckton (rbuckton) deleted the relateDiscriminants branch April 29, 2019 23:58
@jcalz

Joe Calzaretta (jcalz) commented May 30, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Hmm, why doesn't #12052 seem to link back to this issue? Oh, does locking an issue actually prevent it from getting new "@somebody mentioned this" links? That's too bad... it becomes much harder to follow up on these longstanding issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joehillen Joe Hillenbrand (joehillen) mentioned this pull request Jun 21, 2019
[react] add tests for useState DefinitelyTyped/DefinitelyTyped#36338
Merged
6 tasks done
@jack-williams Jack Williams (jack-williams) mentioned this pull request Sep 4, 2019
Union of partial types: common super type of all union members is not assignable to union type #33243
Closed
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Apr 27, 2020
Type not assignable for string literal in signature #38201
Closed
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Jul 1, 2020
Equivalence between tuple containing union type and union type of tuples #39357
Closed
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Jul 2, 2020
Fix for relating covered discriminants in unions #39393
Merged
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Jul 29, 2021
Partially overlapping union discriminators are not assignable from a union of discriminating values #45230
Closed
@dpohvar Andrey Vakhterov (dpohvar) mentioned this pull request Apr 28, 2023
extends operator with literal types #54050
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Mar 6, 2024
Variable loses type narrowing when passed as object literal #57013
Closed
@jcalz Joe Calzaretta (jcalz) mentioned this pull request May 4, 2024
Error on types when using multiple conditional properties #58432
Closed
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Jun 25, 2024
Hex Colors error with TS2590: Expression produces a union type that is too complex to represent #59015
Closed
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Aug 22, 2024
Interface type coercion fails with unsatisfied properties in unions #59716
Open
@jcalz Joe Calzaretta (jcalz) mentioned this pull request May 22, 2025
Unexpected differences in distribution over union types: type string | 5 behaves differently than type string | [5] #61753
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswigham Wesley Wigham (weswigham) weswigham approved these changes
@ahejlsberg Anders Hejlsberg (ahejlsberg) ahejlsberg approved these changes
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Awaiting requested review from DanielRosenwasser
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Awaiting requested review from RyanCavanaugh
@sandersn Nathan Shively-Sanders (sandersn) Awaiting requested review from sandersn

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

error casting interface union field Union of two interfaces rejects keys that should be acceptable Wrong union type matching Unions within/without types should be hoisted/canonicalized to improve type compatibility cannot instantiate a tagged union whose discriminant property is union type
Union type is not compatible to second union type, even with full overlap. Indexed Access Types that should be the same as a given types are somehow different than that type, under strictNullChecks

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@rbuckton @typescript-bot @RyanCavanaugh @jcalz @weswigham @ahejlsberg
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.