-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
fix: only call onchange
once for array mutations
#15073
Conversation
|
|
(I didn't add any tests yet, was just noodling around. Also: maybe we would want to expose the batching function at some point, though I didn't do that preemptively) |
I can add tests in my pr, I was waiting to find a solution for the tests...looks good to me! |
Will this have a non-trivial impact on performance? We're now doing array.includes everytime we |
@dummdidumm wdyt about adding performance tests? I'm not that deep in ecosystem yet, so maybe it got addressed somehow already? If not I can have a look on poc |
We have a series of benchmarks already that runs on CI. But i refer to @trueadm for that 😄 |
Our benchmarks doesn't cover deep reactivity. |
small addition to #15069 — only call
onchange
once for array mutationsThis isn't just about avoiding unnecessary work — it's about correctness. Without something like this, the callback will fire when the array mutation is incomplete, which could lead to all sorts of bad things.
I thought we were already doing something like this in the context of
$inspect
, but it turns out we weren't. Perhaps we should?