mobx-state-tree
mobx-state-tree copied to clipboard
Unexpected patches for Dates with applySnapshot
Bug report
- [x] I've checked documentation and searched for existing issues and discussions
- [x] I've made sure my project is based on the latest MST version
- [x] Fork this code sandbox or another minimal reproduction.
Sandbox link or minimal reproduction code
https://codesandbox.io/p/sandbox/mst-autorun-example-forked-7ds6cs?file=%2Findex.ts%3A1%2C1-35%2C1&workspaceId=84a0645a-1491-4074-88e5-f44a8818ef46
Describe the expected behavior
When running applySnapshot without changing the value for a types.Date, I expect no patches to be emitted.
Describe the observed behavior
Patches are emitted even though the values haven't changed.
Other notes
I can guess at the implementation here and why the patch is being emitted: new Date(123) != new Date(123).
What I'm wondering is how far we should go to minimize the patches 🤔
Do we close this as "working as intended"? If not, should we avoid emitting a patch for a types.frozen with the value { value: "a" } and having a snapshot applied with the same value (but a difference instance)?
Looks related to https://github.com/mobxjs/mobx-state-tree/issues/2080
We have that marked as a breaking change and I haven't released v7 yet so maybe we consider this and bundle it there...
Ah, good catch @coolsoftwaretyler. I was focused on looking for generic snapshot issues, not something specific to dates.
Separate from that, we can perhaps leave this issue to discuss if we want the equality to go any deeper. Could users somehow hook into types.frozen with a "comparer" to avoid generating patches when nothing is changing?
Yeah I like that. We could maybe even avoid a breaking change here by writing something like that. If we provide custom comparators for certain data types (or all perhaps?) - we could preserve the default behavior as-is, and allow users to opt-in to another behavior.
I'm going to mark this as an enhancement for now.