-
-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deepObserve callback getting called multiple times within a single runInAction function #294
Comments
Here is a jsfiddle link for anyone who'd like to try it out: |
This is working as intended, like observe, deepObserve reacts to all
mutations, not to the resulting values, so transactions dont apply to them.
Using observe should be very rarely needed and typically reaction/ autorun
is the better solution although it might require a little mental shift.
…On Sat, 13 Mar 2021, 10:17 Andrew, ***@***.***> wrote:
Here is a jsfiddle link for anyone who'd like to try it out:
https://jsfiddle.net/3jrpatdm/
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN4NBB6AIKP23TTZUZSJ6TTDM3T5ANCNFSM4ZDEFW3Q>
.
|
oh I see, I wasn't aware of this. My use case is that I simply want to call a function whenever my observed array changes (including changes within the elements of the array). |
Yes. See the docs on those and the page about understanding what mobx
reacts to.
…On Sat, 13 Mar 2021, 13:02 Andrew, ***@***.***> wrote:
oh I see, I wasn't aware of this. My use case is that I simply want to
call a function whenever my observed array changes (including changes
within the elements of the array).
Is there a way to do with autorun / reaction?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN4NBHAYLWSLGC7IXLKFGLTDNO65ANCNFSM4ZDEFW3Q>
.
|
Thank you, I read that and played with those now. My understanding is that both const myArray = observable([]);
reaction(() => myArray, () => {
// This will not run in this example
console.log("array update");
document.getElementById("log").innerHTML += "array update<br>";
});
myArray.length = 2;
myArray.push({key: 5}); For me to get notified I need to access I can see 2 solutions:
Does that sound reasonable or did I miss anything? |
Your issue is you don't want your reaction to be called when you set up the
autorun/reacion?
…On Sun, 14 Mar 2021 at 23:01, Andrew ***@***.***> wrote:
Thank you, I read that and played with those now. My understanding is that
both autorun and reaction track the values that are accessed in their
first parameter (effect / data function), so accessing the array itself
will not cause the callback to run:
const myArray = observable([]);
reaction(() => myArray, () => {
// This will not run in this example
console.log("array update");
document.getElementById("log").innerHTML += "array update<br>";});
myArray.length = 2;myArray.push({key: 5});
For me to get notified I need to access myArray.length in the first
parameter function, but that is not enough if I also want to be notified
about changes within the array elements (when myArray.length doesn't
change).
So I'd have to actually call my function in the first parameter, which
accesses all internal values of the array..which in my use case is a bit
problematic, because it's a very heavy function (it uses myArray to loop
through a big data structure), ideally it would only be triggered once
myArray changes, not any time before that, that's why deepObserve seemed
like a good choice.
I can see 2 solutions:
- create a less heavy data function that accesses all parts of myArray
without going though the heavy data processing and use that for autorun
/ reaction
- create a helper for deepObserve which "throttles" notifications per
requestAnimationFrame, so updates will only be triggered at most once per
frame.
Does that sound reasonable or did I miss anything?
Thank you!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACSTYSRGA77QSYXKRO7RGCLTDUW4RANCNFSM4ZDEFW3Q>
.
|
Yes, that would be very convenient, to just say "I want a callback to be invoked when any value within this observed object changes", and ideally only 1 invocation when the multiple things change in the observed object within an action. But those 2 workarounds I mentioned can work too, I think, if there is no other way. |
I think you might still missing the point of autorun, as what you describe
is what it is supposed to do; you run an effect and during that it merely
administers what is read as that is relevant for a next run. I recommend to
read the pages about the philosophy and gist of MobX, and/or setup a
sandbox that clarifies why it doesn't work for your case
…On Sun, 14 Mar 2021, 22:44 Andrew, ***@***.***> wrote:
Yes, that would be very convenient, to just say "I want a callback to be
invoked when any value within this observed object changes", and ideally
only 1 invocation when the multiple things change in the observed object
within an action.
But those 2 workarounds I mentioned can work too, I think, if there is no
other way.
Your issue is you don't want your reaction to be called when you set up
the autorun/reacion?
… <#m_-6930870244993986151_>
On Sun, 14 Mar 2021 at 23:01, Andrew *@*.***> wrote: Thank you, I read
that and played with those now. My understanding is that both autorun and
reaction track the values that are accessed in their first parameter
(effect / data function), so accessing the array itself will not cause the
callback to run: const myArray = observable([]); reaction(() => myArray, ()
=> { // This will not run in this example console.log("array update");
document.getElementById("log").innerHTML += "array update
";}); myArray.length = 2;myArray.push({key: 5}); For me to get notified I
need to access myArray.length in the first parameter function, but that is
not enough if I also want to be notified about changes within the array
elements (when myArray.length doesn't change). So I'd have to actually call
my function in the first parameter, which accesses all internal values of
the array..which in my use case is a bit problematic, because it's a very
heavy function (it uses myArray to loop through a big data structure),
ideally it would only be triggered once myArray changes, not any time
before that, that's why deepObserve seemed like a good choice. I can see 2
solutions: - create a less heavy data function that accesses all parts of
myArray without going though the heavy data processing and use that for
autorun / reaction - create a helper for deepObserve which "throttles"
notifications per requestAnimationFrame, so updates will only be triggered
at most once per frame. Does that sound reasonable or did I miss anything?
Thank you! — You are receiving this because you are subscribed to this
thread. Reply to this email directly, view it on GitHub <#294 (comment)
<#294 (comment)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACSTYSRGA77QSYXKRO7RGCLTDUW4RANCNFSM4ZDEFW3Q
.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN4NBD725WVY3U4VMSDYADTDU35RANCNFSM4ZDEFW3Q>
.
|
I read those pages and they seem clear to me. My concern is that my effect is a very heavy function that I only want to be invoked once the data actually changes, and not initially for initializing You can see that the heavy function is triggered 2x. The second one is in response to changing the
|
I'm not sure what heavy looks like, but calling heavy n times versus n+1
times is the same problem from a cost perspective. Either the function is
unbearably heavy and locking up the app, or it isn't. But the +1 isn't
going to make the fundamental difference and I rather look into optimizing
heavy itself then. For example you might memoize the filter result for
existing entries, and only apply filtering to your newly incoming data
instead of the whole set, etc. But so far it doesn't sound your 1 run less
is going to make the difference, and there is probably a bigger underlying
problem; heavy being heavy.
Note that autorun does support a custom scheduler, in case you want to
debounce it still, even after respecting transactions.
…On Mon, Mar 15, 2021 at 9:58 AM Andrew ***@***.***> wrote:
I read those pages and they seem clear to me. My concern is that my effect
is a very heavy function that I only want to be invoked once the data
actually changes, and not initially for initializing autorun or reaction.
Here is a hopefully better example:
https://jsfiddle.net/xfudacon/2/
You can see that the heavy function is triggered 2x. The second one is in
response to changing the myFilters array so that's fine, but it's also
running the first time, looping through all elements in myData.
So to avoid the heavy function to be called twice, I still think these are
the 2 options:
- use a reaction and create a lightweight data accessor function which
is ok to be called initially. In this example it would be trivial to do
this but in my actual code the heavy function is quite long and uses a
complex filter object, so it's not as straightforward although not
impossible either
- use a throttled deepObserve although that feels like a hack
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN4NBBBAGM7SVLPEVSALS3TDXK5PANCNFSM4ZDEFW3Q>
.
|
In this case, I think it matters. The list of items to go through can be in 10s of thousands and for each item the filter array needs to be looped through too. That stalls the UI for seconds and it makes a huge difference to do this only once or twice initially. Like 4 seconds or 8 seconds, both of them being too much of course, I agree. But debouncing seems like a good option to have too, I'll try and see what I can do with that. Thanks for the help. |
sounds like you'd really benefit from splitting up the computation into
many small async tasks :). But yeah that is definitely beyond the scope of
this project
…On Wed, Mar 17, 2021 at 10:16 PM Andrew ***@***.***> wrote:
In this case, I think it matters. The list of items to go through can be
in 10s of thousands and for each item the filter array needs to be looped
through too. That stalls the UI for seconds and it makes a huge difference
to do this only once or twice initially. Like 4 seconds or 8 seconds, both
of them being too much of course, I agree.
This heavy operation itself is memoized, it is only running again once the
large dataset or the filters change.
But debouncing seems like a good option to have too, I'll try and see what
I can do with that. Thanks for the help.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN4NBHM6VKOOQXHTWMRVJDTEES5HANCNFSM4ZDEFW3Q>
.
|
Ran across this issue today. The reasoning makes sense but it still threw me for a bit of a loop. For any future googlers, I added a simple debounce to my deepObserve callback and that worked for my usecase. |
I have a simple case that looks something like this:
The log in this case runs twice, once for each length modification, immediately after each.
Is this normal? I would expect that because those length updates are in a
runInAction
, the observer only gets updated once, after the action is fully executed.The text was updated successfully, but these errors were encountered: