Object.values wrongly excludes undefined with optional properties

[jer-sen]

Bug Report

🔎 Search Terms

object values entries undefined optional properties

🕗 Version & Regression Information

• This changed between versions 4.2.3 and 4.3.2

⏯ Playground Link

Playground link with relevant code

💻 Code

const a: { b?: 1 | undefined } = { b: undefined };
const c = Object.values(a);
console.log(c); // Returns [undefined] but c has type 1[]

🙁 Actual behavior

c has type 1[] but can be equal to [undefined]

🙂 Expected behavior

c should have type (undefined | 1)[]

[RyanCavanaugh]

This is an intentional consistency change, since it's legal to initialize a with that value even if b doesn't include undefined:

const a: { b?: 1 } = { b: undefined };
const c = Object.values(a);
console.log(c);

However, this is a use case I'd expect to be handled properly with the new strictOptionalProperties flag

[jer-sen]

It's legal to initialize a with that value even if b doesn't include undefined, but Object.values should not exclude undefined from the possible values. How can it be 'intentional' since it leads to wrong dangerous types?

Same issue with Object.entries.

[RyanCavanaugh]

How can it be 'intentional' since it leads to wrong dangerous types?

Well, that's why we added the strictOptionalProperties flag.

[jer-sen]

What is exactly this flag ? Is it yet available ? I can't find it in the doc...

[RyanCavanaugh]

It's new (as in, unreleased); see #43947

[jer-sen]

Thanks. I'm not sure that this tag will solve the issue. It will still be possible to allow undefined as a value for an optional property and Object.values/entries will wrongly exclude this possibility.

[RyanCavanaugh]

It will still be possible to allow undefined as a value for an optional property

?. The entire purpose of the flag is to make that not be allowed.

[jer-sen]

In my understanding, its purpose is to allow (not to force!) an optional property to never be undefined if it is present.

[fatcerberus]

This is an error under strictOptionalProperties:

let x: { foo?: string } = { foo: undefined };

You either have to give foo a value, or else omit it, in which case it would be omitted from Object.values() too. Omitting the property was allowed before, so the flag would serve no purpose if it didn't enforce it.

[jer-sen]

Yes but that is not an error:

let x: { foo?: string | undefined } = { foo: undefined };

And this is the repro for my issue...

[ahejlsberg]

The cause of this change is #43086. It's debatable what the correct behavior is, as undefined in the type of an optional property sometimes should be interpreted as "the property isn't there" and other times as "the property could be there with the value undefined". In --strictOptionalProperties mode it is possible to distinguish (and we should, but don't currently), but without that option it isn't, and we simply have to commit to one or the other interpretation. Our current behavior (stripping undefined from optional properties with inferring to an index signature) is likely the desired behavior most of the time, but strictly speaking it isn't sound--as illustrated by this issue.

[fatcerberus]

But there’s no index signature in the repro…

[susisu]

There's actually an index signature in the type of Object.values:

TypeScript/lib/lib.es2017.object.d.ts

Line 26 in 5540364

values<T>(o: { [s: string]: T } | ArrayLike<T>): T[];

I think it is understandable to strip undefined from optional properties when --strictOptionalProperties=false, because { b?: 1 | undefined } is equivalent to { b?: 1 } in this case.
But is it OK to strip undefined when --strictOptionalProperties=true?
https://www.typescriptlang.org/play?strictOptionalProperties=true&ts=4.4.0-dev.20210613#code/MYewdgzgLgBAhgLhgbxgIwPxIIwwD4wCuYAJgKYBmAlmGSTAL4wC8K6Sx51t9DA3AChQkWMBYwA8mgBWZYFAB0ANzgAbQmQgAKOAEpBwiCFVkFqkAHMtwfQKA
In my opinion, { b?: 1 | undefined } under --strictOptionalProperties=true has a strict meaning that "b is 1 or undefined, or it does not exist" without ambiguity, so I expect c: (1 | undefined)[] here.

[jer-sen]

Don't forget that any external module can choose to export something like { b?: 1 | undefined }. And in that case the undefined possibility should really not be stripped since it could lead to the most common JS bugs that TypeScript wants to address.

[ahejlsberg]

So, there are a few things we should definitely fix:

• In --strictOptionalProperties mode, inference from an optional property to an index signature should simply infer the property type as written. For example, inferring from { a?: number } to { [key: string]: T } should infer number for T, and inferring from { a?: number | undefined } to { [key: string]: T } should infer number | undefined for T.
• In --strictOptionalProperties mode, a mapped type that strips optionality should not strip explicitly included undefined types. For example, Required<{ a?: number }> should be { a: number } and Required<{ a?: number | undefined }> should be { a: number | undefined }.

This will make the current definitions of Object.values and Object.entries behave as expected.

That said, I'm a bit surprised we have strongly typed definitions for Object.values and Object.entries in the first place. We've previously rejected having a similar definition for Object.keys (because an object might have more properties than we know about). I would have expected all of them to follow the same pattern.

In addition, if we stick with strongly typed Object.values and Object.entries we ought to improve their definitions as the current ones have issues. For example, applying Object.values to an object of a class or interface type with no index signatures yields any[], not a strongly typed array. I'm thinking the following would be better declarations:

values<T>(obj: T): T extends readonly any[] ? T[number][] : Required<T>[keyof T][];
entries<T>(obj: T): T extends readonly any[] ? [string, T[number]][] : [string, Required<T>[keyof T]][];

[ahejlsberg]

@DanielRosenwasser Opinions?

[RyanCavanaugh]

That said, I'm a bit surprised we have strongly typed definitions for Object.values and Object.entries in the first place

The "improvements" to those APIs were incorrectly merged at some point and now it's too much of a break to take them out. Maybe we should just give up on Object.keys, since people complain about its incompleteness relentlessly but never notice the unsound definitions for values / entries

The inference rule changes seem both correct and necessary for SOP.

[ahejlsberg]

With #44595, the original example behaves as expected in --strictOptionalProperties mode.

I'm leaving this issue open to track possible changes to Object.values and Object.entries.

[jer-sen]

Thanks a lot!

[RyanCavanaugh]

The key/value/entries issue is tracked at #38520, so closing this

[jcalz]

Mentioning --exactOptionalPropertyTypes since that's the actual name of the flag (to help with searches)