mobx-keystone icon indicating copy to clipboard operation
mobx-keystone copied to clipboard

Generic models with runtime type-checking

Open sisp opened this issue 4 years ago • 7 comments
trafficstars

In continuation of #239, I'd like to discuss an idea for generic models with runtime type-checking without using a class factory. In #242, support for truly generic models was added by means of a generic arrow function (without arguments) that enables returning generic props. I think that adding arguments to this generic arrow function, being the runtime types that correspond to the TS generics, would be the natural extension for having generic runtime type-checked models.

For instance:

  • Generic model (TS only):

    @model("myApp/GenericPoint")
    class GenericPoint<T> extends Model(<T>() => ({
      x: prop<T>(),
      y: prop<T>(),
    }))<T> {
      // ...
    }
    
    const point = new GenericPoint({ x: 1, y: 2 })
    
  • Generic model (TS + runtime type-checking):

    @model("myApp/GenericPoint")
    class GenericPoint<T> extends Model(<T>(valueType: TypeChecker<T>) => ({
      x: tProp(valueType),
      y: tProp(valueType),
    }))<T> {
      // ...
    }
    
    // The 2nd argument is an array of runtime types matching the arguments of the
    // arrow function above. In contrast to TS-only generic models, the runtime types
    // must be passed explicitly because they cannot be inferred.
    const point = new GenericPoint({ x: 1, y: 2 }, [types.integer])
    

    Note that TypeChecker<T> currently does not exist yet. TypeScript would derive the type of the 2nd argument of the constructor from the arguments of the arrow function and make sure compatible runtime type-checkers are provided in this array.

The sketched approach has some implications on snapshots and reconciliation. Model classes with "generic runtime types" are not uniquely/fully defined in contrast to non-generic model classes because here the same model class can be instantiated with different runtime types. This means the runtime types need to become snapshotable and the ones provided upon instantiation need to be stored in the snapshot along with the rest of the model snapshot.

For instance:

const pointSnapshot = getSnapshot(point)

could look like this:

{
  $modelType: "myApp/GenericPoint",
  $modelId: "...",
  $modelTypeCheckers: [
    {
      $checkerType: "mobx-keystone/types/integer<number>",
    }
  ]
}

Runtime types with arguments like types.or could be represented like this:

{
  $checkerType: "mobx-keystone/types/or",
  orTypes: [
    {
      $checkerType: "mobx-keystone/types/integer<number>",
    },
    {
      $checkerType: "mobx-keystone/types/string",
    }
  ]
}

Thus, runtime types like models would need to have globally unique type names and would need to be registered in a global registry, so that the reconciler can load a snapshot and instantiate the types from the snapshot again.

Refinements would require a globally unique name, so they can be stored in the registry, too. At the moment, the name is optional.

I'm curious about some feedback. :slightly_smiling_face:

sisp avatar Jun 28 '21 13:06 sisp

It sounds like a good initial approach. The only part I'm not 100% sold is forcing people to add unique names when creating their own types, but I guess they could just be optional and throw in runtime if those types are being used for a generic model...

Also I'd make the second parameter to new an options object, just in case it grows with more stuff in the future. e.g.

const point = new GenericPoint({ x: 1, y: 2 }, { genericTypes: [types.integer] })

Would you be up to give a try at implementing it? :)

xaviergonz avatar Jun 29 '21 22:06 xaviergonz

Great, thank you very much for your feedback! I'd be happy to give it a try. 🙂

sisp avatar Jun 30 '21 08:06 sisp

This would be my suggested API for ExtendedModel:

  • Generic extended model (TS only):
    @model("myApp/Generic3dPoint")
    class Generic3dPoint<T> extends ExtendedModel(<T>() => ({
      baseModel: modelClass<GenericPoint<T>>(GenericPoint),
      props: {
        z: prop<T>(),
      },
    }))<T> {
      // ...
    }
    
  • Generic extended model (TS + runtime type-checking):
    @model("myApp/Generic3dPoint")
    class Generic3dPoint<T> extends ExtendedModel(<T>(valueType: TypeChecker<T>) => ({
      baseModel: modelClass<GenericPoint<T>>(GenericPoint),
      baseModelGenericTypes: [valueType],
      props: {
        z: tProp(valueType),
      },
    }))<T> {
      // ...
    }
    

What do you think, @xaviergonz?

sisp avatar Jul 13 '21 16:07 sisp

looks good to me :) Maybe it would be possible to extract valueType from the argument of new instead of using baseModelGenericTypes ?

I mean, so calling new would inject baseModelGenericTypes itself

xaviergonz avatar Jul 15 '21 18:07 xaviergonz

I think this doesn't work in the general case. The mapping from the generic types of the extended class to those of the base class is not necessarily 1:1, so it must be possible to map the types explicitly. For instance, the extended class might only have one generic type while the base class has two, and perhaps the generic type of the extended class is assigned to both generic types of the base class. Or one generic type of the base class is hardcoded and the other is passed through from the extended class.

sisp avatar Jul 15 '21 20:07 sisp

lgtm then

xaviergonz avatar Jul 17 '21 18:07 xaviergonz

Apologies for the inactivity here. A quick update: I've started some first steps of implementing this feature, but realized it's actually not so straightforward.

Conceptually, I've been wondering whether passing generic runtime types in the model definition

... extends ExtendedModel(<T>(valueType: TypeChecker<T>) => ({
  baseModel: modelClass<GenericPoint<T>>(GenericPoint),
  baseModelGenericTypes: [valueType],
  // ...
}))<T> {
  // ...
}

is (a) the right approach and (b) sufficient because there are at least two other places that I can see now where something similar is needed:

  • types.model(clazz) would need to be extended to something like types.model(clazz, { genericTypes: [...] })
  • fromSnapshot(type, snapshot) would need to be extended to something like fromSnapshot(type, snapshot, { genericTypes: [...] })

When comparing this approach with TS types, type aliasing like

type NumberPoint = GenericPoint<number>

isn't possible, but perhaps if it were, e.g.

const NumberPoint = modelClass(GenericPoint, { genericTypes: [types.number] })

the API would also become simpler:

  • ... extends ExtendedModel(<T>(valueType: TypeChecker<T>) => ({
      baseModel: modelClass(GenericPoint, { genericTypes: [valueType] }),
      // ...
    }))<T> {
      // ...
    }
    
  • types.model(modelClass(GenericPoint, { genericTypes: [types.number] }))
    
    // or
    
    const NumberPoint = modelClass(GenericPoint, { genericTypes: [types.number] })
    types.model(NumberPoint)
    
  • fromSnapshot(modelClass(GenericPoint, { genericTypes: [types.number] }), snapshot)
    
    // or
    
    const NumberPoint = modelClass(GenericPoint, { genericTypes: [types.number] })
    fromSnapshot(NumberPoint, snapshot)
    

modelClass would create a kind of model class alias at runtime with the passed generic runtime types already set. I imagine this could be done by creating a subclass of the model class and assigning the runtime types to a property of the class type/constructor. But then the constructors of the "aliased" class and the original class wouldn't be identical; not sure if that's a problem.

Instantiating a generic model could be done in three ways:

  1. const point = new GenericPoint({ x: 1, y: 2 }, { genericTypes: [types.number] })
    
    // somewhat equivalent to
    const point = new GenericPoint<number>({ x: 1, y: 2})
    
    Requires a dedicated extension of the model class constructor though which could be avoided by options 2 and 3.
  2. const NumberPoint = modelClass(GenericPoint, { genericTypes: [types.number] })
    const point = new NumberPoint({ x: 1, y: 2 })
    
    // somewhat equivalent to
    const NumberPoint = GenericPoint as GenericPoint<number>
    const point = new NumberPoint({ x: 1, y: 2})
    
  3. const point = new modelClass(GenericPoint, { genericTypes: [types.number] })({ x: 1, y: 2 })
    
    // somewhat equivalent to
    const point = new GenericPoint<number>({ x: 1, y: 2})
    
    although a bit ugly :laughing:

sisp avatar Dec 18 '21 15:12 sisp