-
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
Explicit any type in for..in loop #3500
Comments
@JirkaDellOro I agree. I'd also like to be able to explicitly type it as 'string'. |
@JirkaDellOro it is implicitly typed as @CyrusNajmabadi, |
This year, I chose TypeScript to teach programming in the first year at the university. Up to now, we were using Java/Processing. I really enjoy TypeScript, since it offers access to the huge world of Javascript-Platforms while maintaining a proper coding style. Since the students may switch to other languages later, I enforce type annotations to get them acquainted to explicit types and for practice. All is well, except that I need to avoid the for..in loop, since the inconsistency of not being able to use annotation here, not even with the any-type, is not explainable and confusing! So the value I'd get is mainly didactical, plus we would all use a more consistent TypeScript. Let me also turn the question around: what is the value of disallowing annotation here? |
the type is decided by runtime. it is the keys on the object you are passing. The TypeScript type system is structural, so you could have something like: interface MyObj {
name: string;
}
function f(o:MyObj) {
for(var x in o) {
}
}
f({"name" : "value"}); // x is string
f({name: "", [Symbol.Iterator]: ()=> {}); // x is string|symbol So the compiler can not really provide any sort of safety here, we do not know what other keys are on the object you are iterating on. Originally TypeScript did not have union types, so the only correct annotation was |
just to reiterate.. if you add a type annotation on a variable say, the compiler will make sure that any assignments to this variable have types that are compatible; for the for..in, it does not matter what type annotation you use, it does not gives you any grantees, it all depends on values at run time, and the compiler has no way of knowing them. on a related note, i think requiring an annotation of |
Dan closing this brought the issue back to my attention. As opposed to Cyrus, I'm not talking about a type other than "any". If TypeScript implicitly uses the any-type here, I should be allowed to annotate that type, and only that, explicitly. I don't see the reason to say: "You are not allowed to type four more characters, even if you really want to, since you don't add value". Consequently, you could disallow comments with the same reason, since the user may also add no value, or even create confusion. As I mentioned earlier, disallowing the annotation of "any" here takes away value in terms of consistency of the language and didactic. Same may be true for the catch. It would be a different story, if TypeScript internale uses different types in the for..in loop but "any". |
As Mohamed mentioned, this is consistent with our treatment of variables in I get that you feel there's some pedagogical value in teaching the version of TypeScript where type inference is unused and everything is explicitly annotated but that's not a world we're going to bend language rules around or expend much effort to support. Consistency is great, but so is clarity, simplicity, and succinctness. |
Thanks for the clarification, Dan. I can perfectly understand, that you need to decide where to invest your energy. And I'm very aware that my case appears somewhat special. Assumably, I'm just one of a few using TypeScript to teach programming to beginners. But I think it would be great for TypeScript when our number grows, so it might be worth the support. I hope you don't mind that I keep discussing this and you consider this an interesting debate. And yes, the comparison with comments is sure funny, but it was geared towards the judgement of "adding value". That is a matter of perspective. Now, let's look at this little example: interface ITest {
[key: boolean]: string;
} TypeScript tells me: "An index signature parameter type must be 'string' or 'number'." PERFECT! I got the syntax right and consistent, but there is a logical mistake that I can easily fix. We have a limitation of possible types. Well, what works for two types should work for one as well. Using a type annotation, if I used one at all, other than 'any' in a for..in-loop or a catch clause, should yield something like: "... only supports the annotation of type any" or so. That would be clear, simple and consistent without a loss of succinctness, since I don't have to annotate (as usual in TypeScript)! Instead, disallowing the regular syntax in these special cases is bending the language rules around, isn't it? Making a logical problem a syntactical one unneccessarily adds complexity. And unfortunately, there already has been effort expended to do so... When it comes down to a matter of effort, time and resources, I'd be happy to try to help out together with my more advanced students. |
Always happy to discuss options :) From a user experience point of view certainly there is a fine line between these: try {
// error: Catch clause variable cannot have a type annotation
} catch(e: any) {
}
try {
// error: Catch clause variable can only be of type 'any'
} catch(e: string) {
} in that both are perfectly specific about both the error and how to correct it. From an implementation perspective the former is slightly faster since it's merely a grammatical check while the latter requires checking the type of the annotation. Likewise for the for...in case of course. On one hand we could say the former is preferable since if there's only one valid way to write something (ie a catch/loop variable must be of type |
Thank you for sharing those aspects, let me look at them one by one: "the former is slightly faster since it's merely a grammatical check while the latter requires checking the type of the annotation" "On the other hand it's not unreasonable to say there's value in consistency and always allowing a type annotation and simply reporting errors for invalid types." "we're generally catering to folks who have code without type annotations today (existing JavaScript devs), some of whom find type annotations to be onerous/noise/annoying." "Sometimes when you allow people to do something they'll do it even when it's not precisely in their own best interest (ex a game designer making a game where the winning strategy is the least fun strategy)." Getting back to the target group and wrapping it up: While, as you mentioned, some Javascript-Developers consider TypeScript or some of its features annoying or unnecessary overhead, I'm convinced that "classical"-programmers really do appreciate it. Up to the point, that some use TypeScript to educate the next generation. Continuing to cater to them will sure be a smart move and help TypeScript a lot. And the best thing: allowing the explicit annotation with the type 'any' in for..in and catch statements will not take away a thing from the others! The can still just omit annotations and nothing will change for them... |
I'd appreciate discussing this further. There are some aspects in my last post I wrote almost two months ago, that may be worth thinking about. An yes, #4518 is related and fortunately has not been closed yet. |
@JirkaDellOro i do not understand the proposal, is it allowing type annotation at this position, and it is always error unless it is |
As mentioned from the start throughout this thread, the proposal is simply allowing the explicit annotation of the type any. Nothing more than that. And it would be fine to change the error message from Analogous for the catch-clause. That would be consistent and clear! |
I do not think the utility achieved by the proposed change warrants the work. In the 0.8 time-frame we did allow type annotations in a fashion similar to this proposal and users have found it superfluous; we ended up removing them based on user feedback. I understand the argument of consistency; and we would definitely keep this in mind while designing new language features. but i think changing this now, first would be of little return, and would cause inconsistencies in other parts of the system e.g. noImplicitAny checks. |
Well, as stated in #4518, noImplicitAny is actually inconsistent at this point of time, isn't it? |
I do not think this is about noImplicitAny as much as it is about inferring |
I'd really like to just be albe to say "this is a string". Since I know that's the case with the types I 'for' over. Right now i'm forced to use another statement and I have to introduce another variable, just to get typing to work properly. |
but that is not true. if you do this with other parts of the system, e.g. |
Because you provide me no other means to concisely express the type that I know the variable must have. With 'var' I can do: var x = (string)<expr>; I can specify the type and then get type safety (even though I may end up failing at runtime completely). With And TS allows me to specify a type a ton of times in a declarative position where it is unsafe to do so. For example: var v: Object[] = ["foo", "bar"];
v.forEach((value: string) => value.length) This code is entirely unsafe. Indeed, i can change it to the following and get no problem: var v: Object[] = [0, 1];
v.forEach((value: string) => value.length) Now I have completely broken code that will succeed at typechecking but will fail badly. TS allowed me to specify the type, and it said "ok... even though that might not actually be the case, we're going to trust they knew what they were doing". I'm saying: extend me that trust when it comes for a 'for' loop. Yes, my code might break if it enocunters a symbol. But since 99.999999999% of the time i'm not using a symbol, i'm ok with that. On the other hand, by silently picking 'any' like we do today, I miss every case where I mistype things and say bad stuff like "v.lenth" because the compiler believes that I can do anything with this 'any'. |
And compiling with noImplicitAny doesn't show this implicit any! Isn't that inconsistent? I'm not going as far as CyrusNajmabadi. I could happily live with being able to explicitly annotate just the any type. At least that would keep the system syntactically consistent. For teaching programming to beginners using TypeScript, this is of very high value! |
I'm not sure if this is the right place to talk about this, but I have no idea how I'm supposed to safely use type Stats = {atk: number, def: number};
function clearStats(stats: Stats) {
for (var stat in stats) stats[stat] = 0;
} [ts] Element implicitly has an 'any' type because type '{ atk: number; def: number; }' has no index signature. type StatName = 'atk' | 'def';
type Stats = {atk: number, def: number};
function clearStats(stats: Stats) {
for (var stat: StatName in stats) stats[stat] = 0;
} [ts] The left-hand side of a 'for...in' statement cannot use a type annotation. type StatName = 'atk' | 'def';
type Stats = {[stat: StatName]: number};
function clearStats(stats: Stats) {
for (var stat in stats) stats[stat] = 0;
} [ts] An index signature parameter type must be 'string' or 'number'. Is it just me, or should there be a way to strongly type this? I've been using type StatName = 'atk' | 'def';
type Stats = {atk: number, def: number};
function clearStats(stats: Stats) {
for (var stat in stats) stats[stat as StatName] = 0;
} which works, but involves an unsafe cast. Is there a way to do this safely? |
first, you can not make assumptions about the keys of |
That wakes me up, of course! I'm still struggling with how I should explain these inconsistencies to people learning how to program with this great (seriously) language They may write
but not
|
As i noted earlier, it does not make much sense, and more importantly, it is not any safer. |
I don't care whether or not I use a type annotation, I just want to know what the idiomatic way of using (It would be nice if there was a way to explicitly and tersely assert my assumptions about the keys of |
As you noted earlier (about two and a half years ago), @mhegazy, it appears to make sense:
I understand, though, that it opposes too much work just in order to provide for a teacher or purist. But please keep it in mind. As I stated earlier, I'd probably go with an annotation of any, though that is just a compromise... |
@mhegazy I'd like to once again request the ability to define a closed type. Flow supports this: https://flow.org/en/docs/types/objects/#toc-exact-object-types If you won't do that, could you at least give an easier way to assert this? Your reason is "it is not any safer", but if you refuse to support making it safer, you could at least make it more readable and easier to write, by expanding |
Can someone speak definitively on why we disallow annotations on the iterating variable in As far as I know, In any case, I'm not expecting this to be changed; I'm just wondering what to say when someone asks. |
I have one case where this actually is the only place to declare the types. When async iterating (with Consider this example:
I realize that having a correct type for the stream would be the correct way to solve this, but that a much harder task to solve and if that gets solved, then it would be possible to catch a type error in the for-of construct.
If the stream is outputting objects of type any we will just do a type assertion. If/when we can have typed object-streams then we can typecheck that assertedType is the output of the stream. |
@stefan-jonasson I think you can do something like for await (const data of stream as AsyncGenerator<YourDataType, void, any>) { in that scenario. It's not ideal, but it's worked for me in the past. |
Reviving this one since there are much more plausible types other than |
const locators = {
firstname: {id: 'txtCustomerNameF'},
lastname: {id: 'txtCustomerNameL'},
email: {css: 'input.customerinfo[type=email]'},
phone: {css: 'input.customerinfo[type=number]'},
};
const customerInfo = {
firstname: 'foo',
lastname: 'bar',
};
for (const prop in customerInfo) {
// `prop` is now of type `string` - cannot add annotation to `const prop` either...
await driver.findElement(locators[prop]).sendKeys(customerInfo[prop]);
// ^ Element implicitly has an 'any' type because expression of
// type 'string' can't be used to index type...
// No index signature with a parameter of type 'string' was found on type...
} In this example, I feel that I would not want to add an generic string index type like I try to avoid using this index type whenever it is possible to avoid because I feel it reduces benefit of type checking. For example, if I had this instead: customerInfo = {
firstname: 'foo',
lastname: 'bar',
hasNoLocator: 'baz',
} I would want the compiler to raise an error that |
Cool! Nice to see this issue being reopened and considered again. The discussion progressed far, so I'd like to point back to my original request: #3500 (comment) Still, I'd be happy if I could annotate at least with |
In a for...in loop
I really think that |
Just to add another common reproduction case. You may want to restrict the string key of on object with the Let's take the example Here's the code verbatim: interface CatInfo {
age: number;
breed: string;
}
type CatName = "miffy" | "boris" | "mordred";
const cats: Record<CatName, CatInfo> = {
miffy: { age: 10, breed: "Persian" },
boris: { age: 5, breed: "Maine Coon" },
mordred: { age: 16, breed: "British Shorthair" },
}; Now let's say I want to simply loop over the keys and log each object. for (const cat in cats) {
console.log(cats[cat]);
} The following error occurs:
I've actually ran into this use case multiple times and there isn't a good way around except to not use the |
Hey @adamhenson! |
Expecting none of these to fail, but fails because enum OptionsEnum
{
'option1' = 'option1',
'option2' = 'option2',
'option3' = 'option3',
'option4' = 'option4',
}
type OptionsToUpdate = {
[Key in OptionsEnum]: string;
};
type OptionsToUpdateRecord = Record<keyof typeof OptionsEnum, string>;
const UpdateObjectRecord: OptionsToUpdateRecord = {
'option1': 'value1',
'option2': 'value2',
'option3': 'value3',
'option4': 'value4',
};
const UpdateObject: OptionsToUpdate = {
'option1': 'value1',
'option2': 'value2',
'option3': 'value3',
'option4': 'value4',
};
for (const key in OptionsEnum)
{
if (Object.prototype.hasOwnProperty.call(UpdateObjectRecord, key))
{
/*
Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'OptionsToUpdateRecord'.
No index signature with a parameter of type 'string' was found on type 'OptionsToUpdateRecord'.
*/
const value = UpdateObjectRecord[key]; // Error above
}
}
for (const key in OptionsEnum)
{
if (Object.prototype.hasOwnProperty.call(UpdateObject, key))
{
/*
Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'OptionsToUpdate'.
No index signature with a parameter of type 'string' was found on type 'OptionsToUpdate'.
*/
const value = UpdateObject[key]; // Error above
}
} |
I agree with the OP. The variable should be annotated to avoid mistakes and reduce code duplication (with further type casting) |
But, |
Sorry to revisit this yet again, I just want to be as clear as possible what I want here, in response to this:
To me, this is exactly the problem. Types may be open in the TS type system, but that doesn't mean types are open in my code. The type is clearly closed in my code. If TS can't handle closed types, that's fine, I really wish it could, Flow could, and it's a really useful feature, but I can live without it. But at least let me assert the types I need! Your workaround, let stat: keyof Stats;
for (stat in stats) {
setTimeout(() => { console.log(stat); }); // logs: def, def
}
for (const stat in stats) {
setTimeout(() => { console.log(stat); }); // logs: atk, def
} Let me reiterate: It is completely fine that TypeScript cannot typecheck the kind of code I want to write. I just want an escape hatch more ergonomic than "add ts-ignore comments to every single line of the loop" or "assert the type of I would be happy with something like |
My workaround is to access the property with type assertion like |
You could also consider using a type predicate as objects can be modified at runtime: interface CatInfo {
age: number;
breed: string;
}
// https://www.typescriptlang.org/docs/handbook/2/objects.html#readonly-tuple-types
const validCatNames = ["miffy", "boris", "mordred"] as const;
// https://www.typescriptlang.org/docs/handbook/2/indexed-access-types.html
type ValidCatName = (typeof validCatNames)[number];
// https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates
function isValidCatName(maybeCatName: string): maybeCatName is ValidCatName {
return (validCatNames as readonly string[]).includes(maybeCatName);
} const cats: Record<ValidCatName, CatInfo> = {
miffy: { age: 10, breed: "Persian" },
boris: { age: 5, breed: "Maine Coon" },
mordred: { age: 16, breed: "British Shorthair" },
};
for (const catName in cats) {
if (isValidCatName(catName)) {
console.log(cats[catName]);
} else {
// This is not required depending on your needs, just including for example purposes
throw new Error(`${catName} is not a ValidCatName`);
}
} |
For the reason of consistency, it should be possible to explicitly annotate the iterator variable at least with the "any" type in a for..in loop.
Edit from @sandersn: Fixes should apply to JSDoc too: #43756, and perhaps allow annotations on
for .. of
tooThe text was updated successfully, but these errors were encountered: