-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Narrowing for key type by in
operator
#43284
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
Comments
I was surprised that Typescript doesn't do this already |
I guess this would also solve the issue of this code throwing an error.
|
Note that such narrowing is technically unsafe (although the current narrowing in #10485 is also technically unsafe for the same reason): function f(v: { a: string }, k: string) {
if (k in v) {
// k is now "a" by #43284, so
v[k].toUpperCase();
}
}
const v = { a: "hello", b: 123 };
f(v, "b"); // kaboom This is maybe fine, but people should be aware of it. Also, the suggestion in #21732 (edit: implemented by #50666) and this suggestion acting in concert could do weird things, since on the one hand we'd be extending declare const k: "bar";
declare const v: { foo: 0, baz: 1 };
if (k in v) {
// k is now never by 43284
// v is now {foo: 0, baz: 1, bar: unknown} by 50666
} Maybe there's some reasonable heuristic here? Like, if |
@jcalz Yes, of course, but to make it correct TS would need to distinguish closed and open object types, like Flow did. Right now interface A { a(): string };
interface B { b(): string };
function f(x: A | B): string {
if ("a" in x) {
return x.a(); // x.a is not a function
} else {
return x.b();
}
}
const x = { a: 10, b() { return "hello"; } };
const y: B = x;
f(y); Edit. Of course, you said exactly that, and I somehow missed it. |
Want to point out that this is even worse than that: Assuming that |
Seeing how #48149 was declined, should this one be declined as well? Or should the other be undeclined but a duplicate of this? Just trying to understand where this feature request stands. |
Is there anything preventing this be implemented for |
hello what's the consensus on this? typescript currently suggests we use type-guards in these situations which to me feels no better than type casting |
Current consensus is that this behavior would be wrong and so has a pretty high bar to justify changing it.
This is wrong; just because you have a type originating in a const a = { x: 32 } as const;
const b = { x: 32, y: "hello" } as const;
const c: typeof a = b;
const s = Math.random() > 0.5 ? "x" : "y";
if (s in c) {
// Alleged: s is safely "x" here
// Reality: This branch is always hit, s could be "y"
} |
🤦 Should we then wait for |
Suggestion
A
k in v
check should narrow type ofk: K
toK & keyof typeof v
.🔍 Search Terms
in operator narrowing
✅ Viability Checklist
My suggestion meets these guidelines:
⭐ Suggestion
This code should compile due to narrowing on
x
:Currently it could be made with custom type guard
📃 Motivating Example
When validating a JSON with some kind of AST (or otherwise containing a tagged union) the only type we can have for a tag is a
string
.💻 Use Cases
in_
solves the issue, but it involves a type guard. Type guards are "dangerous" in a sense that they can be used improperly (for example,!(key in object)
would compile as well, but would lead to runtime error).There is a related feature request to narrow type for second parameter of
in
operator. As far as I'm aware, there is no syntax to specify type ofin_
that would guard on both parameters at the same time.Edit. Not even
The text was updated successfully, but these errors were encountered: