mobx-log with Redux Devtools does not properly set state in the RD log entry for observable nested objects

[vaughnkoch]

Hi, thank you for building this useful library. :)

I've been playing with Redux Devtools and mobx-log, see below. Each action has an entry in RD, which is very useful. However, the state associated with each action seems to depend on what's passed in:

• For integers (and likely other primitives), calling the action (e.g. incrementFoo) results in the correct state (e.g. foo = 1).
• However, for objects containing other objects, calling the action (e.g. setTodos) results in a null state for that variable.
• If the object is changed to a primitives-only object, e.g. { someId: 'someValue' }, the state populates the variable.

Here's my store:

import { makeAutoObservable } from 'mobx'

import { makeLoggable } from 'mobx-log'

class MyStore {
  // All 3 properties are observable from makeAutoObservable
  todos: any = null
  foo = 0
  bar = 0

  constructor(rootStore) {
    makeAutoObservable(this)

    // mobx-log
    makeLoggable(this)
  }

  setTodos = (todos) => {
    this.todos = todos
  }

  incrementFoo = () => {
    this.foo = this.foo + 1
  }

  incrementBar = () => {
    this.bar = this.bar + 1
  }

  // Normally this would be an API call with */yield instead of async/await.
  // This is called externally by a React useEffect.
  fetchData = () => {
    // First action called - results in "MyStore@MyStore@1.fetchData" and the state shown below this code
    this.setTodos({
      // 'abc': { id: 'abc', firstName: 'First', lastName: 'Last' }, // Does not work
      'def': { id: 'def' },  // Does not work
      // 'def': 'xyz',  // Works
      // Blank object: Works
    })

    this.incrementFoo()

    this.incrementBar()
  }

}

export default MyStore

I'm using the following:

"mobx": "6.10.2",
"mobx-log": "2.2.1",
"mobx-react-lite": "4.0.5",
Redux Devtools: 3.1.3

Actual:

// In the "MyStore@MyStore@1.fetchData" entry:
{
  todos: null,
  foo: 0,
  bar: 0
}

Expected:

{
  todos: { 'def': { id: 'def' } },
  foo: 0,
  bar: 0
}

If I've missed some configuration or other gotcha, please let me know.

[kubk]

Hi, I’ve copied a part of your code with a nested object, and the issue isn’t reproducible for me.

Code:

May I ask for a simple GitHub reproduction as well as screenshots from the devtools? To avoid setting up everything from scratch, you can fork the repo, go to the example folder and modify the demo project. Use npm i && npm run start to set it up: https://github.com/kubk/mobx-log/tree/master/example

I use this subproject to test the library

[vaughnkoch]

Thanks for investigating. I forked the repo, modified stopwatch-store.ts, and was able to repro, although the behavior seems slightly different.

Commit in my fork: vaughnkoch@e13d240

I labeled the scenarios A through D below. I also attached a zipfile with the screenshots.

mobx-log screenshots - Issue 43.zip

const data = {
  // A: Does not work.
  // todos are populated at StopwatchStore@nextStep,
  // expected at setTodos or fetchData.
  'abc': { id: 'abc', firstName: 'First', lastName: 'Last' },

  // B: Does not work.
  // todos are populated at StopwatchStore@start,
  // expected at setTodos or fetchData.
  // 'def': { id: 'def' },

  // C: Works.
  // todos are populated at StopwatchStore@fetchData,
  // although not at setTodos.
  // 'def': 'xyz',

  // D: Works.
  // todos are populated at StopwatchStore@setTodos.
  // Blank object
}

[kubk]

@vaughnkoch Thanks. Try installing an experimental fix npm i mobx-log@beta. If NPM can't locate that version - clear the cache first via npm cache clean --force

[vaughnkoch]

Hi @kubk, I tried mobx-log@beta (mobx-log@2.2.2-beta.1), and it appears to fix the problem, thanks! When do you think this new fix will be available in non-beta? Also, I'd be curious to know what the fix was.

[kubk]

@vaughnkoch 2.2.3 is ready. Thanks for reporting the issue.

I had to use setTimeout to delay the creation of the store snapshot. The previous implementation only utilized spyReportStart/spyReportEnd, and it seems like it wasn’t enough: https://mobx.js.org/analyzing-reactivity.html#:~:text=report%2Dend-,spyReportEnd,-%3Dtrue%2C%20time%3F%20(total