New type errors in Model.create with version 7.0.0

[coolsoftwaretyler]

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.

Originally reported in the v7.0.0 discussion post: #2224 (comment)

Sandbox link or minimal reproduction code

Here is a CodeSandbox reproduction.

On v6.0.1, we get a type error correctly: https://codesandbox.io/p/sandbox/naughty-bassi-forked-39c7cz?file=%2Fsrc%2Findex.ts&workspaceId=ws_QGa4cGacuoY7BgaNFKGDj8

On v7.0.0, MST will fail at runtime, but TypeScript does not warn us about that problem: https://codesandbox.io/p/sandbox/gv3tn2?file=%2Fsrc%2Findex.ts

Describe the expected behavior

We should receive type errors on the a property when passing a string to a types.number.

Describe the observed behavior

No type error in version 7.0.0

[thegedge]

Let me take a shot at fixing the mess I keep creating 😂 🙈

[thegedge]

I created a regression test here

Looks like it failed. I traced it to the version of TypeScript. Seems like the typechecking error shows up again in >=4.6.2. Is there a minimum TS we want to support, or should we try to make it work for <4.6.2?

[coolsoftwaretyler]

Let me take a shot at fixing the mess I keep creating

Haha, it's our mess, my friend.

The typescript version makes a ton of sense here, because I don't think I've seen this on apps I'm working on with MST 7 and TypeScript 5.

I haven't thought enough about TS version requirements yet. If there's an easy path for supporting an older version, it's worth it. But if you find it's taking too long to fix or is gonna spur more headaches, I think we can evaluate what a good TS target is. We haven't put thought into that for a while here, so it's worth consideration.

[thegedge]

👍 sounds good. I'll pick at it, see if I can't make something work in both versions.

I'll see if I can set up some kind of TS matrix to test older versions. I don't think it's worth going lower than 4, and I'll see the lowest minor version of 4 I can easily get working.

Once we figure out the min version, what do you think about using peer dependencies (and peerDependenciesMeta to make TS optional):

{
  "peerDependencies": {
    "typescript": ">=4.x.y"
  },
  "peerDependenciesMeta": {
    "typescript": {
      "optional": true
    }
  }
}

That way people will know any older version of TS is probably not gonna work. Looks like it was introduced in a later minor version of npm 6. That's more than 4 years old, so I'd say reasonable to expect at least that, eh?

[coolsoftwaretyler]

Yeah I feel good about that as a fallback!

[tm-3]

It's a little more than the typescript version. I'm using 5.7.2 and getting typing errors. I thought I was crazy for a bit because I could have sworn that Model.create() used to infer the type properly

Here's what I am seeing:

• TS =5.7.2
• MST > 7.0.0

Model.create does not properly type the create() method of the ModelType. This goes for maps, arrays, and models.

• TS = 5.7.2
• MST 6.0.1

As stated above everything works normally with MST 6.

The big difference is the use of ts-essentials in v7. I removed teh

From this:

/**
 * Creates an instance for the type given an snapshot input.
 *
 * @returns An instance of that type.
 */
create(snapshot?: C | ExcludeReadonly<T>, env?: any): this["Type"];

to this:

    /**
     * Creates an instance for the type given an snapshot input.
     *
     * @returns An instance of that type.
     */
    create(snapshot?: C , env?: any): this["Type"];

Everything worked as expected here with latest versions. Then I installed ts-essentials as a dependency while I was messing around with the ExcludeReadonly and the use of WritabelKeys from ts-essentials and noticed everything worked as expected. It's probably just not including the ts-essentials library when building declarations or something simple for someone who has more patience than I do with TS.

hth.

[coolsoftwaretyler]

@tm-3, that's super helpful, thank you!

I'm about to be on a cross country move, but by early 2025 I can look deeper into this and see what else we can do to fix things.

Sorry for the inconvenience!

[tm-3]

not a problem at all. Adding ts-essentials as a dependency works for now. If I get extra time I'll dig a bit deeper, but probably not until after Christmas.

Safe travels!

[thegedge]

Appreciate the digging deeper, @tm-3. I've been on vacation and now super busy prepping for the holidays, but I should have some time to look at this before the new year.

It's too bad that all of these changes have caused regressions, but we can capture all of these in typechecking tests and prevent it from happening again 🙌