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

fix: increment derived versions when updating #12047

Merged
merged 4 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brave-pigs-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: increment derived versions when updating
4 changes: 3 additions & 1 deletion packages/svelte/src/internal/client/reactivity/deriveds.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
mark_reactions,
current_skip_reaction,
execute_reaction_fn,
destroy_effect_children
destroy_effect_children,
increment_version
} from '../runtime.js';
import { equals, safe_equals } from './equality.js';

Expand Down Expand Up @@ -104,6 +105,7 @@ export function update_derived(derived, force_schedule) {
var is_equal = derived.equals(value);

if (!is_equal) {
derived.version = increment_version();
derived.v = value;
mark_reactions(derived, DIRTY, force_schedule);

Expand Down
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ export function prop(props, key, flags, fallback) {
from_child = true;
set(inner_current_value, value);
get(current_value); // force a synchronisation immediately
// Increment the value so that unowned/disconnected nodes can validate dirtiness again.
current_value.version++;
}

return value;
Expand Down
5 changes: 3 additions & 2 deletions packages/svelte/src/internal/client/reactivity/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
set_current_untracked_writes,
set_last_inspected_signal,
set_signal_status,
untrack
untrack,
increment_version
} from '../runtime.js';
import { equals, safe_equals } from './equality.js';
import { CLEAN, DERIVED, DIRTY, BRANCH_EFFECT } from '../constants.js';
Expand Down Expand Up @@ -99,7 +100,7 @@ export function set(signal, value) {
signal.v = value;

// 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
// reactions as we only allocate and assign the reactions after the signal
Expand Down
10 changes: 8 additions & 2 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ export function set_last_inspected_signal(signal) {
/** If `true`, `get`ting the signal should not register it as a dependency */
export let current_untracking = false;

/** @type {number} */
let current_version = 0;

// If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the reaction.
export let current_skip_reaction = false;
Expand Down Expand Up @@ -155,6 +158,10 @@ export function set_dev_current_component_function(fn) {
dev_current_component_function = fn;
}

export function increment_version() {
return current_version++;
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
}

/** @returns {boolean} */
export function is_runes() {
return current_component_context !== null && current_component_context.l === null;
Expand Down Expand Up @@ -234,7 +241,6 @@ export function check_dirtiness(reaction) {
// is also dirty.

if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
return !is_equal;
}

Expand All @@ -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
// changed since.
if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
is_dirty = true;
}

reactions = dependency.reactions;
if (reactions === null) {
dependency.reactions = [reaction];
Expand Down
93 changes: 92 additions & 1 deletion packages/svelte/tests/signals/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ describe('signals', () => {
// Ensure we're not leaking dependencies
assert.deepEqual(
nested.slice(0, -2).map((s) => s.deps),
[null, null]
[null, null, null, null]
);
};
});
Expand Down Expand Up @@ -446,4 +446,95 @@ describe('signals', () => {
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