sindresorhus / type-fest

A collection of essential TypeScript types
Creative Commons Zero v1.0 Universal
14.1k stars 533 forks source link

3.5.4 breaks Array.isArray on JsonValue through #549 #550

Open leaumar opened 1 year ago

leaumar commented 1 year ago

Trying to update type-fest to 3.5.4 yields the following breakage: image image

An Array.isArray(JsonValue) guard removes JsonValue[] but not readonly JsonValue[] from the type. Is there a non-hacky solution (i.e. no manual type overrides) or could #549 be reverted and achieved a different way?

Upvote & Fund

Fund with Polar

sindresorhus commented 1 year ago

// @chrisvasz

chrisvasz commented 1 year ago

This appears to be a Typescript bug: https://github.com/microsoft/TypeScript/issues/17002. Looks like the best recommendation in that thread is to override the type definition of Array.isArray. That recommendation may or may not be acceptable for consumers of this library though. Alternatively, we could restore the previous implementation of JsonValue, then add a ReadonlyJsonValue type that accepts ReadonlyArrays. Thoughts?

leaumar commented 1 year ago

It seems a very different issue to me, it's about losing the readonly part of the array type when it is an array. If you ask me, Array.isArray should say that a readonly x[] is an array just the same, type-wise. Making a helper function that expands Array.isArray's type a bit seems to work. Note how the type of data isn't just JsonValue[] anymore but the whole JsonArray (though it doesn't combine the other types into JsonPrimitive the same way): image

Perhaps a helper function in type-fest for each of the Json types? isJsonPrimitive, isJsonArray, and isJsonObject to narrow the general JsonValue?

KonstantinSimeonov commented 8 months ago

I think this change on JsonValue would ideally be reverted and ReadonlyDeep<JsonValue> could be used for restricting mutability. The problem with reverting this is that projects probably already depend on this behavior. An alternative could be adding JsonValueWriteable which doesn't include the readonly array. This could provide an straightforward upgrade path for projects which are behind on versions.

Exporting a utility function is not a good solution for the following reasons: