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

in operator narrowing (key in obj) doesn't work properly when key type is a string union #55561

Open
barroudjo opened this issue Aug 29, 2023 · 8 comments
Labels
Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases

Comments

@barroudjo
Copy link

barroudjo commented Aug 29, 2023

πŸ”Ž Search Terms

in operator narrowing union

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type narrowing

⏯ Playground Link

https://www.typescriptlang.org/play?ssl=39&ssc=4&pln=20&pc=1#code/MYewdgzgLgBAYgeQQfQNIFECaBlGBeGAbQHIAzYgGhmJGIF0YBDCGUSKAbgFgAoN6eEjRZsyBADkAMpjHj0+ImXpMW-TrwCWYKAFMATqUbAd8ECBgBvXjBsxSALhjQ9WgObcetmCEfO3HgF9eXjUYXQECAAotAAcAVyhHODMYAB9LAIBKfAA+S2tbRBQMHAA6UhA9dCMAC0jIgGsdAE9svDyrTy9bDVIYRpaYLSGweKhs-hAAGx1SqZBXaNGEwibmukyPLyytwqES0QlpWXRyyurgOoHW3Pyu7qG+65GRsYnwCGnZ+cXYlbWNrsbDteAEPMEeAB6SFhPTNNxhcx6HQxPQgAAmcWMYRqJlRIAARjMALYwADuGigNScGmJMRmehgcQgJmAzB0EHsEOhrHZMAAjCEPrADvyFCRGJRqATlMxWMKPKFCQArMUECyMRzESVUAlamVg3iis5VWrPdp3Ly9fprF4q-nvSBfOYLSL21YtQGgzbcmFslkwABMQvYMAOgfF2qlxBlDDlakVwu8BOVEfVmuoksNPHDJoudVtFs6VqetuGKsDjs+MxdiwrHvWmxgPIAKrh9Gi9CwybiwENYBAaiA4lN0WBiFAqHKySZGMimRAEQADLRLmBgOdoilgVxOcxUxiwKkmGPEGD4mL6KDNJjAYwQFjokAc8ewGqMGKXsDe8FQv18gBmEMBAOADI1jFR5XYRNQxVMD0y1LMOGbGFQJgJ8OXXEBYDYKBGGGRgwBvNYWBAPo4KNEQALzM1Cw6AobGtZ5yxTACq2dH43VYhtARQx4YDbGAO0qFhhn9ExAyoY8+0pJwhxHdEYFxecoBSAkEn7dDnwgV8YAAQgMn8gA

πŸ’» Code

// case 1
const KEYS1 = ['a', 'b'] as const;
const obj1 = {a: 'a', b: 'b'};
KEYS1.forEach(key => {
    if (key in obj1) console.log(obj1[key]);
});

// case 2
const KEYS2 = ['a', 'b'] as const;
const obj2 = {a: 'a'};
KEYS2.forEach(key => {
    if (key in obj2) console.log(obj2[key]); // TS errors when it shouldn't, as we are using `in` narrowing so that the 'b' property access doesn't happen
});

// case 3
const KEYS3 = ['b'] as const;
const obj3 = {a: 'a'}; // KEYS3 does not contain any keys of obj3
KEYS3.forEach(key => {
    if (key in obj3) console.log(obj3[key]); // if TS errors in case 2, then it should here too but it doesn't !!!
});

πŸ™ Actual behavior

  • in case 2 TS errors when it shouldn't, as we are using in narrowing so that there is no incorrect 'b' property access
  • in case 3 TS doesn't give any errors, when consistency disctates that it should if it did in case 2...

πŸ™‚ Expected behavior

Typescript shouldn't detect any errors in case 2. The type narrowing with the in operator ensures that we are only accessing properties that actually exist.

Additional information about the issue

No response

@jcalz
Copy link
Contributor

jcalz commented Aug 29, 2023

Possible duplicate of #43284, since you seem to be trying to narrow the key and not the object.

in operator narrowing is heuristic in nature and consistency isn't the main driving factor in how it works. You have #15256 and #50666 which narrow the object type in different ways, none of which do anything to the key.

@andrewbranch
Copy link
Member

I think this is pointing out an implementation detail of #50666 where objects get narrowed with in if the LHS is a string literal but not if the LHS is a union of two string literals, and it doesn’t look obvious at first glance why those can’t be treated similarly. Minimal repro: https://www.typescriptlang.org/play?#code/CYUwxgNghgTiAEEQBd4HsBGArAXO74yA3ALABQoksCSqaAdiANIgCeeA5FB6ReNHEQp0jAPIAzACoB3NC1YBnTt3gAfeBww9y5AJbj4ACgbM28XfXxYAlPADe5eE6sA6KLwC+eg8bFTZ8grmlpg29o7OoW68zq4YnuRAA

declare let obj: object;
declare let oneKey: 'a';
declare let oneOfTwoKeys: 'a' | 'b';

if (oneKey in obj) {
    obj.a; // ok
}
if (oneOfTwoKeys in obj) {
    obj.a; // error
    obj.b; // error
}

I think the consistency that @barroudjo would like to see doesn’t necessarily hinge on narrowing the key type. You could imagine the second if statement narrowing the type of obj to Record<"a" | "b", unknown>β€”or { a?: unknown, b?: unknown } to avoid issues with exactOptionalPropertyTypes. I’m not sure if there’s a reason why something like this wasn’t considered... getting the feeling I could be missing something obvious though.

@andrewbranch andrewbranch added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Aug 29, 2023
@fatcerberus
Copy link

fatcerberus commented Aug 29, 2023

I'm actually surprised that oneKey in obj narrows at all, tbh. I always assumed the rule was that the LHS had to be an actual literal, not just literal-typed, as is the rule elsewhere:

declare let o: { foo?: string };
declare let k: "foo";

if (o[k]) {
    let v: string = o[k];  // error
}

@andrewbranch
Copy link
Member

andrewbranch commented Aug 29, 2023

That example narrows if k is a const instead of a let πŸ˜‰

That only strengthens the underlying point that our logic around this is not super consistent.

@barroudjo
Copy link
Author

Yeah my point is indeed one of consistency. I can't fathom why it should work for a string litteral, but not a union of string litterals.

@barroudjo
Copy link
Author

Sorry to bump it, but I'd like to know if this is a bug, or an expected behavior of typescript.

If it's a bug I'll just use a hack and keep the code as close to this as possible, with a TODO comment explaining the situation and that we're waiting for the bug to be solved.

If it's the expected behavior, then I'll rack my brains to find a different solution ;-)

@andrewbranch andrewbranch added Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases and removed Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Sep 5, 2023
@andrewbranch
Copy link
Member

It’s a Possible Improvement. I wouldn’t wait on a solution if I were you.

@barroudjo
Copy link
Author

@andrewbranch thanks for the answer !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Projects
None yet
Development

No branches or pull requests

4 participants