Looks like the test cases could be simplified, there are so many test cases of the form UnwrapPartial<Partial<Something>>, with different values for Something. I think we don't need that many because the value of Something doesn't really matter, so we could just keep a few and remove the others.

Tests that don't have Partial are useful, like UnwrapPartial<{a?: string; b: number}>, UnwrapPartial<{a?: string; b?: number}>.

Hey Som, thanks for the review.

I use UnwrapPartial<Partial<Something>> to ensure that the original Something type gets preserved. And there is a good number of types to consider.

Cases like UnwrapPartial<{a?: string; b: number}> and UnwrapPartial<{a?: string; b?: number}> are already covered (under `UnwrapPartial` preserves optional properties).

I’ll add any missing cases you pointed out, but feel free to prune things on your end as well.

Cases like UnwrapPartial<{a?: string; b: number}> and UnwrapPartial<{a?: string; b?: number}> are already covered (under `UnwrapPartial` preserves optional properties).

Yeah, I meant all of those are useful, don't remove them.

@som-sm — Unless you want to make any other specific changes, I think this PR is ready to merge.

Should these be never or PartialObjectType, what would be better?

The way I see this, you shouldn’t unwrap a modifier on a type that doesn’t have one in the first place (goes for all Unwrap* types). UnwrapPartial<number> should never resolve to number.

I was also wondering earlier:

Should we be more strict about array types? Technically UnwrapPartial<string[]> will pass as string[], but it doesn’t make much sense (neither does defining Partial<string[]>).

After thinking it over, I’m leaning towards not giving arrays any special treatment. Let’s assume the user knows what they’re doing.

After thinking it over, I’m leaning towards not giving arrays any special treatment. Let’s assume the user knows what they’re doing.

Yeah, that's fine.

The way I see this, you shouldn’t unwrap a modifier on a type that doesn’t have one in the first place (goes for all Unwrap* types). UnwrapPartial<number> should never resolve to number.

I brought this up because with unions, IMO, it feels better if UnwrapPartial<Partial<{x: number, y: number}> | [number, number]>> returned {x: number, y: number} | [number, number] as opposed to just {x: number; y: number}. WDYT?

A union with a partial isn’t technically a partial, and handling special cases is a can of worms we probably don’t want to open 🙂

A union with a partial isn’t technically a partial

No, I did not mean that Partial<{x: number, y: number}> | [number, number] is a partial. I meant that instantiating UnwrapPartial with a union that contains both partial and non-partial items would cause the non-partial items to be removed, which might not be desirable.

handling special cases is a can of worms we probably don’t want to open

No, we'd obviously not handle unions separately, I meant replacing never with PartialObjectType. I think that'd be better.

Writable (technically UnwrapReadonly) works that way already, so I’m fine with returning the original type for consistency. Just made the necessary edits.

Writable (technically UnwrapReadonly)

They're not the same, right? Just like Required and UnwrapPartial:

type Config = Partial<{port: number; host: string; secure?: boolean}>;
//=> {port?: number; host?: string; secure?: boolean}

type T1 = UnwrapPartial<Config>;
//=> {port: number; host: string; secure?: boolean}

type T2 = Required<T1>;
//=> {port: number; host: string; secure: boolean}

so I’m fine with returning the original type for consistency. Just made the necessary edits.

👍