mobx icon indicating copy to clipboard operation
mobx copied to clipboard

observable.set triggers a refresh whenever any value in the set changes, even if that value was not accessed in the observer

Open TobiasWehrum opened this issue 3 years ago • 3 comments

Intended outcome:

I am using observable.set with many often-changing values. I also have many small observers observing the presence of single values in that set.

I would expect each observer to only refresh if the value it is observing is changing, e.g. if observable.has("mainValue") switches from false to true or vice versa.

Actual outcome:

The observer that uses observable.has("mainValue") is called any time any value in the set is added or deleted, e.g. if I call observable.add("otherValue") or observable.delete("otherValue").

How to reproduce the issue:

Minimal example showing the problem: https://codesandbox.io/s/mobx-observable-set-rerk61?file=/index.js

Minimal example showing that this isn't happening for observable.maps (which is my current workaround): https://codesandbox.io/s/mobx-observable-map-bhhgxi?file=/index.js

Versions

  • mobx 6.5.0

TobiasWehrum avatar Apr 19 '22 00:04 TobiasWehrum

Working as intended please see: https://github.com/mobxjs/mobx/issues/2336#issuecomment-616128089

urugator avatar Apr 19 '22 18:04 urugator

I see. I'm glad that at least maps have the behaviour I'm looking for. As I have thousands of values that frequently change during a certain period (with costly reactions that really should not run for every unrelated change), the current set implementation is completely unusable in my scenario.

I'd like to note that the behaviour of observable.set is very unintuitive to me and seems to run counter to the MobX philosophy described in README.md:

All changes to and uses of your data are tracked at runtime, building a dependency tree that captures all relations between state and output. This guarantees that computations depending on your state, like React components, run only when strictly needed. There is no need to manually optimize components with error-prone and sub-optimal techniques like memoization and selectors.

Usually I can be sure that reactions only run when necessary, independently of how often values change or how many observers there are. That isn't true for sets, making me unsure of where to use them at all.

Maybe it could at least be mentioned in https://mobx.js.org/api.html#observableset?

TobiasWehrum avatar Apr 19 '22 19:04 TobiasWehrum

It's always a compromise. Array also doesn't track per index, comparators are not deep by default etc. Personally I think it's something worth reconsidering for next majors.

Maybe it could at least be mentioned in https://mobx.js.org/api.html#observableset?

Sure, PR welcome :)

urugator avatar Apr 20 '22 07:04 urugator

PR welcome!

On Tue, 19 Apr 2022, 20:03 Tobias Wehrum, @.***> wrote:

I see. I'm glad that at least maps have the behaviour I'm looking for. As I have thousands of values that frequently change during a certain period (with costly reactions that really should not run for every unrelated change), the current set implementation is completely unusable in my scenario.

I'd like to note that the behaviour of observable.set is very unintuitive to me and seems to run counter to the MobX philosophy described in README.md:

All changes to and uses of your data are tracked at runtime, building a dependency tree that captures all relations between state and output. This guarantees that computations depending on your state, like React components, run only when strictly needed. There is no need to manually optimize components with error-prone and sub-optimal techniques like memoization and selectors.

Usually I can be sure that reactions only run when necessary, independently of how often values change or how many observers there are. That isn't true for sets, making me unsure of where to use them at all.

Maybe it could at least be mentioned in https://mobx.js.org/api.html#observableset?

— Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/3369#issuecomment-1102987997, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBC4HIBNP73KC3IIOF3VF37RZANCNFSM5TXF6JAA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mweststrate avatar Oct 11 '22 08:10 mweststrate

Doc updated.

urugator avatar Jan 30 '23 08:01 urugator