From 8148734812006ae52a314561e26956e02a386f21 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 2 Jan 2025 16:54:43 +0000 Subject: [PATCH] fix: ensure unowned deriveds correctly get re-linked to the graph (#14855) * fix: ensure unowned deriveds correctly get re-linked to the graph * fix: ensure unowned deriveds correctly get re-linked to the graph * fix: ensure unowned deriveds correctly get re-linked to the graph * add test * add test * cleaner apporach * cleaner apporach * cleaner apporach * cleaner apporach --- .changeset/chatty-dolphins-complain.md | 5 +++ .../svelte/src/internal/client/runtime.js | 38 +++++++++---------- packages/svelte/tests/signals/test.ts | 33 ++++++++++++++++ 3 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 .changeset/chatty-dolphins-complain.md diff --git a/.changeset/chatty-dolphins-complain.md b/.changeset/chatty-dolphins-complain.md new file mode 100644 index 0000000000..b2c6f2b971 --- /dev/null +++ b/.changeset/chatty-dolphins-complain.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure unowned deriveds correctly get re-linked to the graph diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 4a90a21971..83d01caabe 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -196,34 +196,34 @@ export function check_dirtiness(reaction) { if (dependencies !== null) { var i; - - if ((flags & DISCONNECTED) !== 0) { - for (i = 0; i < dependencies.length; i++) { - (dependencies[i].reactions ??= []).push(reaction); + var dependency; + var is_disconnected = (flags & DISCONNECTED) !== 0; + var is_unowned_connected = is_unowned && active_effect !== null && !skip_reaction; + var length = dependencies.length; + + // If we are working with a disconnected or an unowned signal that is now connected (due to an active effect) + // then we need to re-connect the reaction to the dependency + if (is_disconnected || is_unowned_connected) { + for (i = 0; i < length; i++) { + dependency = dependencies[i]; + + if (!dependency?.reactions?.includes(reaction)) { + (dependency.reactions ??= []).push(reaction); + } } - reaction.f ^= DISCONNECTED; + if (is_disconnected) { + reaction.f ^= DISCONNECTED; + } } - for (i = 0; i < dependencies.length; i++) { - var dependency = dependencies[i]; + for (i = 0; i < length; i++) { + dependency = dependencies[i]; if (check_dirtiness(/** @type {Derived} */ (dependency))) { update_derived(/** @type {Derived} */ (dependency)); } - // If we are working with an unowned signal as part of an effect (due to !skip_reaction) - // and the version hasn't changed, we still need to check that this reaction - // is linked to the dependency source – otherwise future updates will not be caught. - if ( - is_unowned && - active_effect !== null && - !skip_reaction && - !dependency?.reactions?.includes(reaction) - ) { - (dependency.reactions ??= []).push(reaction); - } - if (dependency.version > reaction.version) { return true; } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 6796655cc8..0a22e30286 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -781,4 +781,37 @@ describe('signals', () => { assert.equal($.get(count), 0n); }; }); + + test('unowned deriveds correctly re-attach to their source', () => { + const log: any[] = []; + + return () => { + const a = state(0); + const b = state(0); + const c = derived(() => { + $.get(a); + return $.get(b); + }); + + $.get(c); + + set(a, 1); + + const destroy = effect_root(() => { + render_effect(() => { + log.push($.get(c)); + }); + }); + + assert.deepEqual(log, [0]); + + set(b, 1); + + flushSync(); + + assert.deepEqual(log, [0, 1]); + + destroy(); + }; + }); });