-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Writing a for-in loop variable with a proper subtype of string should require a type assertion #57288
Comments
You're like the first person I've seen in forever advocating that the key should be typed Anyway, I think this is a duplicate of #49882, which links to #3500 and suggests that this might be intentional. |
Thanks for that issue. This is definitely a dupe of #49882, though I think the title on this one is much clearer ("incorrect type using keyof and for..in" sounds like just another issue requesting the unsound behavior). |
Not quite :P see here (where you also left some comments in the past 😅 ) |
I also agree the variable should be typed as |
@Andarist -- wow, you have a PR for everything! :) @ssalbdivad the "escape hatch" would be an explicit type assertion: for (const kStr in abc) {
const k = kStr as keyof ABC;
} One more variable than the current form, yes, but the same number of lines of code and it's at least more honest about what's going on. let k: keyof ABC;
for (k in abc) {
} |
I do my best 😉
I also think that this is an acceptable escape hatch (the cast I mean). It's at least honest about its nature so the reader can understand better what happens here. |
@danvk @Andarist It's obviously subjective, but the first feels much worse to me:
It also feels like the upside is very limited:
type A = {
a: true
isA: true
}
type B = {
b: true
isB: true
}
const aOrB: A | B = {
a: true,
b: true,
isB: true
}
if ("a" in aOrB) {
// narrowed to A
aOrB.isA
// ^?
} Calling this behavior an anomaly in the first place is a bit of a stretch because it's an anomaly consistent with many other existing "anomalies" which are clearly behaving as intended. That said, barring the implementation of exact types, I'd be fine with the for (const k as keyof ABC in abc) {
...
} |
This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
🔎 Search Terms
🕗 Version & Regression Information
Object.keys
(https://stackoverflow.com/questions/55012174/why-doesnt-object-keys-return-a-keyof-type-in-typescript)⏯ Playground Link
https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgIICEDCyDeBYAKGWOTgC5kQBXAWwCNoBuQk5Oi6+plkhD2hlGYEAvoUIwqIBGGAB7EMhhy5ACjh0+aLAEpcPYgBsIYZAGsKZiAE85MbZmGtlUZKrPJQpTXvxFWvAoAznLGAHSGcgDm6poA2mYAumFgcgBiwAAeEAAmqjo6TiRiouIECMGmGgg5yAC8uOTIAIwANGwUAEztWgDM7TkcEADuyAAicJD5IsLKatU5hcQA9MvIgLwbgKU73ggJyakZ2bXAQZRyVUpSMvKKhEA
💻 Code
🙁 Actual behavior
TypeScript allows this code without a type assertion on
k
.🙂 Expected behavior
TypeScript should require a type assertion on
k
, whose type really should bestring
. Something like:The code it allows is equivalent to a type assertion but does not use the word "as".
Additional information about the issue
This has been the behavior of TS forever, but I had trouble finding an issue requesting that TS be more strict about this, rather than less.
If you want to shoot yourself in the foot, you should at least have to write a type assertion somewhere.
Incidentally, the error you get if you write
let k: "a" | "b"
is a bit misleading given the behavior:string
would be sound, but evidently TS only requires something thatkeyof ABC
is assignable to, notstring
.The text was updated successfully, but these errors were encountered: