mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard

MSTMap does not have `observe` method, but IMSTMap does

Open n9 opened this issue 5 months ago • 12 comments

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/devbox/nameless-shadow-987q7v?workspaceId=ws_V6MwkdyBVdYpA4KMGgF9f5

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

const Tweet = types
  .model("Tweet", {
    body: types.string,
    read: false, // automatically inferred as type "boolean" with default "false"
  })
  .actions((tweet) => ({
    toggle() {
      tweet.read = !tweet.read;
    },
  }));

const TwitterStore = types.model("TwitterStore", {
  tweets: types.map(Tweet),
});

// create an instance from a snapshot
const twitterStore = TwitterStore.create();

twitterStore.tweets.observe((c) => {}); // 🐞 twitterStore.tweets.observe is not a function

Describe the expected behavior

It will not crash.

Describe the observed behavior

It crashes with twitterStore.tweets.observe is not a function.

Thoughts

I assume the cause is that the ObservableMap no longer have the observe method.

PS: Maybe MSTMap should also implement IMSTMap?

n9 avatar Jul 23 '25 14:07 n9

As a workaround, I tried MobX's observe function, but it crashed when I tried to get a snapshot from the newValue arg:

https://codesandbox.io/p/devbox/sharp-cherry-6fc4pv?workspaceId=ws_V6MwkdyBVdYpA4KMGgF9f5

import { getSnapshot, types } from "mobx-state-tree";
import { observe } from "mobx";

const Tweet = types
  .model("Tweet", {
    body: types.string,
    read: false,
  })
  .actions((tweet) => ({
    toggle() {
      tweet.read = !tweet.read;
    },
  }));

const TwitterStore = types
  .model("TwitterStore", {
    tweets: types.map(Tweet),
  })
  .actions((store) => ({
    add(id, tw) {
      store.tweets.set(id, tw);
    },
  }));

const twitterStore = TwitterStore.create();

observe(twitterStore.tweets, (ch) => {
  try {
    getSnapshot(ch.newValue); // 🐞 this will crash
    console.log(`change snapshot ok`);
  } catch (e) {
    console.error(`change ${ch.type} ${ch.name} crash ${e}`);
  }
});

const id = "foo";

twitterStore.add(
  "foo",
  Tweet.create({
    body: "asda",
  })
);

try {
  getSnapshot(twitterStore.tweets.get("foo"));
  console.log(`get snapshot ok`);
} catch (e) {
  console.error(`get crash ${e}`);
}

How to get snapshot from the observe's newValue?

n9 avatar Jul 23 '25 18:07 n9

Hey @n9 - thanks for the bug report.

Can you please make your reproduction sandbox public? I can't view it right now.

coolsoftwaretyler avatar Jul 24 '25 13:07 coolsoftwaretyler

@coolsoftwaretyler Done.

n9 avatar Jul 24 '25 14:07 n9

Good catch @n9. Looks like you're right.

The quickest patch here would be for us to update the typings to reflect that it doesn't exist.

As for using observe - have you tried with reaction instead? Using observer is an anti-pattern in MobX

coolsoftwaretyler avatar Jul 24 '25 20:07 coolsoftwaretyler

@coolsoftwaretyler How can I use reaction on an observable map to get key and value when the map changes?

n9 avatar Jul 25 '25 10:07 n9

@n9 - I think something like this would work:

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

const Tweet = types
  .model("Tweet", {
    body: types.string,
    read: false, // automatically inferred as type "boolean" with default "false"
  })
  .actions((tweet) => ({
    toggle() {
      tweet.read = !tweet.read;
    },
  }));

const TwitterStore = types
  .model("TwitterStore", {
    tweets: types.map(Tweet),
  })
  .actions((self) => ({
    createTweet() {
      self.tweets.set("1", { body: "Hello" });
    },
  }));

// create an instance from a snapshot
const twitterStore = TwitterStore.create();

reaction(
  () => Array.from(twitterStore.tweets.entries()),
  (entries, previousEntries) => {
    console.log("Tweets changed:");
    console.log("Current entries:", entries);
    console.log("Previous entries:", previousEntries);
  }
);

twitterStore.createTweet();

I see this output:

Image

Here is a codesandbox.

coolsoftwaretyler avatar Jul 25 '25 13:07 coolsoftwaretyler

@coolsoftwaretyler But it will simply dump all entries.

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

const Tweet = types
  .model("Tweet", {
    body: types.string,
    read: false, // automatically inferred as type "boolean" with default "false"
  })
  .actions((tweet) => ({
    toggle() {
      tweet.read = !tweet.read;
    },
  }));

const TwitterStore = types
  .model("TwitterStore", {
    tweets: types.map(Tweet),
  })
  .actions((self) => ({
    createTweet(id, body) {
      self.tweets.set(id, { body });
    },
  }));

let i = 0;
// create an instance from a snapshot
const twitterStore = TwitterStore.create({
  tweets: Object.fromEntries(
    [...new Array(100000)].map(() => [
      `key${i++}`,
      {
        body: `body${i}`,
      },
    ])
  ),
});

reaction(
  () => Array.from(twitterStore.tweets.entries()),
  (entries, previousEntries) => {
    console.log("Tweets changed:");
    console.log("Current entries:", entries);
    console.log("Previous entries:", previousEntries);
  }
);

twitterStore.createTweet(`key${i++}`, `body${i}`);

Tweets changed: Current entries: (100001) [Array(2), Array(2), Array(2), Array(2), Array(2), …] Previous entries: (100000) [Array(2), Array(2), Array(2), Array(2), Array(2), …]

How can I get only the changed entries? This will require comparing lists.

n9 avatar Jul 25 '25 16:07 n9

Ah I see what you mean. This is probably a good use case for a patch listener:

import { types, onPatch } from "mobx-state-tree";
import { reaction } from "mobx";

const Tweet = types
  .model("Tweet", {
    body: types.string,
    read: false, // automatically inferred as type "boolean" with default "false"
  })
  .actions((tweet) => ({
    toggle() {
      tweet.read = !tweet.read;
    },
  }));

const TwitterStore = types
  .model("TwitterStore", {
    tweets: types.map(Tweet),
  })
  .actions((self) => ({
    createTweet() {
      self.tweets.set("1", { body: "Hello" });
    },
  }));

// create an instance from a snapshot
const twitterStore = TwitterStore.create();

onPatch(twitterStore, (patch) => {
  console.log(patch);
});

twitterStore.createTweet();

You'll get output like:

Image

coolsoftwaretyler avatar Jul 25 '25 18:07 coolsoftwaretyler

@coolsoftwaretyler I checked the source code. Patches are generated using MobX's observe function:

https://github.com/mobxjs/mobx-state-tree/blob/05c6a3e70cff29854d574fab0ddf99cf4e7d6f9d/src/types/complex-types/map.ts#L309

https://github.com/mobxjs/mobx-state-tree/blob/05c6a3e70cff29854d574fab0ddf99cf4e7d6f9d/src/types/complex-types/map.ts#L387-L424

I found out how to get the snapshot with observe function:

  • not getSnapshot(ch.newValue)
  • but ch.newValue.snapshot

n9 avatar Jul 25 '25 21:07 n9

P.S. Why is change.oldValue.die() only called on "delete" and not on "update" (in MapType.didChange)?

n9 avatar Jul 25 '25 21:07 n9

P.S. Why is change.oldValue.die() only called on "delete" and not on "update" (in MapType.didChange)?

It's hard to say "why" for some of these implementation details. I'm far removed from the original writing of that

Could be an oversight, or perhaps it's not necessary for some reason. I'd be happy to review a PR that changes this, and we could test it out and check for any problems.

coolsoftwaretyler avatar Jul 25 '25 22:07 coolsoftwaretyler

The commit (for reference): https://github.com/mobxjs/mobx-state-tree/commit/fd6d83282245dc46802a64a5a81e8427935f4bb6

n9 avatar Jul 25 '25 22:07 n9