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

Infer keyof Obj instead of string in for(key in obj) loops and Object.keys() #44706

Closed
5 tasks done
antoinep92 opened this issue Jun 23, 2021 · 4 comments
Closed
5 tasks done

Comments

@antoinep92
Copy link

Suggestion

πŸ” Search Terms

for in loop
string keyof infer

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Object.keys(obj as Obj) should have static type Array<keyof Obj> instead of string[]

in for(key in obj as Obj), key should have static type keyof Obj instead of string

Even when objects have well known keys, it is a common practice in javascript to iterate dynamically over the keys using Object.keys or for..in loops.

In typescript it is a bit convoluted to achieve, which confuses newcomers and is a bit painful because:

  1. Object.keys()always returns string[] and in for(key in object), key is always inferred as string
  2. object[key] only work with strings if we explicitly declared a string index in the type declaration of object

πŸ“ƒ Motivating Example

interface Conf {
  color?: string
  size?: number
}
const defaultConf: Conf = { color: "red", size: 13 }
function init(conf?: Conf) {
  if(!conf) conf = {};
  for(const k in defaultConf) {
    if(conf[k] === undefined) conf[k] = defaultConf[k];
// Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'Conf'.
//  No index signature with a parameter of type 'string' was found on type 'Conf'.(7053)
  }
}

This code is sound, in the sense that it wouldn't generate runtime errors or violate (implicit) code invariants and hypothesis. But it doesn't compiles in typescript. To make matter worse, when reading the error, one could be tempted to change the Conf interface to:

interface Conf {
  color?: string
  size?: number
  [key: string]: any
}

And indeed, the code then compiles. Unfortunately, doing so just made the code less type-safe, by broadening the Conf interface too much for no real reason. I think this is related to this issue about the error wording.

The better solution, would be to change the offending line with:

if(conf[k as keyof Conf] === undefined) conf[k as keyof Conf] = defaultConf[k as keyof Conf];

This compiles, and does what we want. But I find it a bit verbose, and generaly I'm not a fan of casts.

I tried changing the type at the for loop instead, but no luck:

  for(const k: keyof Conf in defaultConf) {
// The left-hand side of a 'for...in' statement cannot use a type annotation.(2404)

This is a bit frustrating because for me this is kind of the definition of keyof so it's unfortunate.

Maybe I missed something, an there are probably some corner-case like Record<number, any> where keyof would not be backward compatible with string. But for the most part, I think it shouldn't break anything. Corner cases could be handled using a more complex definition:

  • If keyof is compatible with string, then use keyof
  • Otherwise, use string

πŸ’» Use Cases

There are many use-cases. Javascript (and by extension typescript) is very dynamic, and there are many situations when it makes sense to iterate on objects keys instead of duplicating code: generic checks, deep copies, deep transformations, mixins, and many more.

I feel like there is a broader issue at play, where this kind of small grains of sand add up, and kind of push away developers away from some coding patterns in favor of object oriented paradigm. But hopefully I'm mistaken, and this is probably not the place for this discussion.

@antoinep92 antoinep92 changed the title Infer keyof Objinstead of string in for(key in obj) loops and Object.keys() Infer keyof Obj instead of string in for(key in obj) loops and Object.keys() Jun 23, 2021
@RyanCavanaugh
Copy link
Member

This code is sound, in the sense that it wouldn't generate runtime errors or violate (implicit) code invariants and hypothesis

It isn't, though. Have you read https://stackoverflow.com/questions/55012174/why-doesnt-object-keys-return-a-keyof-type-in-typescript ?

@antoinep92
Copy link
Author

antoinep92 commented Jun 23, 2021

Indeed ! Sorry about that, I guess I was too focused on my use-case (I hit this issue a few time in the past days and was getting annoyed). So it is more complicated than I thought. Nevertheless, I still think there is an ergonomics issue.

I understand we have to opt-in into having keyof instead of string, but arguably we have to opt-in more often than not, and the way to opt-in is a bit heavy. Can we do that at the level of the loop instead of each usage of the key ? Or any other idea to make this less verbose with for..in ?

I guess we can do for(const key of Object.keys(obj) as typeof Obj) but I'm open to suggestions ! πŸ™

Would it make sense to have a WithTypedKeys<> type "generator" (like Readonly<>, Partial<>, etc.) that would work like this ?

for(const key in obj as WithTypedKeys<Obj>) // infers key as keyof Obj

From a developper point of view I think this would at the same time clearly indicate the opt-in, and not be too verbose. But I guess it's not easy to implement (distinguish WithTypedKeys objects from all others).

@RyanCavanaugh
Copy link
Member

We don't add new top-level helpers unless they're necessary for declaration emit, since people have varying opinions about what kind of constraints they should have.

Ultimately the knowledge that the object you have has no aliased properties is an assumption, and type assertions are there to encode your assumptions (or external knowledge).

@antoinep92
Copy link
Author

Understood, thanks for your feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants