fix: ensure unowned derived dependencies are not duplicated when reac… (#15232)

* fix: ensure unowned derived dependencies are not duplicated when reactions are skipped

* added test case

* added test case

* add comment
pull/15234/head
Dominic Gannaway 7 months ago committed by GitHub
parent b0c4fa5246
commit f2c83e5db7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: ensure unowned derived dependencies are not duplicated when reactions are skipped

@ -188,7 +188,8 @@ export function check_dirtiness(reaction) {
dependency = dependencies[i]; dependency = dependencies[i];
// We always re-add all reactions (even duplicates) if the derived was // We always re-add all reactions (even duplicates) if the derived was
// previously disconnected // previously disconnected, however we don't if it was unowned as we
// de-duplicate dependencies in that case
if (is_disconnected || !dependency?.reactions?.includes(reaction)) { if (is_disconnected || !dependency?.reactions?.includes(reaction)) {
(dependency.reactions ??= []).push(reaction); (dependency.reactions ??= []).push(reaction);
} }
@ -404,15 +405,9 @@ export function update_reaction(reaction) {
skipped_deps = 0; skipped_deps = 0;
untracked_writes = null; untracked_writes = null;
active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null; active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null;
// prettier-ignore
skip_reaction = skip_reaction =
(flags & UNOWNED) !== 0 && (flags & UNOWNED) !== 0 &&
(!is_flushing_effect || (!is_flushing_effect || previous_reaction === null || previous_untracking);
// If we were previously not in a reactive context and we're reading an unowned derived
// that was created inside another reaction, then we don't fully know the real owner and thus
// we need to skip adding any reactions for this unowned
((previous_reaction === null || previous_untracking) &&
/** @type {Derived} */ (reaction).parent !== null));
derived_sources = null; derived_sources = null;
set_component_context(reaction.ctx); set_component_context(reaction.ctx);
@ -933,7 +928,10 @@ export function get(signal) {
skipped_deps++; skipped_deps++;
} else if (new_deps === null) { } else if (new_deps === null) {
new_deps = [signal]; new_deps = [signal];
} else { } else if (!skip_reaction || !new_deps.includes(signal)) {
// Normally we can push duplicated dependencies to `new_deps`, but if we're inside
// an unowned derived because skip_reaction is true, then we need to ensure that
// we don't have duplicates
new_deps.push(signal); new_deps.push(signal);
} }
} }

@ -818,6 +818,28 @@ describe('signals', () => {
}; };
}); });
test('unowned deriveds dependencies are correctly de-duped', () => {
return () => {
let a = state(0);
let b = state(true);
let c = derived(() => $.get(a));
let d = derived(() => ($.get(b) ? 1 : $.get(a) + $.get(c) + $.get(a)));
$.get(d);
assert.equal(d.deps?.length, 1);
$.get(d);
set(a, 1);
set(b, false);
$.get(d);
assert.equal(d.deps?.length, 3);
};
});
test('unowned deriveds correctly update', () => { test('unowned deriveds correctly update', () => {
return () => { return () => {
const arr1 = proxy<{ a: number }[]>([]); const arr1 = proxy<{ a: number }[]>([]);

Loading…
Cancel
Save