utility-types icon indicating copy to clipboard operation
utility-types copied to clipboard

DeepXXX<T> code consistency

Open dosentmatter opened this issue 6 years ago • 4 comments
trafficstars

Is your feature request related to a real problem or use-case?

We have DeepReadonly<T>, DeepMutable<T> (added in #114), DeepRequired<T>, DeepNonNullable<T>, and DeepPartial<T>. We should have consistency in their code.

Describe a solution including usage in code example

Proposed changes are below with inline comments:

diff --git a/src/mapped-types.ts b/src/mapped-types.ts
index 862b9c4..fbbb6c2 100644
--- a/src/mapped-types.ts
+++ b/src/mapped-types.ts
@@ -392,13 +392,13 @@ export type PromiseType<T extends Promise<any>> = T extends Promise<infer U>
  *   };
  *   type ReadonlyNestedProps = DeepReadonly<NestedProps>;
  */
-export type DeepReadonly<T> = T extends ((...args: any[]) => any) | Primitive
+export type DeepReadonly<T> = T extends ((...args: any[]) => any) | Primitive | null | undefined // null and undefined not needed after #113
   ? T
-  : T extends _DeepReadonlyArray<infer U>
-  ? _DeepReadonlyArray<U>
-  : T extends _DeepReadonlyObject<infer V>
-  ? _DeepReadonlyObject<V>
-  : T;
+  : T extends _DeepReadonlyArray<any> // _DeepXXXArray<any> vs any[] described below
+  ? _DeepReadonlyArray<T[number]> // Don't need to infer on Arrays, can just index with number
+  : T extends _DeepReadonlyObject<infer U> // _DeepXXXObject<infer U> vs object described below
+  ? _DeepReadonlyObject<U>
+  : never; // all primitives and non-primitives are covered and this case will never happen
 /** @private */
 // tslint:disable-next-line:class-name
 export interface _DeepReadonlyArray<T> extends ReadonlyArray<DeepReadonly<T>> {}
@@ -407,6 +407,21 @@ export type _DeepReadonlyObject<T> = {
   readonly [P in keyof T]: DeepReadonly<T[P]>
 };
 
+export type DeepMutable<T> = T extends ((...args: any[]) => any) | Primitive | null | undefined // added in #114; null and undefined not needed after #113
+  ? T
+  : T extends _DeepMutableArray<any> // _DeepXXXArray<any> vs any[] described below
+  ? _DeepMutableArray<T[number]> // Don't need to infer on Arrays, can just index with number
+: T extends _DeepMutableObject<infer U> // _DeepXXXObject<infer U> vs object described below
+  ? _DeepMutableObject<U>
+  : never; // all primitives and non-primitives are covered and this case will never happen
+/** @private */
+// tslint:disable-next-line:class-name
+export interface _DeepMutableArray<T> extends Array<DeepMutable<T>> {}
+/** @private */
+export type _DeepMutableObject<T> = {
+  -readonly [P in keyof T]: DeepMutable<T[P]>
+};
+
 /**
  * DeepRequired
  * @desc Required that works for deeply nested structure
@@ -427,13 +442,13 @@ export type _DeepReadonlyObject<T> = {
  *   };
  *   type RequiredNestedProps = DeepRequired<NestedProps>;
  */
-export type DeepRequired<T> = T extends (...args: any[]) => any
+export type DeepRequired<T> = T extends ((...args: any[]) => any) | Primitive | null | undefined // Move functions/primitives to be handled in one place; null and undefined not needed after #113
   ? T
   : T extends any[] // _DeepXXXArray<any> vs any[] described below
   ? _DeepRequiredArray<T[number]> // Don't need to infer on Arrays, can just index with number
   : T extends object // _DeepXXXObject<infer U> vs object described below
   ? _DeepRequiredObject<T>
-  : T;
+  : never; // all primitives and non-primitives are covered and this case will never happen
 /** @private */
 // tslint:disable-next-line:class-name
 export interface _DeepRequiredArray<T>
@@ -464,13 +479,13 @@ export type _DeepRequiredObject<T> = {
  *   };
  *   type RequiredNestedProps = DeepNonNullable<NestedProps>;
  */
-export type DeepNonNullable<T> = T extends (...args: any[]) => any
+export type DeepNonNullable<T> = T extends ((...args: any[]) => any) | Primitive | null | undefined
   ? T
   : T extends any[] // _DeepXXXArray<any> vs any[] described below
   ? _DeepNonNullableArray<T[number]> // Don't need to infer on Arrays, can just index with number
   : T extends object // _DeepXXXObject<infer U> vs object described below
   ? _DeepNonNullableObject<T>
-  : T;
+  : never; // all primitives and non-primitives are covered and this case will never happen
 /** @private */
 // tslint:disable-next-line:class-name
 export interface _DeepNonNullableArray<T>
@@ -500,13 +515,13 @@ export type _DeepNonNullableObject<T> = {
  *   };
  *   type PartialNestedProps = DeepPartial<NestedProps>;
  */
-export type DeepPartial<T> = T extends Function
-  ? T
-  : T extends Array<infer U>
-  ? _DeepPartialArray<U>
-  : T extends object
-  ? _DeepPartialObject<T>
-  : T | undefined;
+export type DeepPartial<T> = T extends ((...args: any[]) => any) | Primitive | null | undefined // Change Function to explicit declaration; Move functions/primitives to be handled in one place; null and undefined not needed after #113
+  ? T | undefined
+  : T extends _DeepPartialArray<any> // _DeepXXXArray<any> vs any[] described below
+  ? _DeepPartialArray<T[number]> // Don't need to infer on Arrays, can just index with number
+  : T extends _DeepPartialObject<infer U> // _DeepXXXObject<infer U> vs object described below
+  ? _DeepPartialObject<U>
+  : never; // all primitives and non-primitives are covered and this case will never happen
 /** @private */
 // tslint:disable-next-line:class-name
 export interface _DeepPartialArray<T> extends Array<DeepPartial<T>> {}

_DeepXXXArray<any> vs any[] and _DeepXXXObject<infer U> vs object: DeepReadonly<T>, DeepMutable<T>, and DeepPartial<T> use _DeepXXXArray<any> and _DeepXXXObject<infer U> for their array and object cases respectively.

However, DeepRequired<T> and DeepNonNullable<T> use any[] and object.

The reason to use _DeepXXXArray<any> and _DeepXXXObject<infer U> is because the following test fails with any[] and object: Using any[] and object, causes two _DeepReadonlyObject to show up in the snapshot.

● DeepReadonly › testType<DeepReadonly<DeepReadonly<NestedProps>>>() › (type) should match snapshot

  expect(value).toMatchSnapshot()
  
  Received value does not match stored snapshot 1.
  
- "_DeepReadonlyObject<{ first: { second: { name: string; }; }; }>"
+ "_DeepReadonlyObject<_DeepReadonlyObject<{ first: { second: { name: string; }; }; }>>"

DeepRequired<T> and DeepNonNullable<T> seem to only work with any[] and object because they use NonUndefined<T> and NonNullable<T> in their _DeepXXXArray<T> and _DeepXXXObject<T> definitions. I have to double check.

Who does this impact? Who is this for?

TypeScript users.

dosentmatter avatar Oct 21 '19 18:10 dosentmatter

Hey @dosentmatter, Thanks for great analysis and suggestions. I completely agree with you, that it would be great to have these types updated. Contributions are welcomed! ❤️

piotrwitek avatar Oct 21 '19 20:10 piotrwitek

I got hit by the current DeepRequired behavior. A nested type is incorrectly inferred as never. With the proposed fix it works.

ulken avatar Dec 02 '21 10:12 ulken

I got hit by the current DeepRequired behavior. A nested type is incorrectly inferred as never. With the proposed fix it works.

Got hit by this again. This time it converted T | U to T. And yet again the fix worked.

ulken avatar Jan 19 '22 08:01 ulken

Decided to switch to ts-essentials's DeepRequired instead of keeping a patched version in my code base.

ulken avatar Jan 28 '22 12:01 ulken