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

Incorrect narrowing on partial of discriminated union #55578

Closed
scottmas opened this issue Aug 30, 2023 · 10 comments
Closed

Incorrect narrowing on partial of discriminated union #55578

scottmas opened this issue Aug 30, 2023 · 10 comments
Labels
Duplicate An existing issue was already created

Comments

@scottmas
Copy link

scottmas commented Aug 30, 2023

🔎 Search Terms

Googled "partial discriminated union typescript not narrowing correctly"
Searched repo github issues "discriminated unions partial narrow"

Nothing showed up that looked relevant.

🕗 Version & Regression Information

5.2.2

⏯ Playground Link

https://www.typescriptlang.org/play?#code/C4TwDgpgBAggzgEwGZQLxQN4CgpVJALigCIBDRJYgbhynOSIEYAmAZhoF8at9oBFAO4QATmky1eRYgEchw6rVkimbTtwDGAewB2cYFABGAG1IALIgAVSw4AEtSRgDzxkUAD5RBIgHxjtAVyMjOjg6bRBuWxQACgBCYzMAOl4ASnFcAHoMhNMoW1CIAFswUChNAwArCHVgLA4oCCM4aCioaJzk8GhUHpJ6SjTsTOyTXPyylF5YCjqGpughqCycqDhTTUCEQ2hNSa7POVoOpWEqJYyAUWFhTWE4WLqgA

💻 Code

type Asdf = {
  type: "asdf";
  asdf: 123;
};

type Qwer = {
  type: "qwer";
  qwer: 123;
};

const blah: Partial<Asdf | Qwer> = null as any;

if (!blah.type) {
  //blah is empty object
} else if (blah.type === "asdf") {
  //blah is of type Asdf
} else {
  //blah should be of type Qwer
  blah.qwer; //Errors!
}

🙁 Actual behavior

blah.qwer throws a compiler error.

🙂 Expected behavior

blah is not narrowed as it should be. The first if condition verifies that the type is not undefined. Therefore by the time blah.qwer is accessed, we should be able to have narrowed the type correctly down to Qwer.

Additional information about the issue

No response

@andrewbranch
Copy link
Member

I think what’s happening is there are two separate narrowings going on here: one for blah.type and one for blah.

In if (!blah.type), you aren’t narrowing blah at all because both object types in the union could have an undefined type.

In else if (blah.type), this is a discriminant check, so we can actually narrow blah inside the block.

So in the else block, we are part of two separate control flow narrowing graphs. When we get the type of blah, we are only looking at branches that affected the type of blah—so the else if branch. And taken alone, the negative of blah.type === "asdf" doesn’t help us narrow, because blah.type could be "qwer", but it could also be undefined. The information that blah.type is not undefined isn’t in the antecedents we’re looking at because it didn’t affect the type of blah in the first place.

I have a feeling this is a known design limitation, but I’m not 100% sure.

@scottmas
Copy link
Author

I'm not sure I follow you 100% but look at the following snippet. It seems like test1 and test3 should pass:

type Asdf = {
  type: "asdf";
  asdf: 123;
};

type Qwer = {
  type: "qwer";
  qwer: 123;
};

type PartialUnion = Partial<Asdf> | Partial<Qwer>

type TypeNarrowedPartialUnion = (Omit<Partial<Asdf>, 'type'> & Pick<Asdf, 'type'>) | (Omit<Partial<Qwer>, 'type'> & Pick<Qwer, 'type'>)

const blah: PartialUnion = null as any;

if (blah.type) {
  //Conceptually, blah should now be of type TypeNarrowedPartialUnion but this errors :(
  const test1: TypeNarrowedPartialUnion = blah; 
  
  if (blah.type === "asdf") {
    const test2: Partial<Asdf> = blah; //Huzzah!
  } else {
    const test3: Partial<Qwer> = blah; //Errors :(
  
  }
} 

https://www.typescriptlang.org/play?#code/C4TwDgpgBAggzgEwGZQLxQN4CgpVJALigCIBDRJYgbhynOSIEYAmAZhoF8at9oBFAO4QATmky1eRYgEchw6rVkimbTt15QACqWHAAlqQA2AVQB2egPamx23QcMAeeMgB8UAD5ad+ow8EiXLB5waAAVEIA5HWELIQRbHxNzKzEACgB5AFs9YAcE+ycKFwAaKAByXjK3ADItPQBjAGtC5FKKkKqASg8oDOzc-N9-YRLyypq6pr85NvHOoPqrOGAoACNDUgALIkGky2t0UwBXQ0M6ODpTEG49FFT1rYA6Xm7sXAB6d4BhK3qIMGARyMhhApQemygcE2FhOCCgplia2gFhQGnCkCiwhicV2Zn2ayOK2Amz0FxEMWEFwIqVoi1MyzwEGWjCI6IgmOxEHi3nseJS6HBVCgtFot164OeITQqHQZAoxFetFwdIZwCZwGYOx5vmcSDcAo2myFnwAEkcAF7mra0DhQCCGODQN64KAqonq1hauxDOT6taG43vACiWIslKg1JFuA4WFtQA

@andrewbranch
Copy link
Member

That’s even farther afield from how narrowing works in TypeScript—this second example is definitely working as intended / design limitation. Only unions are narrowed by property checks, and they only filter out constituents. The only time checking a property affects narrowing of the object type that property was accessed on is when that property is a discriminant of a discriminated union. When you check if (blah.type) here, that’s not enough information to filter out either constituent of the union Partial<Asdf> | Partial<Qwer>, so you haven’t affected the type of blah at all.

@scottmas
Copy link
Author

scottmas commented Aug 31, 2023

But it seems like after checking if (blah.type) the narrowed type truly SHOULD be TypeNarrowedPartialUnion, namely (Omit<Partial<Asdf>, 'type'> & Pick<Asdf, 'type'>) | (Omit<Partial<Qwer>, 'type'> & Pick<Qwer, 'type'>). We have gained information about blah by checking if blah.type exists. I do grant that we can't filter out any of the constituents. But we should be able to transform the constituents based on the information we've gained.

Am I seeing things incorrectly though? I've been wrong before on my set theory reasoning, but the transformation like I described truly does seem like the most logical thing to do.

@fatcerberus
Copy link

fatcerberus commented Aug 31, 2023

But we should be able to transform the constituents based on the information we've gained.

That would be #42384 and is a whole different can of worms. Type narrowing currently only works by eliminating entire constituents of a union, and that process is atomic (there are no intermediate states).

@andrewbranch
Copy link
Member

I think that reasoning is sound; I’m only saying that it’s very different from how TypeScript works. Narrowing a union only filters constituents.

@andrewbranch
Copy link
Member

Ah yeah, I came across that issue when I was looking for duplicates, but I disregarded it because when I read “type guard” I thought it was referring to user-defined type guards, which is a very different narrowing path. This does look like a case of that issue.

@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Aug 31, 2023
@fatcerberus
Copy link

I tend to think of "type guard" as the thing that actually induces narrowing (an if condition, e.g.); while the implementation of a UDTG is a "type predicate".

@andrewbranch
Copy link
Member

Huh, going only by SyntaxKind.TypePredicate, only the foo is SomeType bit is the type predicate 🤷

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants