From 7dbb5b478893056e4a9c43bdee5e34e64d86e588 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 12 Jan 2026 12:14:08 -0800 Subject: [PATCH] fix: derived reactivity and perf regressions (#17362) * add test sample * add test for sveltejs/kit#15059 * fix: reconnect deriveds inside branch effects * add changeset * fix: derived with no deps always set as MAYBE_DIRTY fixes #17342 * add test for #17342 * additional changeset * refactor: extract setting derived status to helper, apply to sources.js * add test case for #17352 * fix: reconnect child deriveds when evaluating connected parent derived fixes #17352 * fix import order causing Cannot read properties of undefined on dev load * remove duplicate iteration over deps * minor style tweaks * oops, fix merge * use update_derived_status, so that we never set a dep-less derived MAYBE_DIRTY * tweak * reaction.deps cannot be null for a MAYBE_DIRTY derived * make it such that reactions without deps are never MAYBE_DIRTY * since we no longer need to check reaction.deps === null, we can revert this bit * more explicit check * tidy up * more * gah whoops * move import * simplify test * make dep-less derived behaviour more explicit, move it above is_destroying_effect handling * remove test - this is adequately covered by #17445 * replace tricky unit test with component-based test * remove incorrect test * remove the BRANCH_EFFECT stuff * tidy up * DRY * tweak, add explanatory comment * tweak * explanatory comment * remove changeset * update changeset --------- Co-authored-by: Tee Ming Co-authored-by: Rich Harris --- .changeset/beige-sloths-cry.md | 5 ++ .../svelte/src/internal/client/runtime.js | 26 +++++++--- .../derived-read-outside-reaction/_config.js | 47 +++++++++++++++++++ .../derived-read-outside-reaction/main.svelte | 30 ++++++++++++ 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 .changeset/beige-sloths-cry.md create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/main.svelte diff --git a/.changeset/beige-sloths-cry.md b/.changeset/beige-sloths-cry.md new file mode 100644 index 0000000000..013fd2f6cc --- /dev/null +++ b/.changeset/beige-sloths-cry.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: reconnect clean deriveds when they are read in a reactive context diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 82c5f5a06f..0e12324d72 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -620,15 +620,27 @@ export function get(signal) { return value; } - // TODO this should probably just be `!batch_values?.has(derived)` — the second bit - // should be taken care of by clearing `batch_values` in `mark_reactions`? - if (!batch_values?.has(derived) || (current_batch?.is_fork && !effect_tracking())) { - if (is_dirty(derived)) { - update_derived(derived); + // connect disconnected deriveds if we are reading them inside an effect, + // or inside another derived that is already connected + var should_connect = + (derived.f & CONNECTED) === 0 && + !untracking && + active_reaction !== null && + (is_updating_effect || (active_reaction.f & CONNECTED) !== 0); + + var is_new = derived.deps === null; + + if (is_dirty(derived)) { + if (should_connect) { + // set the flag before `update_derived`, so that the derived + // is added as a reaction to its dependencies + derived.f |= CONNECTED; } + + update_derived(derived); } - if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) { + if (should_connect && !is_new) { reconnect(derived); } } @@ -652,7 +664,7 @@ export function get(signal) { function reconnect(derived) { if (derived.deps === null) return; - derived.f ^= CONNECTED; + derived.f |= CONNECTED; for (const dep of derived.deps) { (dep.reactions ??= []).push(derived); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/_config.js new file mode 100644 index 0000000000..f3a71b93e6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/_config.js @@ -0,0 +1,47 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ` + + +

1, 2, 3

+ `, + + test({ assert, target }) { + const [button1, button2] = target.querySelectorAll('button'); + + button1.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + +

2, 4, 6

+ ` + ); + + button2.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + +

2, 4, 6, 8

+ ` + ); + + button1.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + +

3, 6, 9, 12

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/main.svelte new file mode 100644 index 0000000000..0b5096dfa4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-read-outside-reaction/main.svelte @@ -0,0 +1,30 @@ + + + + + + +

{products.join(', ')}