diff --git a/.changeset/tricky-spiders-collect.md b/.changeset/tricky-spiders-collect.md new file mode 100644 index 0000000000..0ea8b71e5e --- /dev/null +++ b/.changeset/tricky-spiders-collect.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure signal write invalidation within effects is consistent diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index e0facf3b55..4500a7c5a8 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, @@ -29,7 +28,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'; @@ -182,26 +182,20 @@ 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 ( 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 3c8879eb31..eca5ee94f9 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,22 @@ 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 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], + /** @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 +951,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 e198a5a89d..a9d29920cf 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -402,6 +402,42 @@ 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)); + }); + + 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;