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

Object.keys / Object.entries not type checking argument #35145

Closed
rohitf opened this issue Nov 16, 2019 · 10 comments
Closed

Object.keys / Object.entries not type checking argument #35145

rohitf opened this issue Nov 16, 2019 · 10 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@rohitf
Copy link

rohitf commented Nov 16, 2019

TypeScript Version: 3.7.2

Search Terms:
keys
entries

Code

// Some examples
// obj is a number
let obj: number = 23
for (let key of Object.keys(obj)){
    console.log(key)
}

// obj2 is a string
let obj2: string = "hello"
for (let key of Object.keys(obj2)){
    console.log(key)
}

Note: The same issue occurs with Object.entries

Expected behavior:
TypeScript error since number and string are not valid parameter types for Object.keys(object).

Actual behavior:
No error/warning.

Playground Link:
https://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=11&pc=2#code/PTAEHsCMCtQSwM6gIagHYFcC2kCmAnAWACgAbXAFwhgC51s99QBeUAJgGYSAzcJgCnJUA1rgCeEbqADyMXAGMKAOlFiE-KNACUWgN4lQh0PPBoE4cktLgA5v1VaSAXxIkQ1aAEZ4SVAgr4cGg2JEIebHT+gcEsoABEABa4pNZxPHyggpSgqpIycooq4uqabDr6xEbGpuaW1nYOzkA

Related Issues:
N/A

@rohitf
Copy link
Author

rohitf commented Nov 16, 2019

Since the function definition for Object.keys() is
ObjectConstructor.keys(o: {}): string[] (+1 overload), it may also be related to a root issue with the type {}, which should represent an object literal.

Observe:

let value: {} // value has type {}
value = 5 // Does not error
value = "hello, world" // Does not error

@Validark
Copy link

@fatcerberus
Copy link

Everything except null and undefined is assignable to {} because it’s a structural object type which requires no specific properties and all non-nullish values can be treated as objects at runtime.

tl;dr: number is assignable to {} for the same reason it’s assignable to { toString(): string }.

@rohitf
Copy link
Author

rohitf commented Nov 17, 2019

Everything except null and undefined is assignable to {} because it’s a structural object type which requires no specific properties and all non-nullish values can be treated as objects at runtime.

Makes sense, perhaps the expected parameter type for Object.keys / Object.entries should be updated to something like {[x: string]: any}?

P.S: Re the link by @Validark above, is there a reason why the {} type is still used?

@fatcerberus
Copy link

I think primitives are still assignable to that. It would have to be object if you wanted to exclude primitive types.

@Validark
Copy link

@rohitf In general, {} is useful for when you want to use it for what it is: everything but null | undefined. The problem arises when developers mistakenly think it means object.

With regards to Object.keys/Object.entries: I don't know what decisions were made or why.

@resynth1943
Copy link

Technically, it's valid to pass a non-object literal to Object.keys:

Object.keys(0); // []
Object.keys('a'); // ['0']

@rohitf
Copy link
Author

rohitf commented Nov 18, 2019

@resynth1943 wow didn't know that, looks like it's definitely possible but generally not safe / intended behavior. In that case, a warning would likely be better than an error (since it is valid JS).

@fatcerberus
Copy link

There is no distinction between warnings and errors in TS. Sometimes I wish there was. 🙁

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript labels Nov 26, 2019
@RyanCavanaugh
Copy link
Member

"not safe" / "not intended" is in the eye of the beholder. If x[Object.keys(x)[0]] returns a non-undefined value, as it does for string, it can hardly be said to be truly wrong.

In any case, [] for primitive values is a well-defined result that accurately describes a runtime fact without creating any big surprises or coercions, so this doesn't look like something that deserves creating a breaking change for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants