Improve typing for isXYZType narrowing functions (#2190)

Author: thegedge

__tests__/core/type-system.test.ts

@@ -19,9 +19,31 @@ import {
   ModelPrimitive,
   ModelPropertiesDeclaration,
   SnapshotOut,
-  type ISimpleType
+  type ISimpleType,
+  isOptionalType,
+  isUnionType,
+  type IOptionalIType,
+  type ITypeUnion,
+  isMapType,
+  isArrayType,
+  isModelType,
+  isLiteralType,
+  isPrimitiveType,
+  isReferenceType,
+  isIdentifierType,
+  isRefinementType,
+  isLateType
 } from "../../src"
 import { expect, test } from "bun:test"
+import type {
+  DatePrimitive,
+  IAnyComplexType,
+  IAnyModelType,
+  IArrayType,
+  IMapType,
+  IReferenceType,
+  IUnionType
+} from "../../src/internal"
 
 type DifferingKeys<ActualT, ExpectedT> = {
   [K in keyof ActualT | keyof ExpectedT]: K extends keyof ActualT
@@ -34,22 +56,18 @@ type DifferingKeys<ActualT, ExpectedT> = {
 }[keyof ActualT | keyof ExpectedT] &
   string
 
-type NotExactErrorMessage<ActualT, ExpectedT> = ActualT extends Record<string, unknown>
-  ? ExpectedT extends Record<string, unknown>
+type NotExactErrorMessage<ActualT, ExpectedT> = ActualT extends Record<string, any>
+  ? ExpectedT extends Record<string, any>
     ? `Mismatched property: ${DifferingKeys<ActualT, ExpectedT>}`
     : "Expected a non-object type, but received an object"
-  : ExpectedT extends Record<string, unknown>
+  : ExpectedT extends Record<string, any>
   ? "Expected an object type, but received a non-object type"
   : "Types are not exactly equal"
 
-type IsExact<T1, T2> = [T1] extends [T2] ? ([T2] extends [T1] ? Exact<T1, T2> : never) : never
-
 const assertTypesEqual = <ActualT, ExpectedT>(
   t: ActualT,
-  u: IsExact<ActualT, ExpectedT> extends never
-    ? NotExactErrorMessage<ActualT, ExpectedT>
-    : ExpectedT
-): [ActualT, ExpectedT] => [t, u] as [ActualT, ExpectedT]
+  u: Exact<ActualT, ExpectedT> extends never ? NotExactErrorMessage<ActualT, ExpectedT> : ExpectedT
+): [ActualT, ExpectedT] => [t, u] as any
 
 const _: unknown = undefined
 
@@ -1281,3 +1299,36 @@ test("#2186 substitutability type verification for model types extending a commo
       SubTypeRequired
     )
 })
+
+test("#2184 - type narrowing functions should narrow to the expected type", () => {
+  const type: unknown = null
+
+  if (isOptionalType(type)) {
+    assertTypesEqual(type, _ as IOptionalIType<IAnyType, [any, ...any[]]>)
+  } else if (isUnionType(type)) {
+    assertTypesEqual(type, _ as IUnionType<IAnyType[]>)
+  } else if (isFrozenType(type)) {
+    assertTypesEqual(type, _ as ISimpleType<any>)
+  } else if (isMapType(type)) {
+    assertTypesEqual(type, _ as IMapType<IAnyType>)
+  } else if (isArrayType(type)) {
+    assertTypesEqual(type, _ as IArrayType<IAnyType>)
+  } else if (isModelType(type)) {
+    assertTypesEqual(type, _ as IAnyModelType)
+  } else if (isLiteralType(type)) {
+    assertTypesEqual(type, _ as ISimpleType<any>)
+  } else if (isPrimitiveType(type)) {
+    assertTypesEqual(
+      type,
+      _ as ISimpleType<string> | ISimpleType<number> | ISimpleType<boolean> | typeof DatePrimitive
+    )
+  } else if (isReferenceType(type)) {
+    assertTypesEqual(type, _ as IReferenceType<IAnyComplexType>)
+  } else if (isIdentifierType(type)) {
+    assertTypesEqual(type, _ as ISimpleType<string> | ISimpleType<number>)
+  } else if (isRefinementType(type)) {
+    assertTypesEqual(type, _ as IAnyType)
+  } else if (isLateType(type)) {
+    assertTypesEqual(type, _ as IAnyType)
+  }
+})

bun.lockb

@@ -505,8 +505,6 @@ function areSame(oldNode: AnyNode, newValue: any) {
  * @param type
  * @returns `true` if the type is an array type.
  */
-export function isArrayType<Items extends IAnyType = IAnyType>(
-  type: IAnyType
-): type is IArrayType<Items> {
+export function isArrayType(type: unknown): type is IArrayType<IAnyType> {
   return isType(type) && (type.flags & TypeFlags.Array) > 0
 }

src/types/complex-types/array.ts

@@ -509,8 +509,6 @@ export function map<IT extends IAnyType>(subtype: IT): IMapType<IT> {
  * @param type
  * @returns `true` if it is a map type.
  */
-export function isMapType<Items extends IAnyType = IAnyType>(
-  type: IAnyType
-): type is IMapType<Items> {
+export function isMapType(type: unknown): type is IMapType<IAnyType> {
   return isType(type) && (type.flags & TypeFlags.Map) > 0
 }

src/types/complex-types/map.ts

@@ -81,20 +81,20 @@ export interface ModelPropertiesDeclaration {
  */
 export type ModelPropertiesDeclarationToProperties<T extends ModelPropertiesDeclaration> =
   T extends { [k: string]: IAnyType } // optimization to reduce nesting
-  ? T
-  : {
-    [K in keyof T]: T[K] extends IAnyType // keep IAnyType check on the top to reduce nesting
-    ? T[K]
-    : T[K] extends string
-    ? IType<string | undefined, string, string>
-    : T[K] extends number
-    ? IType<number | undefined, number, number>
-    : T[K] extends boolean
-    ? IType<boolean | undefined, boolean, boolean>
-    : T[K] extends Date
-    ? IType<number | Date | undefined, number, Date>
-    : never
-  }
+    ? T
+    : {
+        [K in keyof T]: T[K] extends IAnyType // keep IAnyType check on the top to reduce nesting
+          ? T[K]
+          : T[K] extends string
+          ? IType<string | undefined, string, string>
+          : T[K] extends number
+          ? IType<number | undefined, number, number>
+          : T[K] extends boolean
+          ? IType<boolean | undefined, boolean, boolean>
+          : T[K] extends Date
+          ? IType<number | Date | undefined, number, Date>
+          : never
+      }
 
 /**
  * Checks if a value is optional (undefined, any or unknown).
@@ -124,7 +124,9 @@ type IsOptionalValue<C, TV, FV> = undefined extends C ? TV : FV
 type DefinablePropsNames<T> = { [K in keyof T]: IsOptionalValue<T[K], never, K> }[keyof T]
 
 /** @hidden */
-export type ExtractCFromProps<P extends ModelProperties> = MaybeEmpty<{ [k in keyof P]: P[k]["CreationType"] }>
+export type ExtractCFromProps<P extends ModelProperties> = MaybeEmpty<{
+  [k in keyof P]: P[k]["CreationType"]
+}>
 
 /** @hidden */
 export type MaybeEmpty<T> = keyof T extends never ? EmptyObject : T
@@ -147,7 +149,7 @@ export type ModelCreationType2<P extends ModelProperties, CustomC> = MaybeEmpty<
   keyof P extends never
     ? _CustomOrOther<CustomC, ModelCreationType<EmptyObject>>
     : _CustomOrOther<CustomC, ModelCreationType<ExtractCFromProps<P>>>
->;
+>
 
 /** @hidden */
 export type ModelSnapshotType<P extends ModelProperties> = {
@@ -227,7 +229,7 @@ export interface IModelType<
 /**
  * Any model type.
  */
-export interface IAnyModelType extends IModelType<any, any, any, any> { }
+export interface IAnyModelType extends IModelType<any, any, any, any> {}
 
 /** @hidden */
 export type ExtractProps<T extends IAnyModelType> = T extends IModelType<infer P, any, any, any>
@@ -274,78 +276,82 @@ function toPropertiesObject(declaredProps: ModelPropertiesDeclaration): ModelPro
   })
 
   // loop through properties and ensures that all items are types
-  return keysList.reduce((props, key) => {
-    // warn if user intended a HOOK
-    if (key in Hook) {
-      throw new MstError(
-        `Hook '${key}' was defined as property. Hooks should be defined as part of the actions`
-      )
-    }
+  return keysList.reduce(
+    (props, key) => {
+      // warn if user intended a HOOK
+      if (key in Hook) {
+        throw new MstError(
+          `Hook '${key}' was defined as property. Hooks should be defined as part of the actions`
+        )
+      }
 
-    // the user intended to use a view
-    const descriptor = Object.getOwnPropertyDescriptor(declaredProps, key)!
-    if ("get" in descriptor) {
-      throw new MstError("Getters are not supported as properties. Please use views instead")
-    }
-    // undefined and null are not valid
-    const value = descriptor.value
-    if (value === null || value === undefined) {
-      throw new MstError(
-        "The default value of an attribute cannot be null or undefined as the type cannot be inferred. Did you mean `types.maybe(someType)`?"
-      )
-    }
-    // its a primitive, convert to its type
-    else if (isPrimitive(value)) {
-      props[key] = optional(getPrimitiveFactoryFromValue(value), value)
-    }
-    // map defaults to empty object automatically for models
-    else if (value instanceof MapType) {
-      props[key] = optional(value, {})
-    } else if (value instanceof ArrayType) {
-      props[key] = optional(value, [])
-    }
-    // its already a type
-    else if (isType(value)) {
-      // do nothing, it's already a type
-    }
-    // its a function, maybe the user wanted a view?
-    else if (devMode() && typeof value === "function") {
-      throw new MstError(
-        `Invalid type definition for property '${key}', it looks like you passed a function. Did you forget to invoke it, or did you intend to declare a view / action?`
-      )
-    }
-    // no other complex values
-    else if (devMode() && typeof value === "object") {
-      throw new MstError(
-        `Invalid type definition for property '${key}', it looks like you passed an object. Try passing another model type or a types.frozen.`
-      )
-    } else {
-      throw new MstError(
-        `Invalid type definition for property '${key}', cannot infer a type from a value like '${value}' (${typeof value})`
-      )
-    }
+      // the user intended to use a view
+      const descriptor = Object.getOwnPropertyDescriptor(declaredProps, key)!
+      if ("get" in descriptor) {
+        throw new MstError("Getters are not supported as properties. Please use views instead")
+      }
+      // undefined and null are not valid
+      const value = descriptor.value
+      if (value === null || value === undefined) {
+        throw new MstError(
+          "The default value of an attribute cannot be null or undefined as the type cannot be inferred. Did you mean `types.maybe(someType)`?"
+        )
+      }
+      // its a primitive, convert to its type
+      else if (isPrimitive(value)) {
+        props[key] = optional(getPrimitiveFactoryFromValue(value), value)
+      }
+      // map defaults to empty object automatically for models
+      else if (value instanceof MapType) {
+        props[key] = optional(value, {})
+      } else if (value instanceof ArrayType) {
+        props[key] = optional(value, [])
+      }
+      // its already a type
+      else if (isType(value)) {
+        // do nothing, it's already a type
+      }
+      // its a function, maybe the user wanted a view?
+      else if (devMode() && typeof value === "function") {
+        throw new MstError(
+          `Invalid type definition for property '${key}', it looks like you passed a function. Did you forget to invoke it, or did you intend to declare a view / action?`
+        )
+      }
+      // no other complex values
+      else if (devMode() && typeof value === "object") {
+        throw new MstError(
+          `Invalid type definition for property '${key}', it looks like you passed an object. Try passing another model type or a types.frozen.`
+        )
+      } else {
+        throw new MstError(
+          `Invalid type definition for property '${key}', cannot infer a type from a value like '${value}' (${typeof value})`
+        )
+      }
 
-    return props
-  }, { ...declaredProps } as any)
+      return props
+    },
+    { ...declaredProps } as any
+  )
 }
 
 /**
  * @internal
  * @hidden
  */
 export class ModelType<
-  PROPS extends ModelProperties,
-  OTHERS,
-  CustomC,
-  CustomS,
-  MT extends IModelType<PROPS, OTHERS, CustomC, CustomS>
->
+    PROPS extends ModelProperties,
+    OTHERS,
+    CustomC,
+    CustomS,
+    MT extends IModelType<PROPS, OTHERS, CustomC, CustomS>
+  >
   extends ComplexType<
     ModelCreationType2<PROPS, CustomC>,
     ModelSnapshotType2<PROPS, CustomS>,
     ModelInstanceType<PROPS, OTHERS>
   >
-  implements IModelType<PROPS, OTHERS, CustomC, CustomS> {
+  implements IModelType<PROPS, OTHERS, CustomC, CustomS>
+{
   readonly flags = TypeFlags.Object
 
   /*
@@ -439,8 +445,8 @@ export class ModelType<
       const actionInvoker = createActionInvoker(self as any, name, boundAction)
       actions[name] = actionInvoker
 
-        // See #646, allow models to be mocked
-        ; (!devMode() ? addHiddenFinalProp : addHiddenWritableProp)(self, name, actionInvoker)
+      // See #646, allow models to be mocked
+      ;(!devMode() ? addHiddenFinalProp : addHiddenWritableProp)(self, name, actionInvoker)
     })
   }
 
@@ -515,7 +521,7 @@ export class ModelType<
       } else if (typeof descriptor.value === "function") {
         // this is a view function, merge as is!
         // See #646, allow models to be mocked
-        ; (!devMode() ? addHiddenFinalProp : addHiddenWritableProp)(self, key, descriptor.value)
+        ;(!devMode() ? addHiddenFinalProp : addHiddenWritableProp)(self, key, descriptor.value)
       } else {
         throw new MstError(`A view member should either be a function or getter based property`)
       }
@@ -645,7 +651,7 @@ export class ModelType<
       try {
         // TODO: FIXME, make sure the observable ref is used!
         const atom = getAtom(node.storedValue, name)
-          ; (atom as any).reportObserved()
+        ;(atom as any).reportObserved()
       } catch (e) {
         throw new MstError(`${name} property is declared twice`)
       }
@@ -669,14 +675,14 @@ export class ModelType<
     if (!(patch.op === "replace" || patch.op === "add")) {
       throw new MstError(`object does not support operation ${patch.op}`)
     }
-    ; (node.storedValue as any)[subpath] = patch.value
+    ;(node.storedValue as any)[subpath] = patch.value
   }
 
   applySnapshot(node: this["N"], snapshot: this["C"]): void {
     typecheckInternal(this, snapshot)
     const preProcessedSnapshot = this.applySnapshotPreProcessor(snapshot)
     this.forAllProps((name) => {
-      ; (node.storedValue as any)[name] = preProcessedSnapshot[name]
+      ;(node.storedValue as any)[name] = preProcessedSnapshot[name]
     })
   }
 
@@ -732,7 +738,7 @@ export class ModelType<
   }
 
   removeChild(node: this["N"], subpath: string) {
-    ; (node.storedValue as any)[subpath] = undefined
+    ;(node.storedValue as any)[subpath] = undefined
   }
 }
 ModelType.prototype.applySnapshot = action(ModelType.prototype.applySnapshot)
@@ -853,6 +859,6 @@ export function compose(...args: any[]): any {
  * @param type
  * @returns
  */
-export function isModelType<IT extends IAnyModelType = IAnyModelType>(type: IAnyType): type is IT {
+export function isModelType(type: unknown): type is IAnyModelType {
   return isType(type) && (type.flags & TypeFlags.Object) > 0
 }

src/types/complex-types/model.ts

@@ -238,9 +238,9 @@ export function getPrimitiveFactoryFromValue(value: any): ISimpleType<any> {
  * @param type
  * @returns
  */
-export function isPrimitiveType<
-  IT extends ISimpleType<string> | ISimpleType<number> | ISimpleType<boolean> | typeof DatePrimitive
->(type: IT): type is IT {
+export function isPrimitiveType(
+  type: unknown
+): type is ISimpleType<string> | ISimpleType<number> | ISimpleType<boolean> | typeof DatePrimitive {
   return (
     isType(type) &&
     (type.flags &