`types.compose` does not override base properties for snapshot requirements.

[kylemeenehan]

Bug report

• I've checked documentation and searched for existing issues and discussions
• I've made sure my project is based on the latest MST version
• Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code

https://codesandbox.io/p/sandbox/dpzr23

Describe the expected behavior

When using types.compose, I would expect a child's properties to override its parent's properties. This seemed to work fine in previous versions of MST, but it doesn't seem to be the case for the latest version.

Describe the observed behavior

Given the following code:

import { types } from "mobx-state-tree";

enum Invariant {
    A = 'A',
    B = 'B'
}

const Base = types.model({
    name: types.string,
    invariant: types.enumeration(Object.values(Invariant))
});

const TypeA = types.compose(Base, types.model('TypeA', {
    invariant: Invariant.A
}));

const TypeB = types.compose(Base, types.model('TypeB', {
    invariant: types.literal(Invariant.B)
}));

const a = TypeA.create({
    name: 'Such a great name'
});

const b = TypeB.create({
    name: 'An even better name'
});

I would expect the invariant property to not be required when creating an instance of TypeA or TypeB. However, TypeScript complains and says that the snapshots provided to TypeA.create and TypeB.create are not valid.

Here is the error taken from the codesandbox reproduction:

Argument of type '{ name: string; }' is not assignable to parameter of type 'string | ModelCreationType<{ name: string; invariant: string; }> | undefined'.
  Type '{ name: string; }' is not assignable to type 'ModelCreationType<{ name: string; invariant: string; }>'.
    Property 'invariant' is missing in type '{ name: string; }' but required in type '{ name: string; invariant: string; }'.typescript(2345)

As I say above, I think this is a regression. However, please let me know if I've missed any recent changes in design decisions or misinterpreted any documentation.

Thanks for a great library! If I have some time I might try look into this, but I'm a bit swamped at the moment.

[kylemeenehan]

I should add that the reason it may appear as a regression to me is that we have slightly different TypeScript configuration in our new project (we're migrating code), so my comment that this appears to be a regression may be only due to my environment change.

[thegedge]

Hey, @kylemeenehan.

So this specific case has been a struggle for properly typing MST. Specifically, we do regular type intersection (i.e., A & B), and in that case string & optional(Invariant.A) will take the last common denominator, which removes the optionality:

I did attempt to fix this in #2218, but it unfortunately caused many more "excessively deep" issues and we reverted in #2234. My recommendation would be to 🔥 invariant from Base and only include it in the subtypes, or at least only do that for required params in any base that become optional in the subtype.

At my company, we've done a lot of stuff like this to overcome the issues:

const Base = <NameT extends string, ValueT extends string>(name: NameT, invariant: ValueT) => {
  return types.model(name, {
    name: types.string,
    invariant: types.optional(types.literal(invariant), invariant)
  });
}

const TypeA = Base("TypeA", Invariant.A);
const TypeB = Base("TypeB", Invariant.B);

I can look into why TypeA didn't infer the enum but instead a string, because string & enum of string values should be the enum, so maybe there's something we can do there (there are some tricks you can take to avoid "widening" which I can try).

Thanks for a great library!

❤ we're happy you like it :)

[kylemeenehan]

Hi @thegedge , thanks for the quick reply!

Okay, we'll rejig how this works. We don't actually need it in the snapshot data in the case that I'm dealing with now, so maybe we'll refactor this so that type is a view instead of being part of the actual model declaration. That way we can declare some default type on the base type so that we can still effectively simulate inheritance and expect type on all children of the parent.

Maybe we should add some documentation if this is expected behaviour for now?

[thegedge]

👍 we can clarify in documentation.

And sounds like what you have may be similar to what we did 🙂

const nodeType = <T extends string>(type: T) => types.optional(types.literal(type), type);
const modelKey = () => nanoid(12);

export const modelType = <Name extends string, Properties extends ModelPropertiesDeclaration = {}>(name: Name, options: Properties) =>
  types
    .model(name, {
      type: nodeType(name),
      key: types.optional(types.identifier, () => modelKey(name)),
      ...options,
    })
    .actions(debugActions);

That's our base for pretty much every MST model. We actually use the pattern of base types being functions with generic argument quite a bit, because of the various limitations.

But definitely, if you don't need it in the snapshot, you can likely derive something in a view :)