microsoft/TypeScript

Object.keys / Object.entries not type checking argument

rohitf opened this issue · 10 comments

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

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

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

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?

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

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

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

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

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

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

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