cortex
cortex copied to clipboard
Unable to reset a value within the same tick
This is the original (potential) issue I wanted to report:
When, within the same 'tick', you change a value and then change it again, but back to the original value, the updated cortex does not register that last change.
let cortex = new Cortex({a: 1}, cortex => {
// the new cortex here will contain {a: 2} after the script finishes
});
cortex.a.set(2);
cortex.a.set(1);
To clarify: this only happens when your last call to set (within the same tick) contains the original value (the value your cortex had at the start of the tick). Cortex compares them and sees no change, so decides no update is necessary if I understood right.
PR with failing test coming up.
Feel free to ignore if this is by design. It just felt awkward when I bumped into this issue.
I now fixed it by calling the second set inside a setTimeout.
This is expected behavior since we deliberately skip update if there's no eventual changes. What is the usecase for triggering react update when data did not change?
Even when there's a valid reason to update when no data ultimately change, I think the right approach is to introduce a force option instead
This is expected behavior since we deliberately skip update if there's no eventual changes.
But in the example the update isn't skipped. The onUpdate is called because of the first change.
What is the usecase for triggering react update when data did not change?
The reason I bumped into this was this scenario:
// cortex set-up
let cortex = new Cortex({a: [1, 2, 3]}, cortex => {
// do something with the new cortex
});
// after some user input
cortex.a.set([]); // empty the array
fetchDataFromServer(data => {
cortex.a.set(data); // this can return something different, but can also return [1, 2, 3]
}
In most cases the fetchDataFromServer callback takes place in the next tick. But sometimes it returns before the cortex update can run and then it is basically the example above when the server happens to return the same data ([1,2,3]).
In about 95% of cases this wasn't an issue, but when the server response returned before the cortex update, it created a weird situation where the array was empty although data was returned (be it the same as the original).
Are you testing locally? A network roundtrip + server delay would make it hard to respond within the same tick. In any case, I believe the issue is race condition and should be solved at the application level since cortex can't control the order of data update. I think you're already doing this with setTimeout.
Your bug report is valid though as cortex should not update. I'll investigate and let you know, probably something in the diffing logic
I'm getting this about the same frequency in all environments (locally, testing, production) in Chrome. And not only me, production users too. I vaguely remember Chrome has some kind of optimization system to decide what to prioritize in the current tick and what to postpone to the next. I might remember it wrong, but it doesn't sound like a good idea to reason about the order in which async methods (among which Cortex's onUpdate) are executed. Let's assume they are random.
But it sounds good if you can avoid the update in this scenario. That indeed fixes it. Thanks!
@mquan I'm happy to help out with a PR if you could push me in the right direction.
Hey sorry I've been busy and could only spend a few hours looking at this. I believe the issue is during update diffing if there's a change detected then the update and callback are triggered. So the simple fix is to add a special case check to diff initial and final changes and exit if no eventual changes were introduced
The approach I took in the possible fix in #112:
- Future updates to a cortex (set with
setTimeout) scheduled by previous changes need to be completed, so nothing is cancelled. Rather 'resets' are removed from the queued diffs. - If the
deep-diffmodule doesn't return anything, such a remove event is published. - A remove event contains the path for which queued diffs should be removed.
If this is the right road to take, is there anything I need to change/add?
So I've removed my solution from the PR, hoping the test itself can be merged. About the solution: I hope you can implement a proper one @mquan. I'd be happy to backport it to 0.x as I'm still stuck on that version.
Sorry about the long delay. I haven’t had much time to devote to this project. v2 code is almost a complete rewrite so not sure how much can be ported. One workaround is to force the update to the next tick by adding setTimeout for the calls that can collide. There’s a chance that they will all end up in the next tick so another option is to build your own queue (push the updates to a global array and only trigger the update once for the last entry in the array)