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

noUncheckedIndexedAccess does not narrow properly with "prop" in obj #43614

Closed
andreialecu opened this issue Apr 9, 2021 · 8 comments Β· Fixed by #51653
Closed

noUncheckedIndexedAccess does not narrow properly with "prop" in obj #43614

andreialecu opened this issue Apr 9, 2021 · 8 comments Β· Fixed by #51653
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@andreialecu
Copy link

Bug Report

Apologies if this is a duplicate, but I wasn't able to find anything.

πŸ”Ž Search Terms

noUncheckedIndexedAccess property in object

⏯ Playground Link

Playground link

πŸ’» Code

declare function invariant(condition: boolean): asserts condition;

function foo(obj: Readonly<Record<string, string>>) {
  invariant(obj && "test" in obj);
  // this one below does narrow to `string`
  // invariant(!!obj["test"]);

  const x = obj["test"] // string | undefined but should be string
}

πŸ™ Actual behavior

x is typed as string | undefined

πŸ™‚ Expected behavior

x should be typed as string

@andreialecu
Copy link
Author

Seems related to #41848

@andrewbranch
Copy link
Member

This isn’t really related to noUncheckedIndexedAccessβ€”in narrowing doesn’t ever take undefined out of the domain of a property: https://www.typescriptlang.org/play?noUncheckedIndexedAccess=true&ts=4.3.0-beta#code/CYUwxgNghgTiAEA3W8D2AjAVgLngb3gBcQBnQgflzJgEsA7Ac3gF8BuAKHZoDN4AKAETEyA+PTRYAlPnbx4YVHTLwAHvAC8EzAG0hpQgIC6HOQqWF4ATw1aAdMMIdm7IA

You should check for equality / non-equality with undefined if you want that behavior.

@andrewbranch andrewbranch added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 9, 2021
@typescript-bot
Copy link
Collaborator

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

@andreialecu
Copy link
Author

Not sure I understand. The type of obj cannot have undefined values in it as per its definition.

How does undefined fit into Readonly<Record<string, string>>?

@andrewbranch
Copy link
Member

Ah, I see. When you set --noUncheckedIndexedAccess, the read type of property access on a Readonly<Record<string, string>> goes from string to string | undefined. But we don’t currently have the ability to model the difference between properties that are undefined and properties that are missing. If we were to introduce a missing type in the future, we would be able to implement the behavior you expect hereβ€”the read type from that index signature would be string | missing, and you could know that it’s not missing via an in test. But right now, because we lack the concept of missing, an in test on an object with an index signature does nothing.

@andrewbranch andrewbranch added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Apr 23, 2021
@andrewbranch andrewbranch reopened this Apr 23, 2021
@derolf
Copy link

derolf commented Sep 23, 2021

With exactOptionalPropertyTypes, unchecked index access could map to
missing and not undefined.

Then key in guarded access should work equally on optionally declared properties and index signatures.

Example highlighting the obvious disparity:

https://www.typescriptlang.org/play?noUncheckedIndexedAccess=true&exactOptionalPropertyTypes=true&ts=4.4.2#code/CYUwxgNghgTiAEA3W8D2AjAVgLngJXFRmAB4BnAFxgEsA7AcwBp5KaGA+AbgChRJYEyGGiwAmXAG8KISgH5crOvQC+3btQBm8ABQAiaZV3w6IzAEp4E7vHgB6W-A1RqEMtfhhUtSvACe8BSoleABeUwA6AwoeVXUtPSijEwxMUQsrG3t4AHciAGs3G09vCj8AliCGUNNRSJkKbligA

@KaelWD
Copy link

KaelWD commented Dec 10, 2021

This seems like a bug rather than a design limitation then.

@tskj
Copy link

tskj commented Nov 8, 2022

I agree that this is neither a design limitation nor working as intended. This is simply a bug and @derolf's example proves it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
6 participants