From 946fe108acd814d6b0e93627131fd6963326168d Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 13 Jan 2025 01:04:10 +0000 Subject: [PATCH 1/4] fix: ensure signal write invalidation within effects is persistent --- .changeset/tricky-spiders-collect.md | 5 ++ .../src/internal/client/reactivity/sources.js | 17 +++---- .../svelte/src/internal/client/runtime.js | 51 +++++++++++++++---- packages/svelte/tests/signals/test.ts | 37 ++++++++++++++ 4 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 .changeset/tricky-spiders-collect.md diff --git a/.changeset/tricky-spiders-collect.md b/.changeset/tricky-spiders-collect.md new file mode 100644 index 000000000000..50d8c628cead --- /dev/null +++ b/.changeset/tricky-spiders-collect.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure signal write invalidation within effects is persistent diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index e0facf3b55d3..ebc4eedc015a 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -29,7 +29,8 @@ import { INSPECT_EFFECT, UNOWNED, MAYBE_DIRTY, - BLOCK_EFFECT + BLOCK_EFFECT, + ROOT_EFFECT } from '../constants.js'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -191,17 +192,13 @@ export function internal_set(source, value) { is_runes() && active_effect !== null && (active_effect.f & CLEAN) !== 0 && - (active_effect.f & BRANCH_EFFECT) === 0 + (active_effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ) { - if (new_deps !== null && new_deps.includes(source)) { - set_signal_status(active_effect, DIRTY); - schedule_effect(active_effect); + + if (untracked_writes === null) { + set_untracked_writes([source]); } else { - if (untracked_writes === null) { - set_untracked_writes([source]); - } else { - untracked_writes.push(source); - } + untracked_writes.push(source); } } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 3c8879eb317b..e8d9e5d8e0f0 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -382,6 +382,34 @@ export function handle_error(error, effect, previous_effect, component_context) } } +/** + * @param {Value} signal + * @param {Effect} effect + * @param {number} [depth] + */ +function schedule_possible_effect_self_invalidation(signal, effect, depth = 0) { + var reactions = signal.reactions; + if (reactions === null) return; + + for (var i = 0; i < reactions.length; i++) { + var reaction = reactions[i]; + if ((reaction.f & DERIVED) !== 0) { + schedule_possible_effect_self_invalidation( + /** @type {Derived} */ (reaction), + effect, + depth + 1 + ); + } else if (effect === reaction) { + if (depth === 0) { + set_signal_status(reaction, DIRTY); + } else if ((reaction.f & CLEAN) !== 0) { + set_signal_status(reaction, MAYBE_DIRTY); + } + schedule_effect(/** @type {Effect} */ (reaction)); + } + } +} + /** * @template V * @param {Reaction} reaction @@ -434,6 +462,18 @@ export function update_reaction(reaction) { deps.length = skipped_deps; } + // If we're inside an effect and we have untracked writes, then we need to + // ensure that if any of those untracked writes result in re-invalidation + // of the current effect, then we need to re-schedule the current effect + if (untracked_writes !== null && (reaction.f & (DERIVED | MAYBE_DIRTY | DIRTY)) === 0) { + for (i = 0; i < /** @type {Source[]} */ (untracked_writes).length; i++) { + schedule_possible_effect_self_invalidation( + untracked_writes[i], + /** @type {Effect} */ (reaction) + ); + } + } + // If we are returning to an previous reaction then // we need to increment the read version to ensure that // any dependencies in this reaction aren't marked with @@ -907,17 +947,6 @@ export function get(signal) { } else { new_deps.push(signal); } - - if ( - untracked_writes !== null && - active_effect !== null && - (active_effect.f & CLEAN) !== 0 && - (active_effect.f & BRANCH_EFFECT) === 0 && - untracked_writes.includes(signal) - ) { - set_signal_status(active_effect, DIRTY); - schedule_effect(active_effect); - } } } else if (is_derived && /** @type {Derived} */ (signal).deps === null) { var derived = /** @type {Derived} */ (signal); diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index e198a5a89de7..41be722ff885 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -402,6 +402,43 @@ describe('signals', () => { }; }); + test('schedules rerun when writing to signal before reading it from derived', (runes) => { + if (!runes) return () => {}; + let log: any[] = []; + + const value = state(1); + const double = derived(() => $.get(value) * 2); + + user_effect(() => { + set(value, 10); + log.push($.get(double)); + set(value, 10); + }); + + return () => { + flushSync(); + assert.deepEqual(log, [20]); + }; + }); + + test('schedules rerun when writing to signal after reading it from derived', (runes) => { + if (!runes) return () => {}; + let log: any[] = []; + + const value = state(1); + const double = derived(() => $.get(value) * 2); + + user_effect(() => { + log.push($.get(double)); + set(value, 10); + }); + + return () => { + flushSync(); + assert.deepEqual(log, [2, 20]); + }; + }); + test('effect teardown is removed on re-run', () => { const count = state(0); let first = true; From d7b508033082753baf7c00aba36319649067d65a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 13 Jan 2025 01:05:32 +0000 Subject: [PATCH 2/4] fix: ensure signal write invalidation within effects is persistent --- .changeset/tricky-spiders-collect.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tricky-spiders-collect.md b/.changeset/tricky-spiders-collect.md index 50d8c628cead..0ea8b71e5efc 100644 --- a/.changeset/tricky-spiders-collect.md +++ b/.changeset/tricky-spiders-collect.md @@ -2,4 +2,4 @@ 'svelte': patch --- -fix: ensure signal write invalidation within effects is persistent +fix: ensure signal write invalidation within effects is consistent From 4f1defcef74f7b87645e4260cf06c17f6a96ff55 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 13 Jan 2025 01:13:44 +0000 Subject: [PATCH 3/4] fix: ensure signal write invalidation within effects is persistent --- packages/svelte/src/internal/client/reactivity/sources.js | 2 -- packages/svelte/src/internal/client/runtime.js | 8 ++++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index ebc4eedc015a..7fff7f3f7ba9 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -3,7 +3,6 @@ import { DEV } from 'esm-env'; import { component_context, active_reaction, - new_deps, active_effect, untracked_writes, get, @@ -194,7 +193,6 @@ export function internal_set(source, value) { (active_effect.f & CLEAN) !== 0 && (active_effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ) { - if (untracked_writes === null) { set_untracked_writes([source]); } else { diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index e8d9e5d8e0f0..eca5ee94f907 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -464,8 +464,12 @@ export function update_reaction(reaction) { // If we're inside an effect and we have untracked writes, then we need to // ensure that if any of those untracked writes result in re-invalidation - // of the current effect, then we need to re-schedule the current effect - if (untracked_writes !== null && (reaction.f & (DERIVED | MAYBE_DIRTY | DIRTY)) === 0) { + // of the current effect, then that happens accordingly + if ( + is_runes() && + untracked_writes !== null && + (reaction.f & (DERIVED | MAYBE_DIRTY | DIRTY)) === 0 + ) { for (i = 0; i < /** @type {Source[]} */ (untracked_writes).length; i++) { schedule_possible_effect_self_invalidation( untracked_writes[i], From a4229699a9ab9eaabb4424bcc9285d0c8d9ee648 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 14 Jan 2025 12:08:20 +0000 Subject: [PATCH 4/4] address feedback --- packages/svelte/src/internal/client/reactivity/sources.js | 5 ++--- packages/svelte/tests/signals/test.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7fff7f3f7ba9..4500a7c5a84a 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -182,9 +182,8 @@ export function internal_set(source, value) { mark_reactions(source, DIRTY); - // 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 - // has fully executed. So in the case of ensuring it registers the reaction + // It's possible that the current reaction might not have up-to-date dependencies + // whilst it's actively running. So in the case of ensuring it registers the reaction // properly for itself, we need to ensure the current effect actually gets // scheduled. i.e: `$effect(() => x++)` if ( diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 41be722ff885..a9d29920cf84 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -412,7 +412,6 @@ describe('signals', () => { user_effect(() => { set(value, 10); log.push($.get(double)); - set(value, 10); }); return () => {