fix: increment derived versions when updating (#12047)

We need to ensure that if derived a depends on derived b, and a is reconnecting to the dependency graph, that if b was updated more recently than a, a also updates.

Fixes #11988
Fixes #12044
pull/12051/head
Rich Harris 6 months ago committed by GitHub
parent 95d07de7f9
commit f5f38796ba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: increment derived versions when updating

@ -8,7 +8,8 @@ import {
mark_reactions, mark_reactions,
current_skip_reaction, current_skip_reaction,
execute_reaction_fn, execute_reaction_fn,
destroy_effect_children destroy_effect_children,
increment_version
} from '../runtime.js'; } from '../runtime.js';
import { equals, safe_equals } from './equality.js'; import { equals, safe_equals } from './equality.js';
@ -104,6 +105,7 @@ export function update_derived(derived, force_schedule) {
var is_equal = derived.equals(value); var is_equal = derived.equals(value);
if (!is_equal) { if (!is_equal) {
derived.version = increment_version();
derived.v = value; derived.v = value;
mark_reactions(derived, DIRTY, force_schedule); mark_reactions(derived, DIRTY, force_schedule);

@ -323,8 +323,6 @@ export function prop(props, key, flags, fallback) {
from_child = true; from_child = true;
set(inner_current_value, value); set(inner_current_value, value);
get(current_value); // force a synchronisation immediately get(current_value); // force a synchronisation immediately
// Increment the value so that unowned/disconnected nodes can validate dirtiness again.
current_value.version++;
} }
return value; return value;

@ -14,7 +14,8 @@ import {
set_current_untracked_writes, set_current_untracked_writes,
set_last_inspected_signal, set_last_inspected_signal,
set_signal_status, set_signal_status,
untrack untrack,
increment_version
} from '../runtime.js'; } from '../runtime.js';
import { equals, safe_equals } from './equality.js'; import { equals, safe_equals } from './equality.js';
import { CLEAN, DERIVED, DIRTY, BRANCH_EFFECT } from '../constants.js'; import { CLEAN, DERIVED, DIRTY, BRANCH_EFFECT } from '../constants.js';
@ -99,7 +100,7 @@ export function set(signal, value) {
signal.v = value; signal.v = value;
// Increment write version so that unowned signals can properly track dirtiness // Increment write version so that unowned signals can properly track dirtiness
signal.version++; signal.version = increment_version();
// If the current signal is running for the first time, it won't have any // If the current signal is running for the first time, it won't have any
// reactions as we only allocate and assign the reactions after the signal // reactions as we only allocate and assign the reactions after the signal

@ -122,6 +122,9 @@ export function set_last_inspected_signal(signal) {
/** If `true`, `get`ting the signal should not register it as a dependency */ /** If `true`, `get`ting the signal should not register it as a dependency */
export let current_untracking = false; export let current_untracking = false;
/** @type {number} */
let current_version = 0;
// If we are working with a get() chain that has no active container, // If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the reaction. // to prevent memory leaks, we skip adding the reaction.
export let current_skip_reaction = false; export let current_skip_reaction = false;
@ -155,6 +158,10 @@ export function set_dev_current_component_function(fn) {
dev_current_component_function = fn; dev_current_component_function = fn;
} }
export function increment_version() {
return current_version++;
}
/** @returns {boolean} */ /** @returns {boolean} */
export function is_runes() { export function is_runes() {
return current_component_context !== null && current_component_context.l === null; return current_component_context !== null && current_component_context.l === null;
@ -234,7 +241,6 @@ export function check_dirtiness(reaction) {
// is also dirty. // is also dirty.
if (version > /** @type {import('#client').Derived} */ (reaction).version) { if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
return !is_equal; return !is_equal;
} }
@ -257,9 +263,9 @@ export function check_dirtiness(reaction) {
// In thise case, we need to re-attach it to the graph and mark it dirty if any of its dependencies have // In thise case, we need to re-attach it to the graph and mark it dirty if any of its dependencies have
// changed since. // changed since.
if (version > /** @type {import('#client').Derived} */ (reaction).version) { if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
is_dirty = true; is_dirty = true;
} }
reactions = dependency.reactions; reactions = dependency.reactions;
if (reactions === null) { if (reactions === null) {
dependency.reactions = [reaction]; dependency.reactions = [reaction];

@ -232,7 +232,7 @@ describe('signals', () => {
// Ensure we're not leaking dependencies // Ensure we're not leaking dependencies
assert.deepEqual( assert.deepEqual(
nested.slice(0, -2).map((s) => s.deps), nested.slice(0, -2).map((s) => s.deps),
[null, null] [null, null, null, null]
); );
}; };
}); });
@ -446,4 +446,95 @@ describe('signals', () => {
assert.equal(state?.reactions, null); assert.equal(state?.reactions, null);
}; };
}); });
test('deriveds update upon reconnection #1', () => {
let a = source(false);
let b = source(false);
let c = derived(() => $.get(a));
let d = derived(() => $.get(c));
let last: Record<string, boolean | null> = {};
render_effect(() => {
last = {
a: $.get(a),
b: $.get(b),
c: $.get(c),
d: $.get(a) || $.get(b) ? $.get(d) : null
};
});
return () => {
assert.deepEqual(last, { a: false, b: false, c: false, d: null });
flushSync(() => set(a, true));
flushSync(() => set(b, true));
assert.deepEqual(last, { a: true, b: true, c: true, d: true });
flushSync(() => set(a, false));
flushSync(() => set(b, false));
assert.deepEqual(last, { a: false, b: false, c: false, d: null });
flushSync(() => set(a, true));
flushSync(() => set(b, true));
assert.deepEqual(last, { a: true, b: true, c: true, d: true });
flushSync(() => set(a, false));
flushSync(() => set(b, false));
assert.deepEqual(last, { a: false, b: false, c: false, d: null });
flushSync(() => set(b, true));
assert.deepEqual(last, { a: false, b: true, c: false, d: false });
};
});
test('deriveds update upon reconnection #2', () => {
let a = source(false);
let b = source(false);
let c = source(false);
let d = derived(() => $.get(a) || $.get(b));
let branch = '';
render_effect(() => {
if ($.get(c) && !$.get(d)) {
branch = 'if';
} else {
branch = 'else';
}
});
return () => {
assert.deepEqual(branch, 'else');
flushSync(() => set(c, true));
assert.deepEqual(branch, 'if');
flushSync(() => set(a, true));
assert.deepEqual(branch, 'else');
set(a, false);
set(b, false);
set(c, false);
flushSync();
assert.deepEqual(branch, 'else');
flushSync(() => set(c, true));
assert.deepEqual(branch, 'if');
flushSync(() => set(b, true));
assert.deepEqual(branch, 'else');
set(a, false);
set(b, false);
set(c, false);
flushSync();
assert.deepEqual(branch, 'else');
flushSync(() => set(c, true));
assert.deepEqual(branch, 'if');
};
});
}); });

Loading…
Cancel
Save