Skip to content
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

Open
andrewvarga opened this issue Mar 12, 2021 · 13 comments

Comments

@andrewvarga
Copy link

andrewvarga commented Mar 12, 2021

I have a simple case that looks something like this:

const myArray = observable([]);
deepObserve(myArray, () => {
   console.log("array update");
});

runInAction(() => {
   myArray.length = 1;
   myArray.length = 2;
});

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.

@andrewvarga
Copy link
Author

Here is a jsfiddle link for anyone who'd like to try it out:
https://jsfiddle.net/3jrpatdm/

@mweststrate
Copy link
Member

mweststrate commented Mar 13, 2021 via email

@andrewvarga
Copy link
Author

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?

@mweststrate
Copy link
Member

mweststrate commented Mar 13, 2021 via email

@andrewvarga
Copy link
Author

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!

@NaridaL
Copy link
Collaborator

NaridaL commented Mar 14, 2021 via email

@andrewvarga
Copy link
Author

andrewvarga commented Mar 14, 2021

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.

@mweststrate
Copy link
Member

mweststrate commented Mar 14, 2021 via email

@andrewvarga
Copy link
Author

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

@mweststrate
Copy link
Member

mweststrate commented Mar 15, 2021 via email

@andrewvarga
Copy link
Author

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.

@mweststrate
Copy link
Member

mweststrate commented Mar 17, 2021 via email

@dested
Copy link

dested commented Jan 4, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants