Feedback on Object.values Changes in v2.8.0

[mrsekut]

I have some thoughts on the changes in v2.8.0.

With the original definition, the code below would result in never[] due to the strict typing:

interface ObjectConstructor {
  values<K extends PropertyKey, V>(o: Record<K, V>): V[];
}

let myObj: object = {};

const values = Object.values(myObj);
// ^? const values: never[]

After the change, it becomes unknown[]:

interface ObjectConstructor {
  values<K extends PropertyKey, V>(
    o: Record<K, V>,
  ): (string extends K ? V : number extends K ? V : unknown)[];
}

let myObj: object = {};

const values = Object.values(myObj);
// ^? const values: unknown[]

This update, although necessary, introduces some inconvenience. For instance, if we have code like this:

type ProductPrices = {
  widget: Price;
  gadget: Price;
};

export function total(productPrices: ProductPrices) {
  return Object.values(productPrices).map(getProductPrice).reduce(sum, 0);
  // This would not error in v2.7.
  // However, it results in an error in v2.8.
}

type Price = { price: number };

function getProductPrice(price: Price): number {
  return price.price;
}

function sum(acc: number, curr: number): number {
  return acc + curr;
}

We would now need to reassign the variable just to satisfy the type system, like this:

export function total(productPrices: ProductPrices) {
  const productPrices_: Record<string, Price> = productPrices; // Reassignment
  return Object.values(productPrices_).map(getProductPrice).reduce(sum, 0);
}

Or we might have to use an obvious as cast:

export function totalPrices(productPrices: ProductPrices) {
  return Object.values(productPrices as Record<string, Price>) // Cast
    .map(getProductPrice)
    .reduce(sum, 0);
}

The root of this issue lies in the new implementation of Object.values in this version. The new check uses string extends K instead of K extends string, which results in a false condition for keyof:

values<K extends PropertyKey, V>(
  o: Record<K, V>,
): (string extends K ? V : number extends K ? V : unknown)[];

type Test<K extends PropertyKey> = string extends K ? true : false;
type V = Test<keyof ProductPrices>; // false

This might have been done intentionally, but it does require extra handling in the code. Although the change is understandable and I'm aware that a perfect solution might be difficult to achieve, as mentioned in PR#42, I wanted to share my experience with the update.

Thank you for all your hard work and efforts in improving this library.

[uhyo]

Thank you for the feedback! This is done intentionally. I know this is inconvenient but I wanted to choose a bit of improved safety.

As a workaround, as you mentioned, you need to manually convert your object to a Record type.

Do note that this is unsafe -- you need to consider cases like this:

type ProductPrices = {
  widget: Price;
  gadget: Price;
};

const obj = {
  widget: { price: 100 },
  gadget: { price: 500 },
  banana: "1万円",
};

// oops!
total(obj);

Actually, assigning ProductPrices to Record<string, Price> has the same unsafety, although TS allows it unfortunately.

The intention of the change is that now you need to do this unsafe operation by yourselves. After all, it is very difficult to safely use Object.values and Object.entries in TypeScript, so I think it is inevitable that using them is hard while using better-typescript-lib, a library with type safety as its primary goal.

As a general recommendation, you might want to consider using Maps instead of object for this kind of logic.

[mrsekut]

Thank you for your prompt response.

I appreciate the insight on not accessing the specific structure of objects in TypeScript. I had overlooked this principle when I encountered the changes in v2.8.

Your point about avoiding the use of Object.values and Object.keys and making the process intentionally cumbersome to enforce better practices is well-taken, especially now that I understand the intention behind it.

The concrete example you provided was very helpful. Thank you for the clarity and detailed explanation.