Skip to content

Update type definition of Object.keys#17267

Closed
kube wants to merge 1 commit into
microsoft:masterfrom
kube:patch-1
Closed

Update type definition of Object.keys#17267
kube wants to merge 1 commit into
microsoft:masterfrom
kube:patch-1

Conversation

@kube

@kube kube commented Jul 18, 2017

Copy link
Copy Markdown

Solves need to explicitly specify type of key when in Strict Mode.

Issue it solves

const someObj = {
  a: 42,
  b: 'Hello'
}

Object.keys(someObj)
  .forEach(key =>
    someObj[key] // ERROR: Element implicitly has an 'any' type because type ... has no index signature.
  )

Solves need to explicitly specify type of `key` when in **Strict Mode**.

## Issue it solves

```ts
const someObj = {
  a: 42,
  b: 'Hello'
}

Object.keys(someObj)
  .forEach(key =>
    someObj[key] // ERROR: Element implicitly has an 'any' type because type ... has no index signature.
  )
```
@msftclas

Copy link
Copy Markdown

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@ikatyang

Copy link
Copy Markdown
Contributor

These files are not meant to be edited by hand. If you need to make modifications, the respective files should be changed within the repository's top-level src directory. Running jake LKG will then appropriately update the files in this directory. (/lib@master#read-this)

---> /src/lib@master#read-this

And also, there is already a similar PR, see #17261 (only modify the type of o).

@kube

kube commented Jul 18, 2017

Copy link
Copy Markdown
Author

Ok sorry for not reading, and thanks for your comment. :/

PR #17261 does not solve this issue, as it just restricts what can be passed to Object.keys.

My PR is to link the output type of Object.keys to the input type, and I don't think it's possible without a generic type.

@ikatyang

ikatyang commented Jul 18, 2017

Copy link
Copy Markdown
Contributor

EDIT: overload does nothing in this case.

I think it'd be better to add overload instead of modifying the original one, since Object.keys should always return string[], consider the following case:

declare function keys<A>(o: A): (keyof A)[];
declare const x: object;

keys(x); //=> never[]; <------- should be string[]

@jwbay

jwbay commented Jul 18, 2017

Copy link
Copy Markdown
Contributor

See - #15295,
#13254,
#12253 (comment)

Comment thread lib/lib.es5.d.ts
* @param o Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
*/
keys(o: any): string[];
keys<A>(o: A): (keyof A)[];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<A extends {}>

The problem with making Object.keys return keyof A is that typescript makes no guarantees that the input object has only the keys that are specified in its type. There may be valid values in the array that are not considered by the type.

@rbuckton rbuckton requested a review from sandersn August 9, 2017 20:49
@rbuckton

rbuckton commented Aug 9, 2017

Copy link
Copy Markdown
Contributor

@sandersn, can you take a look?

@sandersn

sandersn commented Aug 9, 2017

Copy link
Copy Markdown
Member

I think the issue raised in #12253 (comment) still holds: (keyof T)[] is not useful or accurate when Object.keys is called with an object that has more properties than its declared type. I'll close this PR in favour of the simpler #17261 that just has o: {}.

@sandersn sandersn closed this Aug 9, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants