From 36a73fa7948eee53914d918c3012c853ba79b859 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 16 Sep 2024 19:18:18 +0100 Subject: [PATCH] fix: avoid flushing sync with $inspect (#13239) * fix: avoid flushing sync with $inspect * add test --- .changeset/hot-glasses-roll.md | 5 +++ .../src/internal/client/reactivity/sources.js | 35 +++++++++---------- .../samples/effect-order-3/_config.js | 26 ++++++++++++++ .../samples/effect-order-3/main.svelte | 18 ++++++++++ 4 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 .changeset/hot-glasses-roll.md create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-order-3/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-order-3/main.svelte diff --git a/.changeset/hot-glasses-roll.md b/.changeset/hot-glasses-roll.md new file mode 100644 index 0000000000..938a5404b8 --- /dev/null +++ b/.changeset/hot-glasses-roll.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid flushing sync with $inspect diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 92bc4f264d..01ec1d3073 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -16,8 +16,9 @@ import { update_effect, derived_sources, set_derived_sources, - flush_sync, - check_dirtiness + check_dirtiness, + set_is_flushing_effect, + is_flushing_effect } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import { @@ -170,23 +171,21 @@ export function set(source, value) { if (DEV && inspect_effects.size > 0) { const inspects = Array.from(inspect_effects); - if (current_effect === null) { - // Triggering an effect sync can tear the signal graph, so to avoid this we need - // to ensure the graph has been flushed before triggering any inspect effects. - // Only needed when there's currently no effect, and flushing with one present - // could have other unintended consequences, like effects running out of order. - // This is expensive, but given this is a DEV mode only feature, it should be fine - flush_sync(); - } - for (const effect of inspects) { - // Mark clean inspect-effects as maybe dirty and then check their dirtiness - // instead of just updating the effects - this way we avoid overfiring. - if ((effect.f & CLEAN) !== 0) { - set_signal_status(effect, MAYBE_DIRTY); - } - if (check_dirtiness(effect)) { - update_effect(effect); + var previously_flushing_effect = is_flushing_effect; + set_is_flushing_effect(true); + try { + for (const effect of inspects) { + // Mark clean inspect-effects as maybe dirty and then check their dirtiness + // instead of just updating the effects - this way we avoid overfiring. + if ((effect.f & CLEAN) !== 0) { + set_signal_status(effect, MAYBE_DIRTY); + } + if (check_dirtiness(effect)) { + update_effect(effect); + } } + } finally { + set_is_flushing_effect(previously_flushing_effect); } inspect_effects.clear(); } diff --git a/packages/svelte/tests/runtime-runes/samples/effect-order-3/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-order-3/_config.js new file mode 100644 index 0000000000..bfc7025031 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-order-3/_config.js @@ -0,0 +1,26 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, logs }) { + const [b1] = target.querySelectorAll('button'); + flushSync(() => { + b1.click(); + }); + flushSync(() => { + b1.click(); + }); + assert.deepEqual(logs, [ + 'effect', + 0, + 'in-increment', + 1, + 'effect', + 1, + 'in-increment', + 2, + 'effect', + 2 + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-order-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-order-3/main.svelte new file mode 100644 index 0000000000..0c74acfc29 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-order-3/main.svelte @@ -0,0 +1,18 @@ + + +