Fix model types when extending in any way.

__tests__/core/type-system.test.ts

@@ -1,38 +1,37 @@
+import { describe, expect, test } from "bun:test"
 import {
-  types,
-  getSnapshot,
-  unprotect,
-  getRoot,
-  getParent,
-  SnapshotOrInstance,
-  cast,
-  SnapshotIn,
-  Instance,
-  castToSnapshot,
-  IType,
-  isStateTreeNode,
-  isFrozenType,
-  TypeOfValue,
   IAnyType,
+  IType,
+  Instance,
   ModelPrimitive,
   ModelPropertiesDeclaration,
+  SnapshotIn,
+  SnapshotOrInstance,
   SnapshotOut,
-  type ISimpleType,
-  isOptionalType,
-  isUnionType,
-  type IOptionalIType,
-  type ITypeUnion,
-  isMapType,
+  TypeOfValue,
+  cast,
+  castToSnapshot,
+  getParent,
+  getRoot,
+  getSnapshot,
   isArrayType,
-  isModelType,
+  isFrozenType,
+  isIdentifierType,
+  isLateType,
   isLiteralType,
+  isMapType,
+  isModelType,
+  isOptionalType,
   isPrimitiveType,
   isReferenceType,
-  isIdentifierType,
   isRefinementType,
-  isLateType
+  isStateTreeNode,
+  isUnionType,
+  types,
+  unprotect,
+  type IOptionalIType,
+  type ISimpleType
 } from "../../src"
-import { describe, expect, test } from "bun:test"
 import type {
   DatePrimitive,
   IAnyComplexType,
@@ -1403,3 +1402,84 @@ test("#1627 - union dispatch function is typed", () => {
     types.null
   )
 })
+
+test("#2216 - should respect optionality when extending another type", () => {
+  const Base = types.model("ErrorStore", { value: types.string }).extend((self) => ({
+    actions: {
+      setValue(value?: string): boolean {
+        self.value = value || "test"
+        return true
+      },
+
+      setAnotherValue(value?: string): boolean {
+        self.value = value || "test"
+        return true
+      }
+    },
+    views: {
+      get spam(): string {
+        return self.value
+      },
+
+      get eggs(): string {
+        return self.value
+      }
+    },
+    state: {
+      anotherValue: "test" as string,
+      soManyValues: "test" as string
+    }
+  }))
+
+  const Extended = Base.named("Extended")
+    .props({
+      value: "test"
+    })
+    .extend((self) => ({
+      actions: {
+        setValue(value: string): number {
+          self.value = value
+          return value.length
+        }
+      },
+      views: {
+        get spam(): boolean {
+          return !!self.value
+        }
+      },
+      state: {
+        anotherValue: "test" as string | undefined
+      }
+    }))
+    .actions((self) => ({
+      setAnotherValue(value: string): number {
+        self.value = value
+        return value.length
+      }
+    }))
+    .views((self) => ({
+      get eggs(): boolean {
+        return !!self.value
+      }
+    }))
+    .volatile((self) => ({
+      soManyValues: "test" as string | undefined
+    }))
+
+  type InputSnapshot = SnapshotIn<typeof Extended>
+  type InstanceType = Instance<typeof Extended>
+
+  assertTypesEqual(_ as InputSnapshot, _ as { value?: string })
+  assertTypesEqual(
+    _ as InstanceType,
+    _ as {
+      value: string
+      setValue(value: string): number
+      setAnotherValue(value: string): number
+      spam: boolean
+      eggs: boolean
+      anotherValue: string | undefined
+      soManyValues: string | undefined
+    }
+  )
+})

src/types/complex-types/model.ts

@@ -1,61 +1,60 @@
 import {
+  IObjectDidChange,
+  IObjectWillChange,
   _getAdministration,
   _interceptReads,
   action,
   computed,
   defineProperty,
-  intercept,
   getAtom,
-  IObjectWillChange,
+  intercept,
+  makeObservable,
   observable,
   observe,
-  set,
-  IObjectDidChange,
-  makeObservable
+  set
 } from "mobx"
 import {
-  addHiddenFinalProp,
-  addHiddenWritableProp,
+  AnyNode,
+  AnyObjectNode,
   ArrayType,
   ComplexType,
-  createActionInvoker,
-  createObjectNode,
   EMPTY_ARRAY,
   EMPTY_OBJECT,
-  escapeJsonPath,
+  FunctionWithFlag,
+  Hook,
+  IAnyType,
+  IChildNodesMap,
+  IJsonPatch,
+  IType,
+  IValidationContext,
+  IValidationResult,
+  Instance,
+  MapType,
   MstError,
+  TypeFlags,
+  _CustomOrOther,
+  _NotCustomized,
+  addHiddenFinalProp,
+  addHiddenWritableProp,
+  assertArg,
+  assertIsString,
+  createActionInvoker,
+  createObjectNode,
+  devMode,
+  escapeJsonPath,
   flattenTypeErrors,
   freeze,
   getContextForPath,
   getPrimitiveFactoryFromValue,
   getStateTreeNode,
-  IAnyType,
-  IChildNodesMap,
-  IValidationContext,
-  IJsonPatch,
   isPlainObject,
   isPrimitive,
   isStateTreeNode,
   isType,
-  IType,
-  IValidationResult,
   mobxShallow,
   optional,
-  MapType,
-  typecheckInternal,
   typeCheckFailure,
-  TypeFlags,
-  Hook,
-  AnyObjectNode,
-  AnyNode,
-  _CustomOrOther,
-  _NotCustomized,
-  Instance,
-  devMode,
-  assertIsString,
-  assertArg,
-  FunctionWithFlag,
-  type IStateTreeNode
+  typecheckInternal
 } from "../../internal"
 
 const PRE_PROCESS_SNAPSHOT = "preProcessSnapshot"
@@ -74,6 +73,9 @@ export interface ModelPropertiesDeclaration {
   [key: string]: ModelPrimitive | IAnyType
 }
 
+/** intersect two object types, but omit keys of B from A before doing so */
+type OmitMerge<A, B> = Omit<A, keyof B> & B
+
 /**
  * Unmaps syntax property declarations to a map of { propName: IType }
  *
@@ -117,6 +119,8 @@ type IsOptionalValue<C, TV, FV> = undefined extends C ? TV : FV
 // type _E = IsOptionalValue<any, true, false> // true
 // type _F = IsOptionalValue<unknown, true, false> // true
 
+type AnyObject = Record<string, any>
+
 /**
  * Name of the properties of an object that can't be set to undefined, any or unknown
  * @hidden
@@ -199,23 +203,28 @@ export interface IModelType<
   // so it is recommended to use pre/post process snapshot after all props have been defined
   props<PROPS2 extends ModelPropertiesDeclaration>(
     props: PROPS2
-  ): IModelType<PROPS & ModelPropertiesDeclarationToProperties<PROPS2>, OTHERS, CustomC, CustomS>
+  ): IModelType<
+    OmitMerge<PROPS, ModelPropertiesDeclarationToProperties<PROPS2>>,
+    OTHERS,
+    CustomC,
+    CustomS
+  >
 
-  views<V extends Object>(
+  views<V extends AnyObject>(
     fn: (self: Instance<this>) => V
-  ): IModelType<PROPS, OTHERS & V, CustomC, CustomS>
+  ): IModelType<PROPS, OmitMerge<OTHERS, V>, CustomC, CustomS>
 
   actions<A extends ModelActions>(
     fn: (self: Instance<this>) => A
-  ): IModelType<PROPS, OTHERS & A, CustomC, CustomS>
+  ): IModelType<PROPS, OmitMerge<OTHERS, A>, CustomC, CustomS>
 
-  volatile<TP extends object>(
-    fn: (self: Instance<this>) => TP
-  ): IModelType<PROPS, OTHERS & TP, CustomC, CustomS>
+  volatile<VS extends AnyObject>(
+    fn: (self: Instance<this>) => VS
+  ): IModelType<PROPS, OmitMerge<OTHERS, VS>, CustomC, CustomS>
 
-  extend<A extends ModelActions = {}, V extends Object = {}, VS extends Object = {}>(
+  extend<A extends ModelActions = {}, V extends AnyObject = {}, VS extends AnyObject = {}>(
     fn: (self: Instance<this>) => { actions?: A; views?: V; state?: VS }
-  ): IModelType<PROPS, OTHERS & A & V & VS, CustomC, CustomS>
+  ): IModelType<PROPS, OmitMerge<OTHERS, A & V & VS>, CustomC, CustomS>
 
   preProcessSnapshot<NewC = ModelCreationType2<PROPS, CustomC>>(
     fn: (snapshot: NewC) => WithAdditionalProperties<ModelCreationType2<PROPS, CustomC>>
@@ -235,6 +244,7 @@ export interface IAnyModelType extends IModelType<any, any, any, any> {}
 export type ExtractProps<T extends IAnyModelType> = T extends IModelType<infer P, any, any, any>
   ? P
   : never
+
 /** @hidden */
 export type ExtractOthers<T extends IAnyModelType> = T extends IModelType<any, infer O, any, any>
   ? O
@@ -458,7 +468,7 @@ export class ModelType<
     return this.cloneAndEnhance({ properties })
   }
 
-  volatile<TP extends object>(fn: (self: Instance<this>) => TP) {
+  volatile<TP extends AnyObject>(fn: (self: Instance<this>) => TP) {
     if (typeof fn !== "function") {
       throw new MstError(
         `You passed an ${typeof fn} to volatile state as an argument, when function is expected`
@@ -483,7 +493,7 @@ export class ModelType<
     set(self, state)
   }
 
-  extend<A extends ModelActions = {}, V extends Object = {}, VS extends Object = {}>(
+  extend<A extends ModelActions = {}, V extends AnyObject = {}, VS extends AnyObject = {}>(
     fn: (self: Instance<this>) => { actions?: A; views?: V; state?: VS }
   ) {
     const initializer = (self: Instance<this>) => {
@@ -500,15 +510,15 @@ export class ModelType<
     return this.cloneAndEnhance({ initializers: [initializer] })
   }
 
-  views<V extends Object>(fn: (self: Instance<this>) => V) {
+  views<V extends AnyObject>(fn: (self: Instance<this>) => V) {
     const viewInitializer = (self: Instance<this>) => {
       this.instantiateViews(self, fn(self))
       return self
     }
     return this.cloneAndEnhance({ initializers: [viewInitializer] })
   }
 
-  private instantiateViews(self: this["T"], views: Object): void {
+  private instantiateViews(self: this["T"], views: AnyObject): void {
     // check views return
     if (!isPlainObject(views))
       throw new MstError(`views initializer should return a plain object containing views`)