-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
"No-op" batch triggers effects #242
Comments
As I understand by working with the internals the last weeks,
Basically, signals use a 'version' number rather than the actual value to know when a 'computed' or 'effect' needs to re-run. This is done because comparing numbers is cheaper. The "downside" is that if you set the same value as it was before the batch, there is no way to know if the value is the same, again because it compares version numbers which increase of every set and not the actual value. There is this comment in the code regarding the version:
|
I think you're confusing nodes with signals. Nodes only have a version. Signals and computed signals have a version and keep a reference to the current / cached value. Signals and computed signals could also keep a reference to the value (and version) which was valid before the batch. Then, in the commit phase of the batch, the signal could compare its current value with its previous value and reset its version to the previous version if they match. If they don't match, it can discard the previous value. |
As far as I am aware, there is no way to implement collapsing of batched value assignments/computations without retaining previous values, degrading performance and causing GC. It's also unnecessary for a large number of the intended use-cases for Signals. I like to use the Preact adapter as a gauge for whether functionality is specialized to Signals usage, since the adapters are the largest and primary user of Signals. In the adapter, the effect that binds a signal to a prop has implicit access to its previous value by virtue of the fact that has to assign values to the |
That's not always possible, e.g., when a computed signal returns a new reference every time it is executed. const foo = signal(42);
const bar = computed(() => [foo.value]);
let previous = undefined;
effect(() => {
// always true
if (previous !== bar.value) {
console.log(bar.value);
}
previous = bar.value;
});
You would only have to keep a single previous value for each signal while the batch hasn't committed yet. class Computed<T> {
version: number
committedVersion: number
value: T
committedValue: T
notify() {
this.version = this.committedVersion
this.value = this.committedValue
}
refresh() {
const this.value= /* ... */
if (this.value === this.committedValue) {
this.version = this.committedVersion
}
}
commit() {
this.committedVersion = this.version
this.committedValue = this.value
}
} |
I've tried to implement it in my local fork some time ago but failed 😅 batch(() => {
foo.value = 0; // notifies all targets (effects are scheduled to run)
foo.value = 42; // notifies all targets (what should it do here?)
}); Batch batches effects, so you'd need to un-schedule those effects. This is tricky because an effect can be scheduled to run by other signals too so you can't just remove them from the linked list of batched effects. If you keep the prev. value of foo (e.g: I used This could kinda work but it also doesn't take into account that computed and effect can have side-effects: const foo = signal(42);
effect(() => {
foo.value;
foo.value = 42;
});
effect(() => console.log("Print or no print", foo.value));
batch(() => {
foo.value = 123;
}); |
Yes, that's what I meant. I am sorry that I was being unclear. I think that this linked list is going to very short in most cases. You only have to schedule |
Is it possible to queue up a list of the signals whose values have changed rather than queueing up a list of callbacks to run? This would require tracking the signals with updated values and the original values of those signals. Then at the end of the batch you would iterate over the collected signals and compare their current value against the previous value and only trigger their associated callbacks if the value changed during the batch. The argument above about GC and memory pressure are legitimate, so that would still need to be weighed against the value added by this feature. |
Given a signal
foo
and an effect logging its value.When I run a batch, which changes the value of
foo
in such a way that it is essentially a no-op,then the value of
foo
is logged even though the value offoo
is the same as before the batch.This is also the case when the effect depends on a computed signal, but only if the computed value is read in between.
The text was updated successfully, but these errors were encountered: