Skip to content

SnapshotIn Type unexpectedly changed behavior in MST version 7 (when upgrading from 6) #2254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jrmyio opened this issue Mar 25, 2025 · 4 comments
Labels
Typescript Issue related to Typescript typings

Comments

@jrmyio
Copy link
Contributor

jrmyio commented Mar 25, 2025

In MST 6 when you do SnapshotIn<typeof ParentType> it always returned a simple type. Since MST 7 any child / sub type inside ParentType seem to return their SnapshotOrInstance version instead of SnapshotIn of the child type.

I would expect SnapshotIn to always expect a simple JSON rather than allow instances to still be part of the Snapshot type.

Here you can see the MST 6 behavior:
https://codesandbox.io/p/sandbox/angry-vaughan-4zcrps

MST 7 behavior:
https://codesandbox.io/p/devbox/mst-6-forked-yx22tx

Where I think it makes it a mistake in above sandbox:
// UNEXPECTED: // getSomething() is valid here?

@thegedge
Copy link
Collaborator

Curious about that expectation, @jrmyio. Specifically, why don't think you an instance should be acceptable as an input snapshot? I ask because an instance (in most cases) satisfies the interface of an input snapshot.

Note that this choice was intentional. We implemented it in #2199, and you can see links to various discussions on that decision, #1568 (comment) in particular.

@thegedge thegedge added the Typescript Issue related to Typescript typings label Mar 25, 2025
@jrmyio
Copy link
Contributor Author

jrmyio commented Mar 25, 2025

Thanks for your quick reply. I agree it's good to support instances when using Type.create({}). I am not sure the type returned by SnapshotIn needs to be the exact same type as the first argument of the create function.

I am not so convinded one should call an object with possible instances in it a Snapshot. As far as I know, a Snapshot in MST always meant the json-like structure which is different from an instance. Functions like getSnapshot(), onSnapshot(), or types like SnapshotOut<> always return this JSON like structure.

Imo the MST 6 SnapshotIn type behavior made sense for its name.
The new SnapshotIn in MST 7 is more like a CreateType to me.

The reason I ran into issues is that I've been using this SnapshotIn type as the type that an external API would send to the client, or the type a cache layer would store before sending it to the client. This type is now more complicated, is no longer a strict structure, is no longer guaranteed to be JSON serializable and gets confusing when you want to utilize type completion.

Maybe another type helper could be added but I cannot think of a name other than SnapshotIn because to me that made perfect sense.

@coolsoftwaretyler
Copy link
Collaborator

Hey @jrmyio - I'm sorry about the mixup this caused on your end. That's the trouble with breaking changes.

I'm inclined to agree with @thegedge here (we talked about it at length at the time).

I don't think it makes sense to reverse the decision, but I'd be open to a PR to add another type helper, or I'd be curious if you can utilize SnapshotOut here instead?

@thegedge
Copy link
Collaborator

thegedge commented Mar 29, 2025

Alternatively, I may have been able to take a far more sophisticated approach, where I could derive an interface that matches the model instance type, but only has the identifier attribute. So something like:

const MyModel = types.model({
  myIdentifier: types.identifierNumber,
  tags: types.array(types.string),
  description: types.string,
})

// { myIdentifier: number }
type InstanceInput = AsCreationParam<MyModel>;

The idea would be that we'd have a fairly primitive interface, but Instance<typeof MyModel> would actually validate as an InstanceInput

[EDIT]
Nope, this probably isn't a wise thing to do, because the instantiate method wants a state tree node, so I think the type needs to reflect a statetree node, otherwise we're being incorrect:

const identifier = isStateTreeNode(initialValue)
? getIdentifier(initialValue)!
: initialValue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typescript Issue related to Typescript typings
Projects
None yet
Development

No branches or pull requests

3 participants