Generic models

[sisp]

I've been working around the limitations of generic models for a while, but I keep arriving at the same obstacles. I'm wondering whether there is any way to further improve generic models, so creating model classes is closer to what I would write in plain Typescript independent of mobx-keystone.

The main problem is that I can't declare a generic model class with generic props:

class M<V extends number | string | boolean> extends Model({
  value: prop<V>() // TS ERROR
}) {}

Current solutions:

1. Use a model class factory and create model classes for all the generic types. This can lead to combinatorial explosion when composing models because I need to create as many models as I have combinations of generic types.
2. Use an abstract base class where generic props are plain abstract class properties, inherit from the base class and make the abstract class properties real model props with concrete types. This also leads to combinatorial explosion when composing models.

With runtime types, it seems the equivalent of generics is a factory, so there may not be a solution in that case. But when using prop<T>() there are no runtime types and I'm wondering whether there is a way to get better support for generic models in that case.

[xaviergonz]

A wild idea is that you could cache the "generic" class and reuse it

let cachedClass: any;

function createA<P>() {
  class A extends Model({
    value: prop<P>(),
  }) {
  }

  if (!cachedClass) {
    cachedClass = A
  }

  return cachedClass as any as A
}

// these two should use the same class, but their typing is ok
const StringA = createA<string>()
const NumberA = createA<number>()

The downside is that you'd create a new class per "generic", but it should be quickly garbage collected

[sisp]

Thanks for this interesting idea! 🙂

I'm trying to avoid materializing actual classes (or just differently typed aliases) for different "generics". Perhaps a slightly extended example makes my struggles clearer. This is a simplified example of what I'd like to achieve:

@model("M")
class M<V extends number | string | boolean> extends Model({
  value: prop<V>() // TS ERROR
}) {}

@model("A")
class A extends Model({
  x: prop<M<string>>()
}) {}

@model("B")
class B extends Model({
  y: prop<M<number | boolean>>()
}) {}

Here, M<T> is a generic model (a container if you will) and the models A and B all have a prop of type M but with different constraints on the generic T. This is purely a matter of typing, the transpiled JS code is identical. I would like to make sure that when I'm accessing the prop of any instance of A or B I have proper static type-checking (and IntelliSense in VS Code) about the type of value of M.

Currently, it seems I need to create a class for each variant of M (or an alias with your idea above), e.g.:

abstract class BaseM<T extends number | string | boolean> extends Model({}) {
  abstract value: T
}

// NOTE: Just for getting concretely typed `M`s, we have different registered models
// which is redundant.

@model("MString")
class MString extends ExtendedModel(modelClass<BaseM<string>>(BaseM), {
  value: prop<string>()
}) {}

@model("MNumberOrBoolean")
class MNumberOrBoolean extends ExtendedModel(modelClass<BaseM<number | boolean>>(BaseM), {
  value: prop<number | boolean>()
}) {}

@model("A")
class A extends Model({
  x: prop<MString>()
}) {}

@model("B")
class B extends Model({
  y: prop<MNumberOrBoolean>()
}) {}

Or with your idea which results in only one registered model "M":

let cachedClass: any

function createM<T extends number | string | boolean>() {
  class M extends Model({
    value: prop<T>()
  }) {}

  if (!cachedClass) {
    cachedClass = model("M")(M)
  }

  return cachedClass as any as M
}

const MString = createM<string>()
type MString = InstanceType<typeof MString>

const MNumberOrBoolean = createM<number | boolean>()
type MNumberOrBoolean = InstanceType<typeof MNumberOrBoolean>

@model("A")
class A extends Model({
  x: prop<MString>()
}) {}

@model("B")
class B extends Model({
  y: prop<MNumberOrBoolean>()
}) {}

Perhaps this could be a workaround:

abstract class BaseM<T extends number | string | boolean> extends Model({}) {
  abstract value: T
}

@model("A")
class A extends Model({
  x: prop<BaseM<string>>()
}) {}

@model("B")
class B extends Model({
  y: prop<BaseM<number | boolean>>()
}) {}

// The concrete subclasses for all "generics" still need to be created once
// `A` and `B` need to be instantiated.

The pattern of this workaround would be:

1. Create an abstract model class with generics that extends Model({}) (no model props specified yet).
2. If needed, create subclasses with generics by extending this base class without using ExtendedModel (so still no model props are specified).
3. Create a specific model subclass (without generics) for each "generic" ...
   1. ... using ExtendedModel(modelClass<...>(...), { ... }) and specify all model props there with the correct concrete types which leads to several registered models, or
   2. ... using your idea which leads to only one registered model. The downside of this approach is that the factory needs to be created for every generic model (slightly redundant code - maybe a higher-order/wrapper function could avoid repeating code) and that instanceof checks aren't easily done because there are only specific classes available (although they are all identical, so actually it's just one).
4. For typing props of other models, use the abstract base class (created in 1.) or an abstract subclass (created in 2.). For instantiating models, use the specific model subclasses (created in 3.).

[sisp]

Your TS proposal microsoft/TypeScript#36406 would solve this problem more elegantly, but there has been no activity.

[sisp]

Maybe something like this:

function createModel<F extends () => ModelClass<AnyModel>>(
  modelType: string,
  factory: F
): F {
  let cachedClass: any

  return (() => {
    const clazz = factory()

    if (!cachedClass) {
      cachedClass = model(modelType)(clazz)
    }

    return cachedClass
  }) as any
}

test("cached model factory with generics", () => {
  abstract class BaseM<T extends number | string> extends Model({}) {
    abstract value: T
  }

  const createM = createModel("M", <T extends number | string>() => {
    class M extends ExtendedModel(BaseM, {
      value: prop<T>(),
    }) {}

    return M
  })

  const M1 = createM<string>()
  // eslint-disable-next-line @typescript-eslint/no-redeclare
  type M1 = InstanceType<typeof M1>

  const M2 = createM<number>()
  // eslint-disable-next-line @typescript-eslint/no-redeclare
  type M2 = InstanceType<typeof M2>

  expect(M1).toBe(M2)

  assert(
    _ as ModelData<M1>,
    _ as { [modelIdKey]: string; value: string } & { [modelIdKey]: string }
  )
  assert(
    _ as ModelData<M2>,
    _ as { [modelIdKey]: string; value: number } & { [modelIdKey]: string }
  )
})

[xaviergonz]

Nice, that way you don't even create new classes :)
Btw, you shouldn't need modelClass<BaseM<T>>(BaseM) but just BaseM in this case

[sisp]

True, I've updated the snippet above.

Do you think this could be an "officially recommended" pattern for mobx-keystone? And do you think it's worth adding the createModel function to the library? Not sure if modelType (and thus decorating the class) should be optional, i.e. whether this function makes much sense when it returns an undecorated model class.

[xaviergonz]

I'm not sure. On the one hand it is useful for people who rely on TS for type checking, but on the other hand it introduces a separation between type checked and untype checked patterns and also I don't know how widespread is the case where you need many generics forms from a safe base.

About decorating the class inside the function, I think it makes sense (better be reminded of the decorator when calling the function than forgetting about it and seeing it error in runtime).

[sisp]

Hm, in general would you say that generic models are rather an anti-pattern because they don't work with runtime type-checking anyway? Should factories be preferred over generics?

[sisp]

Regarding real generic models, if there was a way to dynamically create a class with generics, I think we could do this:

class M<T> extends Model(<U>() => ({ value: prop<U>() }))<T> {}

Here, Model(<U>() => ({ value: prop<U>() })) would have to return a class with the generic U and then the generic T from the child class gets passed to the dynamically created base class. But I'm not sure if that's possible with Typescript.

[sisp]

Check this out:

// Generic "A" seems to be required or else the returned constructor won't be generic
function Model<A extends never, R extends Record<string, any>>(fn: (...args: A[]) => R): { new(): R } {
  return null as any
}

function prop<T>(): T {
  return null as any
}

class M<T extends number | string> extends Model(<T extends number | string>() => ({
  v: prop<T>()
}))<T> {}

const m1 = new M()         // number | string
const m2 = new M<number>() // number
const m3 = new M<string>() // string

TS Playground

Might be related to microsoft/TypeScript#30215.

I think it makes a lot of sense that something like

class M<T> extends Model({ value: prop<T>() }) {}

does not work because Model({ value: prop<T>() }) can be called outside the context of subclassing, e.g.

const BaseM = Model({ value: prop<T>() }) // Where is `T` coming from?

class M<T> extends BaseM {} // What happens to `T` here?

so when we want a generic model, Model(...) needs to return a generic class. Since Model is all about defining model props (and setting up lots of internal things of course), we need to be able to define generic model props and make sure the returned class has those generics, too.

What do you think about this idea?

[xaviergonz]

Nice idea. I got it working for Model, but unfortunately I didn't manage to get it working for ExtendedModel so far when the class being extended is a generic one (since the parameter for extended model is the class itself rather than the class type, which is the one that includes generics).

I'll keep experimenting (unless you think a generic base Model is good enough)

[sisp]

Nice idea. I got it working for Model [...]

Awesome! 🙂

[...] but unfortunately I didn't manage to get it working for ExtendedModel so far when the class being extended is a generic one (since the parameter for extended model is the class itself rather than the class type, which is the one that includes generics).

What would your ExtendedModel(...) call look like with generics? I was thinking about something like:

class Child<T> extends ExtendedModel(
  <T>() => ({
    baseClass: modelClass<Parent<T>>(Parent),
    props: {
      other: prop<T>()
    }
  })
)<T> {}

We need both the base class and the new props to be able to access the same generics, so both must be specified in the same generic arrow function. With modelClass<Parent<T>>(Parent) I think the correct type should be specified.

[xaviergonz]

Good call! I got this working in the end

test("new pattern for generics", () => {
  @model("GenericModel")
  class GenericModel<T1, T2> extends Model(<U1, U2>() => ({
    v1: prop<U1>(),
    v2: prop<U2>(),
    v3: prop<number>(0),
  }))<T1, T2> {}

  assert(
    _ as ModelData<GenericModel<string, number>>,
    _ as { [modelIdKey]: string; v1: string; v2: number; v3: number }
  )
  assert(
    _ as ModelData<GenericModel<number, string>>,
    _ as { [modelIdKey]: string; v1: number; v2: string; v3: number }
  )

  const s = new GenericModel<string, number>({ v1: "1", v2: 2, v3: 3 })
  expect(s.v1).toBe("1")
  expect(s.v2).toBe(2)
  expect(s.v3).toBe(3)

  @model("ExtendedGenericModel")
  class ExtendedGenericModel<T1, T2> extends ExtendedModel(<T1, T2>() => ({
    baseModel: modelClass<GenericModel<T1, T2>>(GenericModel),
    props: {
      v4: prop<T2>(),
    },
  }))<T1, T2> {}

  const e = new ExtendedGenericModel<string, number>({ v1: "1", v2: 2, v3: 3, v4: 4 })
  expect(e.v1).toBe("1")
  expect(e.v2).toBe(2)
  expect(e.v3).toBe(3)
  expect(e.v4).toBe(4)
})

just double checking, would that be missing something?