sindresorhus / type-fest

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

`PartialDeep`: recurse into objects in array without changing the array itself #514

Open yifanwww opened 1 year ago

yifanwww commented 1 year ago

I saw this proposal https://github.com/sindresorhus/type-fest/issues/367 and the relevant context. I understand the need of recurseIntoArrays: false, but still get a little bit confused.

This https://github.com/sindresorhus/type-fest/issues/356#issuecomment-1022114231 talks about the situation about what problem recurseIntoArrays: true would cause.

Array types like (string|undefined)[] are thorny because JSON.parse(JSON.stringify(['a', undefined, 'b'])) becomes ['a', null, 'b'] on the other side and null is not assignable to undefined. This is where PartialDeep was correctly causing a type failure.

I agree with

In some sense I think PartialDeep is going a little too far. {foo: string[]} becomes {foo?: (string|undefined)[]} but the user probably just intended {foo?: string[]} which would be fine for serialization.

But I think we didn't consider all the type possibilities.

For serialization,

Type-generically, there are 3 results that { foo: T[] } can become:

  1. { foo?: T[] }
  2. { foo?: PartialDeep<T, Options>[] }
  3. { foo?: PartialDeep<T | undefined, Options>[] }

I think No.2 is what most users want when serializing, not No.1.

Upvote & Fund

Fund with Polar

sindresorhus commented 1 year ago

// @bbrk24

bbrk24 commented 1 year ago

Huh, I see. I understand the request but I'm not sure how best to represent this in the options object.

plehnen commented 1 year ago

Not sure if this is related, but I can't get PartialDeep to work with arrays when I want to re-assign values. Please have a look at the demo: reproduction-link

any advice maybe? :)

bbrk24 commented 1 year ago

@plehnen My initial reaction is that it seems to be a limitation of the language. After all, it works fine if you assign it to a temporary constant:

    const innerValue = impl[id1].innerArray[id2];
    assertIsDefined(innerValue);
    innerValue.id = 'a'; 
plehnen commented 1 year ago

But how come that it works with any other "regular" type (see "payload2" example in the same link), but only with PartialDeep this error is introduced?

PS: my example is simplified. It is actually inside a vue ref, so assigning it to a temporary constant will lose its reactivity. Currently I solve this as I did in the example: Using Omit and re-assigning the "innerArray" with the new type. But I hate that I have to repeat myself and that TS wouldn't complain if someone would rename the originals "innerArray". Using PartialDeep seems so much more elegant - if only it would work the same. Weirdly enough, as both type should basically be the same, right?

Livven commented 3 months ago

Heavy +1 for option 2. I think option 2 should be the default, option 1 defeats the point of a PartialDeep utility type in the first place since it arbitrarily stops at arrays which is very unintuitive, and there is IMO no need at all for option 3.

So with option 2 as the default the recurseIntoArrays parameter could be completely removed, or maybe provide a different parameter to allow for option 3 (but I don't think it would be needed).