microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.03k stars 12.29k forks source link

Explicit any type in for..in loop #3500

Open JirkaDellOro opened 9 years ago

JirkaDellOro commented 9 years ago

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: https://github.com/microsoft/TypeScript/issues/43756, and perhaps allow annotations on for .. of too

CyrusNajmabadi commented 9 years ago

@JirkaDellOro I agree. I'd also like to be able to explicitly type it as 'string'.

mhegazy commented 9 years ago

@JirkaDellOro it is implicitly typed as any. and --noImplicitAny is excluding it, so what is the additional value you would be getting?

@CyrusNajmabadi, string is not accurate, though we should be able to type it as a PropertyKey now that we have union types (i.e. string|number|symbol).

JirkaDellOro commented 9 years ago

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?

mhegazy commented 9 years ago

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 any. The interesting part is we used to allow annotation, but ignore it, and ppl found that confusing. so we remove it to avoid confusion, similarly catch parameter can not be annotated either.

mhegazy commented 9 years ago

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 any for for..in and catch variables is just an overkill. there is really no value added, nor there is ambiguity involved. it is just making the user type 4 more characters.

JirkaDellOro commented 8 years ago

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".

danquirk commented 8 years ago

As Mohamed mentioned, this is consistent with our treatment of variables in catch statements, another case where the only valid type is any. We consistently don't allow type annotations in places where they do nothing. Allowing a type annotation there implicitly suggests there's a space of options available, when in fact there isn't. Your comparison to the value of comments is pretty off base and I think you know it. We would have to spend dev time to allow this type annotation, when it serves no purpose in the language other than to satisfy the people who require a type annotation on every variable (when our goal is precisely to not require you to write type annotations to that extent).

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.

JirkaDellOro commented 8 years ago

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.

danquirk commented 8 years ago

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 any) then that way should just be the default (ie don't even write a type annotation and the compiler has the right type), and allowing semantics there implies there's a set of options when there really isn't, which actually could make the code less clear in some respect. 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. I think what tips the scales for us in addition to the perf and any subjective opinion on the relative merits of the two philosophies is the fact that 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. So in that respect it's desirable to minimize where type annotations are required, or even allowed, as long as the compiler can do an equally good job as an explicit annotation. We want to encourage devs to feel like they can rely on type inference to do the right thing. 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).

JirkaDellOro commented 8 years ago

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" => I'd assume, that checking the type of annotation is only necessary if there IS an annotation. So far, we already check for the existence. So there should not be a performance loss for those who do not explicitly annotate, since then the type-check is skipped.

"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." => thanks

"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." => so far I understood, that the TypeScript-feature of explicit annotation is rather geared towards developers coming from a "classical" background (like me). And TypeScript does a really great job here! (with those two unfortunate exceptions of for..in and catch).

"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)." => I like your example of game design, since that is actually my area of expertise here at the university. The worst thing a game designer can do is to disallow the player to play the game the way the player likes to play it, especially disallowing this by using inconsistency...

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...

JirkaDellOro commented 8 years ago

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.

mhegazy commented 8 years ago

@JirkaDellOro i do not understand the proposal, is it allowing type annotation at this position, and it is always error unless it is any?

JirkaDellOro commented 8 years ago

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 "The left-hand side of a 'for...in' statement cannot use a type annotation" to "The left-hand side of a 'for...in' statement cannot use a type annotation other than 'any'

Analogous for the catch-clause. That would be consistent and clear!

mhegazy commented 8 years ago

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.

JirkaDellOro commented 8 years ago

Well, as stated in #4518, noImplicitAny is actually inconsistent at this point of time, isn't it?

mhegazy commented 8 years ago

I do not think this is about noImplicitAny as much as it is about inferring string|symbol instead of any. This will be a breaking change. we will have to do some real world code analysis to see how big of a break it is, and see if it is feasible.

CyrusNajmabadi commented 8 years ago

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.

mhegazy commented 8 years ago

I'd really like to just be albe to say "this is a string".

but that is not true. if you do this with other parts of the system, e.g. var x: string = 3 you get an error, or var x: string = foo() // where foo returns a string|symbol. why should we turn a blind eye here, but not there.

CyrusNajmabadi commented 8 years ago

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 for you don't give me any way to do that (outside of needing an ugly temporary).

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'.

JirkaDellOro commented 8 years ago

And compiling with noImplicitAny doesn't show this implicit any! Isn't that inconsistent? And it's not allowed to annotate an explicit any, which at least would bring the focus to the dynamic type. Though it is implicitly typed as any. Isn't that inconsistent too? While I can annotate anywhere else (besides catch which has the same problem). 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!

Zarel commented 6 years ago

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 for..in in TypeScript.

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?

mhegazy commented 6 years ago

first, you can not make assumptions about the keys of stats , since types are open in the TS type system, e.g. clearStats({atk: 1, def: 2, anohter:"sss"}). second, if you really, really want to put a type annotation, left the declaration outside the for loop, e.g. var stat: keyof Stats; for(stat in stats) {...}

JirkaDellOro commented 6 years ago

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

var stat: keyof Stats;
for(stat in stats) {
   ...
}

but not

for(var stat: keyof Stats in stats) {
    ...
}
mhegazy commented 6 years ago

As i noted earlier, it does not make much sense, and more importantly, it is not any safer.

Zarel commented 6 years ago

if you really, really want to put a type annotation

I don't care whether or not I use a type annotation, I just want to know what the idiomatic way of using for...in is, because the naive way produces an error.

(It would be nice if there was a way to explicitly and tersely assert my assumptions about the keys of stats, though. Something like for (var A! in B) to expand to var A: keyof typeof B; for (A in B) – or, even more nice, a way to assert a closed type, something analogous to Map<StatName, number> but JSON-compatible.)

JirkaDellOro commented 6 years ago

As you noted earlier (about two and a half years ago), @mhegazy, it appears to make sense:

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.

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...

todor2810 commented 6 years ago

@mhegazy, here[1] you wrote that "x is string|symbol" which I don't think is true. Keys of an object get converted to strings. In other words, x is always a string in a for...in loop. Which leads me to the question:

Shouldn't we accept the following syntax:

let object = {
    100: 'value1',
    [Symbol()]: 'value2'
};

let array = [10, 20, 30];

for (let key: string in object) {}
for (let index: string in array) {}

for the for...in statement?

I agree with you about the for...of and try...catch statements. Nothing can be done there.

[1] https://github.com/Microsoft/TypeScript/issues/3500#issuecomment-112983763

mnpenner commented 5 years ago

I'm having a similar issue. Not sure if I should open a new ticket.

Here's my code:

for await(const oldClient of db.stream("select * from emr_client limit 5")) {

stream is typed as AsyncIterable<{[k:string]:any}>, but in this scenario, I know which columns emr_client has, so I can be more explicit. Thus, I'd like to be able to write:

for await(const oldClient: Client of db.stream("select * from emr_client limit 5"))

But I can't (TS2483). For the time being, I can write this instead:

for await(const oldClient of db.stream("select * from emr_client limit 5") as AsyncIterable<Client>) {

But I don't really care about AsyncIterable part, so I'd prefer not to have to include that.

Zarel commented 5 years ago

@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 for (var A! in B) to var A: keyof typeof B; for (A in B).

jcalz commented 5 years ago

Can someone speak definitively on why we disallow annotations on the iterating variable in for...of? Every place I look ( e.g., https://github.com/microsoft/TypeScript/commit/7cb2a643503698d3da5469b01d1a8fcd67d2125f ) seems to come back here, but most of the comments here are specifically about for...in loops, in which the iterating variable can't be narrowed from keyof any.

As far as I know, for (let x: string of someStringArray) {} would be redundant, but not nonsense or unsafe like in for...in. And the same redundancy would be true of let x: string = someStringArray[0], so redundancy alone can't be the issue. In fact, it would serve the same purpose as other inferrable-but-annotated-anyway annotations: fail-fast warnings in the case of changes (e.g., someStringArray later becomes a number[])

In any case, I'm not expecting this to be changed; I'm just wondering what to say when someone asks.

stefan-jonasson commented 4 years ago

I have one case where this actually is the only place to declare the types. When async iterating (with for-await-of) over a stream it's not possible to type annotate the output of that stream (as far as i know).

Consider this example:

const stream = createAReadableStream();
for await (const data of stream) {
  // data is of type any
}

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.

const stream = createAReadableStream();
for await (const data: assertedType of stream) {
  // data is of type assertedType 
}

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.

mnpenner commented 4 years ago

@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.

RyanCavanaugh commented 4 years ago

Reviving this one since there are much more plausible types other than string that might be written here (particularly keyof T or other things that produce string unions)

monkpit commented 4 years ago
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 prop could have the type keyof T as mentioned by @RyanCavanaugh.

I would not want to add an generic string index type like {[key: string]: string} to my customerInfo, and also would not want to add {[key: string]: Locator} to locators.

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 hasNoLocator does not exist on type of locators.

JirkaDellOro commented 4 years ago

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: https://github.com/microsoft/TypeScript/issues/3500#issue-87994819

Still, I'd be happy if I could annotate at least with any or unknown for the sake of syntactical consistency. Same applies to catch. Please go that route, if a detailed type annotation is too complex.

NemoStein commented 3 years ago

In a for...in loop

for (const x in y) { ... }

I really think that x should be implicitly typed to keyof y instead of string (or any) considering that the loop will only assign enumerable non-Symbol properties of the y and nothing else can be assigned to x as it's a const.

adamhenson commented 2 years ago

Just to add another common reproduction case. You may want to restrict the string key of on object with the prop1 | prop2 syntax.

Let's take the example Record from the official docs.

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:

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'Record<CatName, CatInfo>'.

I've actually ran into this use case multiple times and there isn't a good way around except to not use the | syntax and just use string. But, I use that syntax specifically so other developers can see what the valid values could be.

Lincer commented 2 years ago

Hey @adamhenson! I'm constantly running into the same limitation/error through this exact scenario. What's your go-to workaround for it?

MoazAlkharfan commented 2 years ago

Expecting none of these to fail, but fails because for in expands to string

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
    }
}
maicol07 commented 2 years ago

I agree with the OP. The variable should be annotated to avoid mistakes and reduce code duplication (with further type casting)

ShivamJoker commented 1 year ago

I am also facing the same issue, can Typescript team please fix this?

image

QC2168 commented 6 months ago

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

var stat: keyof Stats;
for(stat in stats) {
   ...
}

but not

for(var stat: keyof Stats in stats) {
    ...
}

But, key Has not been recycled.

Zarel commented 6 months ago

Sorry to revisit this yet again, I just want to be as clear as possible what I want here, in response to this:

first, you can not make assumptions about the keys of stats , since types are open in the TS type system, e.g. clearStats({atk: 1, def: 2, anohter:"sss"}). second, if you really, really want to put a type annotation, left the declaration outside the for loop, e.g. var stat: keyof Stats; for(stat in stats) {...}

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) {...} has the problem in that it has completely different semantics from for (const stat: keyof Stats in stats) {...}. Namely, these two produce different results:

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 stats every single time I use it". Any other variable can have its type declared in its definition. Even the one in catch now! Why not this one?

I would be happy with something like for (const stat: keyof Stats in stats as any). Something that makes it clear I'm doing something unsafe. I just want it to be possible at all.

JorensM commented 5 months ago

Just to add another common reproduction case. You may want to restrict the string key of on object with the prop1 | prop2 syntax.

Let's take the example Record from the official docs.

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:

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'Record<CatName, CatInfo>'.

I've actually ran into this use case multiple times and there isn't a good way around except to not use the | syntax and just use string. But, I use that syntax specifically so other developers can see what the valid values could be.

My workaround is to access the property with type assertion like [cat as CatName]. But I would love to see built-in type inference for for...in loops! Also I've heard people say that TypeScript has open types and because of that this feature cannot be done. But I'm not sure what is meant by 'open types' here. For example if I have a type Thing with properties subthing1 and subthing2, I won't be able to access a property someothersubthing on type Thing, doesn't that make the type closed?. Or does 'open types' mean something else?

TrevorSayre commented 2 months ago

@adamhenson https://github.com/microsoft/TypeScript/issues/3500#issuecomment-893769107

@JorensM https://github.com/microsoft/TypeScript/issues/3500#issuecomment-1923231387

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`);
  }
}

TypeScript Playground