msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: ensure derived signals properly capture consumers (#10213)

Browse Source 
* fix: ensure derived signals properly capture consumers

* fix: ensure derived signals properly capture consumers
pull/10216/head
Dominic Gannaway 12 months ago committed by GitHub
parent 1798e58300
commit 2b2bd06d6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 106 additions and 42 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/thick-cycles-rule.md
Unescape View File

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: ensure derived signals properly capture consumers

86
packages/svelte/src/internal/client/runtime.js
Unescape View File

@ -48,6 +48,7 @@ let current_scheduler_mode = FLUSH_MICROTASK;
let is_micro_task_queued = false;
let is_task_queued = false;
let is_raf_queued = false;
let is_flushing_effect = false;
// Used for $inspect
export let is_batching_effect = false;
@ -295,7 +296,6 @@ function is_signal_dirty(signal) {
let i;
for (i = 0; i < length; i++) {
const dependency = dependencies[i];
if ((dependency.f & MAYBE_DIRTY) !== 0 && !is_signal_dirty(dependency)) {
set_signal_status(dependency, CLEAN);
continue;
@ -343,7 +343,7 @@ function execute_signal_fn(signal) {
current_consumer = signal;
current_block = signal.b;
current_component_context = signal.x;
current_skip_consumer = current_effect === null && (signal.f & UNOWNED) !== 0;
current_skip_consumer = !is_flushing_effect && (signal.f & UNOWNED) !== 0;
current_untracking = false;
// Render effects are invoked when the UI is about to be updated - run beforeUpdate at that point
@ -366,27 +366,28 @@ function execute_signal_fn(signal) {
res = /** @type {() => V} */ (init)();
}
let dependencies = /** @type {import('./types.js').Signal<unknown>[]} **/ (signal.d);
if (current_dependencies !== null) {
let i;
if (dependencies !== null) {
const deps_length = dependencies.length;
// Include any dependencies up until the current_dependencies_index.
const full_dependencies =
const full_current_dependencies =
current_dependencies_index === 0
? dependencies
? current_dependencies
: dependencies.slice(0, current_dependencies_index).concat(current_dependencies);
const dep_length = full_dependencies.length;
const current_dep_length = full_current_dependencies.length;
// If we have more than 16 elements in the array then use a Set for faster performance
// TODO: evaluate if we should always just use a Set or not here?
const current_dependencies_set = dep_length > 16 ? new Set(full_dependencies) : null;
for (i = current_dependencies_index; i < dep_length; i++) {
const dependency = full_dependencies[i];
const full_current_dependencies_set =
current_dep_length > 16 ? new Set(full_current_dependencies) : null;
for (i = current_dependencies_index; i < deps_length; i++) {
const dependency = dependencies[i];
if (
(current_dependencies_set !== null && !current_dependencies_set.has(dependency)) ||
!full_dependencies.includes(dependency)
(full_current_dependencies_set !== null &&
!full_current_dependencies_set.has(dependency)) ||
!full_current_dependencies.includes(dependency)
) {
remove_consumer(signal, dependency, false);
remove_consumer(signal, dependency);
}
}
}
@ -415,7 +416,7 @@ function execute_signal_fn(signal) {
}
}
} else if (dependencies !== null && current_dependencies_index < dependencies.length) {
remove_consumers(signal, current_dependencies_index, false);
remove_consumers(signal, current_dependencies_index);
dependencies.length = current_dependencies_index;
}
return res;
@ -435,10 +436,9 @@ function execute_signal_fn(signal) {
* @template V
* @param {import('./types.js').ComputationSignal<V>} signal
* @param {import('./types.js').Signal<V>} dependency
* @param {boolean} remove_unowned
* @returns {void}
*/
function remove_consumer(signal, dependency, remove_unowned) {
function remove_consumer(signal, dependency) {
const consumers = dependency.c;
let consumers_length = 0;
if (consumers !== null) {
@ -454,14 +454,10 @@ function remove_consumer(signal, dependency, remove_unowned) {
}
}
}
if (remove_unowned && consumers_length === 0 && (dependency.f & UNOWNED) !== 0) {
if (consumers_length === 0 && (dependency.f & UNOWNED) !== 0) {
// If the signal is unowned then we need to make sure to change it to dirty.
set_signal_status(dependency, DIRTY);
remove_consumers(
/** @type {import('./types.js').ComputationSignal<V>} **/ (dependency),
0,
true
);
remove_consumers(/** @type {import('./types.js').ComputationSignal<V>} **/ (dependency), 0);
}
}
@ -469,10 +465,9 @@ function remove_consumer(signal, dependency, remove_unowned) {
* @template V
* @param {import('./types.js').ComputationSignal<V>} signal
* @param {number} start_index
* @param {boolean} remove_unowned
* @returns {void}
*/
function remove_consumers(signal, start_index, remove_unowned) {
function remove_consumers(signal, start_index) {
const dependencies = signal.d;
if (dependencies !== null) {
const active_dependencies = start_index === 0 ? null : dependencies.slice(0, start_index);
@ -481,7 +476,7 @@ function remove_consumers(signal, start_index, remove_unowned) {
const dependency = dependencies[i];
// Avoid removing a consumer if we know that it is active (start_index will not be 0)
if (active_dependencies === null || !active_dependencies.includes(dependency)) {
remove_consumer(signal, dependency, remove_unowned);
remove_consumer(signal, dependency);
}
}
}
@ -502,7 +497,7 @@ function destroy_references(signal) {
if ((reference.f & IS_EFFECT) !== 0) {
destroy_signal(reference);
} else {
remove_consumers(reference, 0, true);
remove_consumers(reference, 0);
reference.d = null;
}
}
@ -585,19 +580,26 @@ function flush_queued_effects(effects) {
const length = effects.length;
if (length > 0) {
infinite_loop_guard();
let i;
for (i = 0; i < length; i++) {
const signal = effects[i];
const flags = signal.f;
if ((flags & (DESTROYED | INERT)) === 0) {
if (is_signal_dirty(signal)) {
set_signal_status(signal, CLEAN);
execute_effect(signal);
} else if ((flags & MAYBE_DIRTY) !== 0) {
set_signal_status(signal, CLEAN);
const previously_flushing_effect = is_flushing_effect;
is_flushing_effect = true;
try {
let i;
for (i = 0; i < length; i++) {
const signal = effects[i];
const flags = signal.f;
if ((flags & (DESTROYED | INERT)) === 0) {
if (is_signal_dirty(signal)) {
set_signal_status(signal, CLEAN);
execute_effect(signal);
} else if ((flags & MAYBE_DIRTY) !== 0) {
set_signal_status(signal, CLEAN);
}
}
}
} finally {
is_flushing_effect = previously_flushing_effect;
}
effects.length = 0;
}
}
@ -822,10 +824,7 @@ function update_derived(signal, force_schedule) {
updating_derived = true;
const value = execute_signal_fn(signal);
updating_derived = previous_updating_derived;
const status =
current_skip_consumer || (current_effect === null && (signal.f & UNOWNED) !== 0)
? DIRTY
: CLEAN;
const status = current_skip_consumer || (signal.f & UNOWNED) !== 0 ? DIRTY : CLEAN;
set_signal_status(signal, status);
const equals = /** @type {import('./types.js').EqualsFunctions} */ (signal.e);
if (!equals(value, signal.v)) {
@ -1083,7 +1082,10 @@ function mark_subtree_children_inert(signal, inert, visited_blocks) {
if (references !== null) {
let i;
for (i = 0; i < references.length; i++) {
mark_subtree_inert(references[i], inert, visited_blocks);
const reference = references[i];
if ((reference.f & IS_EFFECT) !== 0) {
mark_subtree_inert(references[i], inert, visited_blocks);
}
}
}
}
@ -1262,7 +1264,7 @@ export function destroy_signal(signal) {
const destroy = signal.y;
const flags = signal.f;
destroy_references(signal);
remove_consumers(signal, 0, true);
remove_consumers(signal, 0);
signal.i =
signal.r =
signal.y =

57
packages/svelte/tests/signals/test.ts
Unescape View File

@ -171,4 +171,61 @@ describe('signals', () => {
}
};
});
test('effects correctly handle unowned derived values that do not change', () => {
const log: number[] = [];
let count = $.source(0);
const read = () => {
const x = $.derived(() => ({ count: $.get(count) }));
return $.get(x);
};
const derivedCount = $.derived(() => read().count);
$.user_effect(() => {
log.push($.get(derivedCount));
});
return () => {
$.flushSync(() => $.set(count, 1));
// Ensure we're not leaking consumers
assert.deepEqual(count.c?.length, 1);
$.flushSync(() => $.set(count, 2));
// Ensure we're not leaking consumers
assert.deepEqual(count.c?.length, 1);
$.flushSync(() => $.set(count, 3));
// Ensure we're not leaking consumers
assert.deepEqual(count.c?.length, 1);
assert.deepEqual(log, [0, 1, 2, 3]);
};
});
let count = $.source(0);
let calc = $.derived(() => {
if ($.get(count) >= 2) {
return 'limit';
}
return $.get(count) * 2;
});
test('effect with derived using new derived every time', () => {
const log: Array<number | string> = [];
const effect = $.user_effect(() => {
log.push($.get(calc));
});
return () => {
$.flushSync(() => $.set(count, 1));
$.flushSync(() => $.set(count, 2));
$.flushSync(() => $.set(count, 3));
$.flushSync(() => $.set(count, 4));
$.flushSync(() => $.set(count, 0));
// Ensure we're not leaking consumers
assert.deepEqual(count.c?.length, 1);
assert.deepEqual(log, [0, 2, 'limit', 0]);
$.destroy_signal(effect);
// Ensure we're not leaking consumers
assert.deepEqual(count.c, null);
};
});
});

Write Preview
Loading…
Cancel
Save