Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aliasing discriminated union tag breaks case analysis #10830

Closed
Swatinem opened this issue Sep 9, 2016 · 5 comments · Fixed by #46266
Closed

aliasing discriminated union tag breaks case analysis #10830

Swatinem opened this issue Sep 9, 2016 · 5 comments · Fixed by #46266
Labels
Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@Swatinem
Copy link
Contributor

Swatinem commented Sep 9, 2016

TypeScript Version: 2.1.0-dev.20160909

Code

type A = {t: "a", a: string};
type B = {t: "b", b: string};
type U = A | B;

const a: U = {t: "a", a: "a"} as U;

if (a.t === "b") {
    a.b; // works as expected
}

const {t} = a;
if (t === "b") {
    a.b; // error TS2339: Property 'b' does not exist on type 'U'.
}

Expected behavior:

The type of a.t should correctly transfer to t when assigning or using destructuring. t should be usable as discriminant.

Actual behavior:

The type of t is widened from "a" | "b" to string and therefore can not be used as discriminant any more.

As the same happens also for flow (See facebook/flow#2409), it may be my thinking that is a little bit off. But I think that the case should be valid. The programmer should not be forced to repeat base. all over, that’s what variables are for.

@RyanCavanaugh
Copy link
Member

The type of a.t should correctly transfer to t when assigning or using destructuring. t should be usable as discriminant.

Tracking this correctly would require that variables "remember where they came from" and what implications that has on other variables. It'd probably be possible/reasonable to make that work at 1 level of indirection (rather than at 0 levels of indirection), but tracking it for N levels of indirection would be very complex, and then presumably you'd be surprised when it worked at 1 level of indirection but not 2.

So given the choice between "be surprised at 2 and things get way more complex in the compiler implementation" vs "be surprised at 1", we're generally sticking with the latter.

@mhegazy mhegazy added the Too Complex An issue which adding support for may be too complex for the value it adds label Sep 9, 2016
@mhegazy mhegazy closed this as completed Sep 9, 2016
@ligaz
Copy link

ligaz commented Sep 15, 2016

What about the following case:

const { type, payload } = action;
  switch (type) {
    case 'SOME_STRING':

Is it too complex or there is possibility that you can add support for it?

@mhegazy
Copy link
Contributor

mhegazy commented Sep 15, 2016

@ligaz this already works as intended:

declare const action: { type: "A", payload: number } | { type: "B", payload: string };


const { type, payload } = action;
switch (type) {
    case 'SOME_STRING': // "SOME_STRING"" is not compatible with "A" | "B"

}

@ligaz
Copy link

ligaz commented Sep 16, 2016

The problem is that the payload is not typed if the tag is matched:

declare const action: { type: "A", payload: number } | { type: "B", payload: string };

const { type, payload } = action;
switch (type) {
    case 'A': // payload type is 'number | string' not 'number'.

}

@RyanCavanaugh RyanCavanaugh added the Suggestion An idea for TypeScript label Sep 16, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 16, 2016

There is no way to know that there is a relationship between the two variables.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Oct 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants