Broken model types in 6.0.0 version

[dfilatov]

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

const BaseModel = types.model();
type BaseModelType = typeof BaseModel;

const DerivedModel = BaseModel.props({ a: '' });

const test: BaseModelType = DerivedModel;

Describe the expected behavior

No type errors

Describe the observed behavior

Type 'IModelType<{ a: IType<string | undefined, string, string>; }, {}, _NotCustomized, _NotCustomized>' is not assignable to type 'IModelType<{}, {}, _NotCustomized, _NotCustomized>'.
  The types of 'views(...).CreationType' are incompatible between these types.
    Type 'ModelCreationType<ExtractCFromProps<{ a: IType<string | undefined, string, string>; }>>' has no properties in common with type 'IStateTreeNode<IAnyType>'.ts(2322)

It has been broken in 6.0.0 version. It works in 5.4.1 version as expected.

[coolsoftwaretyler]

Thanks for the bug report @dfilatov! Sorry for the inconvenience. I will look into this soon.

[coolsoftwaretyler]

Hi @dfilatov - what TypeScript version are you using? I'm having trouble reproducing. Here's a CodeSandbox without errors: https://codesandbox.io/p/sandbox/issue-2186-reproduction-dgy9g3

Version 6.0.0 upgraded TypeScript to 5.3.3 which seems a possible culprit if you're seeing issues locally.

[dfilatov]

Hi @dfilatov - what TypeScript version are you using?

5.4.5

[dfilatov]

I've opened your link and see error:

[coolsoftwaretyler]

Oh nice. Must have been a bug on CodeSandbox.

Will continue investigating!

[coolsoftwaretyler]

I ran git bisect on the reproduction code you provided and it identified 46334b6 as the commit that introduced the error.

I wonder if WithAdditionalProperties here: 46334b6#diff-8ce65433bb1651566fa2253bfc8167953b5871ef77ed073d1d47d0e4876fabdcR217 is the culprit?

Will keep investigating!

[coolsoftwaretyler]

Ok, looks like the issue is with ModelCreationType2 and its changes. Here is a commit that fixes your reported issue: 86c8cf3#diff-8ce65433bb1651566fa2253bfc8167953b5871ef77ed073d1d47d0e4876fabdcR139 (it's a little big because I'm on a new machine that doesn't seem to respect our formatting right now, my bad), but it causes other type specifications in the test suite to fail.

@dfilatov - are you only encountering this issue specifically when you create a base model with no properties? I want to figure out if this is happening because empty models are a special case or if there's a bigger problem with the props builder on models.

Let me know!

[dfilatov]

@dfilatov - are you only encountering this issue specifically when you create a base model with no properties?

yep, looks like it's happened with base models without own props (but with own actions/views)

[coolsoftwaretyler]

Very helpful! I think we just need to figure out some way to carve out the types to accommodate a model like that.

As a stopgap while we figure it out, I might suggest adding some kind of placeholder property in that model definition to clear the error if you don't want to ts-ignore/ts-expect-error

[coolsoftwaretyler]

The root of the problem is, I believe, here:

export type ModelCreationType2<P extends ModelProperties, CustomC> = keyof P extends never
  ? // When there are no props, we want to prevent passing in any object. We have two objects we want to allow:
  //  1. The empty object
  //  2. An instance of this model
  //
  // The `IStateTreeNode` interface allows both. For (1), these props are optional so an empty object is allowed.
  // For (2), an instance will contain these two props, including the "secret" `$stateTreeNodeType` prop. TypeScript's
  // excess property checking will then ensure no other props are passed in.
  IStateTreeNode
  : _CustomOrOther<CustomC, ModelCreationType<ExtractCFromProps<P>>>

Introduced here: 46334b6#diff-8ce65433bb1651566fa2253bfc8167953b5871ef77ed073d1d47d0e4876fabdcR139

If we go back to the old definition of ModelCreationType2, these tests begin failing, which I think may be a worse state, because it would allow people to instantiate models with non-existent properties without TypeScript warning them of an issue there.

I brought this up in the maintainers' Discord channel because I'm not sure how to reconcile the old behavior with the new. If anyone out there has a good TypeScript idea, let me know.

[coolsoftwaretyler]

There is one workaround that exists today. If you use the Instance type alias on typeof BaseModel, TypeScript is happy about it:

  const BaseModel = types.model()
  type BaseModelType = Instance<typeof BaseModel>

  const DerivedModel = BaseModel.props({ a: "" })

  const test: BaseModelType = DerivedModel

Here's no red lines in VS Code:

I would say this is actually more correct anyway, as typeof MODEL.type is the "legacy" way to do this. I know that's different than your original code snippet, but I'm feeling pretty good about just recommending the use of Instance. See: https://mobx-state-tree.js.org/tips/typescript#using-a-mst-type-at-design-time

[coolsoftwaretyler]

I suppose it isn't exactly correct to say that this is an Instance, since it's an extension of the definition.

Still, this makes me think we can maintain the TypeScript changes from 6.0.0 and make another type alias available that you can use to opt into the second clause of the conditional type. I will think some more on it but this feels like a viable path forward.

[thegedge]

I suppose it isn't exactly correct to say that this is an Instance, since it's an extension of the definition.

💯

I do think this should be permitted. I'll try to get this fixed sometime this weekend and make sure we expand the test suite for types.

Thanks for the report, @dfilatov!

[coolsoftwaretyler]

Should be fixed in #2189, which I'll merge and release sometime this weekend or early next week.

[coolsoftwaretyler]

Just merged! This will be out in a patch release soon.

[dfilatov]

Just merged! This will be out in a patch release soon.

Great, thank you!