Mobx Array Handling is Unintuitive

[grithin]

I like mobx. I built a complex form handler with it in 2021. Coming back to it, I've noticed unexpected behavior that I imagine throws many people off and is probably one of the reasons mobx is not more popular. I understand this is a byproduct of the behavior of proxies, but if you look at something like vue, they manage the push well (https://codepen.io/cfjedimaster/pen/GObpZM) . I imagine very few people who initially put a shallow observe on an array intended it to only observe a full swap (reference change).

Shallow Array Unexpected Behavior

import { makeObservable, observable, action, autorun, toJS } from "mobx"
/*
The mobx observable array handling (ObservableArrayAdministration), is horrible.
*/
let observed;
// 1. The whole array is not observed unless swapped
observed = observable({ b: ['moo'] })

autorun(() => { // runs once for init
  console.log("Whole array observed", observed.b)
})
// `push` does not trigger
action(() => observed.b.push("bob"))()
// `delete` does not trigger
action(() => delete observed.b[0])()
// element assignment does not trigger
action(() => observed.b[1] = 'foo')()

// full array assignment does trigger
action(() => (observed.b = "foo"))()


// 2. all elements and `length respond to changes of any other elements or `length`
observed = observable({ b: ["moo", "bill"] })
autorun(() => {
  console.log("autorun 2", observed.b.length)
})

// `push` triggers (expected)
action(() => observed.b.push("bob"))()

// element assignment triggers (unexected)
action(() => observed.b[0] = 'smoo')()

// To impress the oddity of triggering when some other property changes:
autorun(() => {
  console.log("autorun 3", observed.b[1])
})

// assignment to different-than-observed-element triggers
action(() => observed.b[0] = '123')()

This last bit surprised me when I found a component observing x.length re running when x.length did not change.

Deep Array Unexpected Behavior

I wondered whether deep array observable would solve some of these problems. But, it results in more unexpected behavior.

let observableDeep = (x)=> observable(x, {}, {deep:true})
let observed

// 1. push does trigger, but other element changes don't
observed = observableDeep({ b: ['moo'] })

autorun(() => { // runs once for init
  console.log("Whole array observed", observed.b)
})
// `push` does trigger
action(() => observed.b.push("bob"))()
// element assignment does not trigger
action(() => observed.b[0] = 'foo')()

// `delete` does not trigger
action(() => delete observed.b[0])()
// new element assignment does not trigger
action(() => observed.b[1] = 'foo')()

// full array assignment does trigger
action(() => (observed.b = "foo"))()

// 2. all elements and `length` respond to changes of any other elements or `length`
observed = observableDeep({ b: ["moo", "bill"] })
autorun(() => {
  console.log("autorun 2", observed.b.length)
})
// `push` triggers (expected)
action(() => observed.b.push("bob"))()

// element assignment triggers (unexected)
action(() => observed.b[0] = 'smoo')()

observed = observableDeep({ b: ["moo", "bill"] })
autorun(() => {
  console.log("autorun 3", observed.b[0])
})

// separate element assignment triggers (unexpected)
action(() => observed.b[1] = 'smoo')()

Most annoying, observing x.length triggers on an element change regardless of whether x.length changes.

Popularity and Changes

Reactivity with proxies is somewhat complex and that limits audience. Vue deals with this by hiding some things. For instance, in the https://codepen.io/cfjedimaster/pen/GObpZM example, the methods.checkForm is a sort of mobx action, where the errors observable changes only cause a single re-render. The vue developer doesn't need to know that.

However, when you add on to the complexity unexpected behavior, like x.push handling, observing x.length, etc, you get even more of a fall off of audience.

Years ago when I started with mobx, I had a lot of frustration before I found toJS, which should be very apparent in the docs.

Any way, I've not considered how to solve these things until there is an acknowledgement that they are problems and should be solved.

[urugator]

You're just confused by console.log behavior. If everything is deeply observable (which is default, no need for explicit { deep: true }), whenever there is a change that would cause the "outcome" of an autorun to be different, the autorun will trigger.
The "outcome" of console.log("Whole array observed", observed.b) is something like Whole array observed Proxy(Object) {} regardless of the b's content. So there is no reason for autorun to re-run, when b's content changes.
Imagine it's not a console, but a DOM or React's virtual DOM or fetch. You don't want to cause updates if the DOM would look the same or trigger request for the same set of params.
The console in dev tools allows you to inspect the passed reference, but this is not visible for autorun, you would have to do this inspection in autorun itself. Also notice the thing you see in the console is not a snapshot taken at a time of running console.log, but a real time reference:

const o = {};
console.log(o);
o.a = 5;
// inspecting `o` in dev tools displays `{ o: 5 }`

So in some sense the autorun would have to see in the future, which would be quite cool, but we are not there yet.

The shallow array works exactly as you would expect.

Most annoying, observing x.length triggers on an element change regardless of whether x.length changes.

Could be the case, not sure atm. I think the reasoning could be that observing length alone is so rare, that extra length observable per array isn't worth it, but it's just my guess or something I vaguely remember.

[grithin]

I'm not confused about how console.log behaves. This was run on the command line, not browser dev tools. However, I'm getting the same results now between the use of { deep: true }and without. Very odd, and sorry for the confusion. I'll stick less into examples.

Here was my particular problem that initiated this

let observed;
observed = observable(['moo', 'bob'])

autorun(() => {
  console.log("Ran", observed.length)
})

// triggers
action(() => observed.push('bill'))()
// triggers
action(() => observed[1] = 'moo')()

output

Ran 2
Ran 3
Ran 3

Part of the object of mobx is to reduce re-rendering. Here we see that the autorun that is observing .length runs when an element changes value, even when the length doesn't change.

This is problematic because using .length is the solution given, if I recall, to the common error of doing this:

let observed;
observed = observable(['moo', 'bob'])

autorun(() => {
  console.log("Ran", observed)
})

// does not trigger
action(() => observed.push('bill'))()

This can be resolved by using computed, but I've never seen this in the wild:

let observed;
observed = observable(['moo', 'bob'])

let computed_length = computed(()=> {
  console.log('computed')
  return observed.length})

  autorun(() => {
  console.log("Ran", computed_length.get())
})

// does trigger
action(() => observed.push('bill'))()
// does not trigger
action(() => observed[1] = 'moo')()

output

computed
Ran 2
computed
Ran 3
computed

As I also pointed out in my example, this cross-property observation has further problems than just .length :

let observed;
observed = observable(['moo', 'bob'])

autorun(() => {
  console.log("Ran", observed[0])
})

// triggers
action(() => observed.push('bill'))()
// triggers
action(() => observed[1] = 'moo')()

Output

Ran moo
Ran moo
Ran moo

Here, in attempting to observe just observed[0], the autorun runs on observed[1] changes

[mweststrate]

This was a very deliberate performance tradeoff, most observers do something for an entire collection, and the current approach makes it much cheaper to track loops / maps / filters, or handle splice operations, as it triggers only one atom change instead of length amount of atoms, saving memory and improving invalidation speed. However, you can get the behavior you want where length is tracked separately, as well as every individual item, you can use observable maps that behaves exactly like that, with the downside that specific operations like splice or unshift are much more expensive, if you'd mimic those

…

On Sat, Dec 28, 2024, 21:13 Adam Frederick ***@***.***> wrote: I'm not confused about how console.log behaves. This was run on the command line, not browser dev tools. However, I'm getting the same results now between the use of { deep: true }and without. Very odd, and sorry for the confusion. I'll stick less into examples. Here was my particular problem that initiated this let observed;observed = observable(['moo', 'bob']) autorun(() => { console.log("Ran", observed.length)}) // triggersaction(() => observed.push('bill'))()// triggersaction(() => observed[1] = 'moo')() output Ran 2 Ran 3 Ran 3 Part of the object of mobx is to reduce re-rendering. Here we see that the autorun that is observing .length runs when an element changes value, even when the length doesn't change. This is problematic because using .length is the solution given, if I recall, to the common error of doing this: let observed;observed = observable(['moo', 'bob']) autorun(() => { console.log("Ran", observed)}) // does not triggeraction(() => observed.push('bill'))() This can be resolved by using computed, but I've never seen this in the wild: let observed;observed = observable(['moo', 'bob']) let computed_length = computed(()=> { console.log('computed') return observed.length}) autorun(() => { console.log("Ran", computed_length.get())}) // does triggeraction(() => observed.push('bill'))()// does not triggeraction(() => observed[1] = 'moo')() output computed Ran 2 computed Ran 3 computed As I also pointed out in my example, this cross-property observation has further problems than just .length : let observed;observed = observable(['moo', 'bob']) autorun(() => { console.log("Ran", observed[0])}) // triggersaction(() => observed.push('bill'))()// triggersaction(() => observed[1] = 'moo')() Output Ran moo Ran moo Ran moo Here, in attempting to observe just observed[0], the autorun runs on observed[1] changes — Reply to this email directly, view it on GitHub <#3988 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBCLXU4TV4IIMATRRIT2H4A7ZAVCNFSM6AAAAABUBXTNLKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCNRYGY3TSNI> . You are receiving this because you commented.Message ID: ***@***.***>

[grithin]

I understand the performance issue with adding atoms for things like observed[0]. But, .length would require a single new atom, and doesn't require an additional Proxy. I hacked together a test in the dist to try out having an atom for length:

I've added a "changed" log to the Atom.reportChanged class

let observed;
observed = observable(['moo', 'bob'])

autorun(() => {
  console.log("Ran", observed.length)
})

action(() =>  observed[0] = 'mes')()
action(() =>  observed.push('bill'))()

Before my hack, the output is

Output

changed
Ran 2
changed
Ran 2
changed
Ran 3

After hack, Output

changed
changed
Ran 2
changed
changed
changed
Ran 3

This amounts to an additional reportChanged call per splice operation that results in a changed length. Is there a performance concern with this, or some other concern?

[mweststrate]

That would be less of a concern. At that point the question becomes, are there enough realistic real-life scenarios (that aren't contrived) where it holds that

a) observers are interested in the length of a collection, but not in the contents at all, and
b) the contents often changes without actually changing the length with it?

Note that this scenario primarily only applies to arrays of primitives. With arrays of objects, you typically update the nested objects, not replace them, so length observers don't trigger anyway. For example when you update an item in a shopping cart to change the amount. So only a scenario like swapping the order of items in the cart would benefit here.

At this moment it becomes a tradeoff whether the above scenario is common enough to justify doubling the reactivity costs of arrays in general vs. saving downstream side effect costs in these very specific scenarios.

[grithin]

Examples

Interesting. I don't recall the problem that prompted me, but I've made some example code. I see this issue can be a problem both with arrays of objects and arrays of primitives

Array of Objects

let data = new Data([{name:'bob'}, {name:'sue'}])
console.log(data)

const NameDisplayer = observer((props) => {
	console.log('running NameDisplayer with '+props.person.name)
	return (<span style={{paddingRight:'20px'}}>{props.person.name}</span>)
})
const NamesDisplayer = observer((props) => {
	console.log('running NamesDisplayer')
	return (props.people.map((person, i)=> <NameDisplayer key={i} person={person} />))
})
const LengthComputationDisplay = observer((props) => {
	console.log('running LengthComputationDisplay')
	return <div>Length Computation: {props.people.length * 10}</div>
})
const PeopleManager = observer((props) => {
	console.log('re-run PeopleManager')
	return ( <div>
			<LengthComputationDisplay people={data.people} />
			<NamesDisplayer people={data.people} />
		</div> )
})

Doing data.people[0] = {'name': 'bill'}; gets

running LengthComputationDisplay
running NamesDisplayer
running NameDisplayer with bill

Here we see, when the object is replaced, the length computation re-runs. A lot of times, this length computation is not isolated into another component, but instead at the top level (PeopleManager), which would trigger the top level to re-run (PeopleManager).

Array of Primitive
You can get the same problems by changing the code to allow let data = new Data(['bob', 'sue']) and data.people[0] = 'bill'

Problem Abstracted

1. array of objects performs inconsistently with expectation
   data.people[0] = {'name': 'bill'}; will trigger a people.length watch rerun, but data.people[0].name = 'bill'; won't.
2. array of primitives performs unexpectedly.
   data.people[0] = 'bill' causes re-run on a data.people.length watch

The Cost

The cost is usually not doubling the reactivity costs. The doubling scenario only exists if the only action on the array is length changers within actions.

If you look at something like this

let observed;
observed = observable(['moo', 'bob'])

autorun(() => {
  console.log("Ran", observed.length)
})


action(() => {
  for(const k in  observed){
    observed[k] += '.'
  }
})()
/*
without length Atom
changed
Ran 2
changed
changed
Ran 2

With length Atom:
changed
changed
Ran 2
changed
changed
*/

There is no reactivity cost to the length Atom apart from the initialization.

So, the cost benefit analysis is more like:

1. cost increase on initialization of new observed arrays
2. cost increase on array length change operations
   vs
3. expectability of watching .length
4. avoided runs from unexpected .length behavior

Lets say most observed arrays are arrays of objects, and lets say the average array is 20 items long, then each array initialization costs at least 21 Atoms without a length Atom, and 22 with a length Atom.

I don't think adding a length Atom is a performance issue.

[mweststrate]

data.people[0] = {'name': 'bill'};

That is quite unidiomatic MobX, more a redux-ism and generally now how one would/should update objects.

Let's take a step back, as all things in algorithms like MobX are trade-offs. Are you experiencing real life performance issues with this, and if so, would your suggested change fix it? Note that you can always throw in a computed that tracks length, and then you'll have a second atom as well.

[grithin]

As you mentioned, the problem exists with ordering too. And, thinking about it, perhaps I encountered the problem with an array of inventory, when user swapped inventory spots (items didn't change, but the array got re-ordered).
But, regardless, it was an unexpected run of something watching the .length.

Now, let's suppose there were some logic on this that tracked whether an item was added or removed.

if(old_length - x.length < 0){
// do length increase code
}else{
// do length decrease code
}

Here, because it is expected that .length watch only runs when the length is changed, there is incorrect logic because of that false assumption. So it's not just a matter of something being unperformant because of triggering needless re-renders, it's that the expectation can cause wrong behavior.
I understand this can be avoided with a computed (that's in my earlier example), but before I found this problem out by encountering it, I had written various code expecting a x.length watch to only run when length changed, and I was given false certainty because that's what it appears to do (in most cases)

Like I said at the outset, my concern is with adoption of mobx. And that comes down to complexity and expectability.

Here's the mobx dist code diff:
https://gist.github.com/grithin/2c761923a9fffbeb1b6e918bb959fe69

[mweststrate]

The example code looks like something that shouldn't pass any code review for not handling the equal case. That aside, the proper way to set up such a case where old and new value are compared is to use a reaction, and that wouldn't change either on a length change.

The more fundamental problem however, is that you can always find a case where the thing you are object here holds. E.g. the length case was 'fixed', and you'd throw in a map or filter operation, you'd be exactly back at square one. E.g. something simple like observableArray.filter(() => true) would expose the exact same behavior on a swap operation, even after introducing a length atom.

So I think the mental model here is off. MobX is like a caching layer that tries to avoid downstream computations and effects. It is not a guarantee that in every case and circumstance it will be the theoretical minimum however, and systems should be robust against that (hence the importance of pure computations, and well orchestrated effects. This is very in a sense very similar to what React does with useMemo / useEffect, where it remains at React digression to invalidate those at moments it chooses).

Doesn't mean that it is a bad idea btw to introduce that second atom, but just want to raise it might not solve the problem you think it is solving.

[grithin]

The short example code was just there to convey the concept - if there is the reasonable expectation that watch(x.length) only reacts to x.length changing, then code that is based on that can be incorrect. Of course there are ways to get around this once you know watch(x.length) doesn't behave the way one would assume when one doesn't look at the mobx source code.

would expose the exact same behavior on a swap operation, even after introducing a length can you give me an example of this in full, because what you present does not cause a problem with the length Atom.

that tries to avoid downstream computations and effects, yes, and that's why I saw the problem in the first place, because a component was re-rendering when it shouldn't, because there was a watch on x.length.

not a guarantee ... theoretical minimum however .... React does with useMemo / useEffect, where it remains at React digression to invalidate those at moments it chooses To my knowledge, React's discretion of invalidation/reaction is both predictable and expectable.

Any way, I'm repeating my point now, and if it's not agreed, I will close this

[urugator]

it is expected that .length watch only runs when the length is changed

Why is it expected? The initial run of the autorun does not map to any change. Why are you trying to infer what happenned from the fact that autorun is executed? It's unreasonable and fragile. Add another dependency and it's broken. Introduce action for resetting old_length also broken.

because a component was re-rendering when it shouldn't

Your component logic surely doesn't depend on the exact number of re-renders or on what exactly caused the re-render. Same goes for useEffect logic etc. If it does, you're not using it correctly. The default is, that these things are executed whenever any part of your state changes - in other words - you won't miss an update. Everything else is an optimization, not a semantic guarantee. Same with autorun.

[grithin]

Why is it expected?

For the same reason some code like bind('event_name', fn) should trigger fn only when event_name occurs.

The default is, that these things are executed whenever any part of your state changes - in other words - you won't miss an update.

No. A primary point of mobx is to increase the specificity of reaction and decrease unnecessary reactions. What's wrong with a number of React apps written by amateurs is excessive re-rendering causing lag.

I don't think you and I can have a useful discussion.

[mweststrate]

can you give me an example of this in full, because what you present does not cause a problem with the length Atom.

@computed
get allAdults() {
  return this.persons.filter(p => p.age >= 18)
}

Now if you'd render somewhere <div>store.allAdults.length</div>, it'd re-render unnecessarily if you do an operation like store.persons[2].age++ for someone who is already an adult! Because the computed needs to invalidate, producing a new array every time, even though you could reason that both the old and new adults array will have the same length. And your additional atom wouldn't change that case here. However, if you'd introduce @computed get adultsCount() { return this.allAdults.length }, and used that, it wouldn't re-render (or autorun), as you introduced an additional caching point for MobX.

We consider both approaches semantically the same, although the difference can be observed in side effects / perf. So MobX isn't thát magical that it finds the most optimal amount of caching / propagation points, and that is why the semantic correctness of your app shouldn't depend on what fires when. This is very similarly to how React components should not yield a different UI regardless if it decides to render once or twice in DEV mode (or suspense) for example. Even though this can be observed if you want to, if the correct working of your app depends on it, that considered a bug.

I think the mental model you have in mind for MobX is not what it presents to be. In terms of observable systems, you have basically two flavours: systems that propagate events (e.g. RxJS / event emitters), and systems that propagate state (e.g. MobX). The former is lossless, and allows you to reason precisely on how often things fire, as well as deriving current state from past states (your example).

The latter category is intentionally lossy, leaving the system the freedom to both overfire or underfire (e.g. if an item is added, and shortly thereafter removed again, it can decide to skip(!) both changes and it shouldn't matter, as the system is designed to only reason from the current state. This has significant perf benefits but is something that clearly wouldn't fly with your prime example above, as you're expressing your logic in terms of events that happened to the data, rather than what the current state of the data is at this moment in time. So I think there is a misalignment here with the system properties you are seeking here, and the properties that MobX provides. To quote an old wiki page

But if time plays an important role, like when throttling, accumulating events or having complex join patterns like zip, these are the cases where you want to work with streams.

MobX suffices and is easier and more efficient in 90% of the cases; where you want to derive something consistently from the latest state. But for the other 10% of use cases, reactive stream libraries really shine. This is usually when I/O plays an important role.

[grithin]

This has significant perf benefits but is something that clearly wouldn't fly with your prime example above, as you're expressing your logic in terms of events that happened to the data

There are three different scenarios here.

1. watch(x.length) to react to all x.length changes
2. watch(x.length) to react to net x.length changes, after action resolution
3. "time plays an important part" (ie, doing a lot of changes to big arrays)

Since #1 is excluded by the fundamentals of mobx (actions), I didn't explicitly say my watch(x.length) refers to #2. #3 is also not a matter of consideration in my particular objection to functionality.

I've also not looked at whether to or how to correct a computed's watch(computed.length) since, if changing watch(x.length) behavior on a non-computed can't be agreed on, it wouldn't matter.

[grithin]

I will assume there is no agreement. I am closing this.