From 0fdf121fa07466918b80f83a199b7fb44dbd854e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 17 Nov 2023 12:02:46 +0000 Subject: [PATCH 1/2] fix: address unowned propagation signal issue --- .changeset/fifty-steaks-float.md | 5 +++ .../svelte/src/internal/client/runtime.js | 6 ++- .../class-state-derived-unowned/_config.js | 45 +++++++++++++++++++ .../class-state-derived-unowned/main.svelte | 38 ++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 .changeset/fifty-steaks-float.md create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte diff --git a/.changeset/fifty-steaks-float.md b/.changeset/fifty-steaks-float.md new file mode 100644 index 000000000000..b100f215bed8 --- /dev/null +++ b/.changeset/fifty-steaks-float.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: address unowned propagation signal issue diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a67f300b219a..2698612ccc2c 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -895,15 +895,17 @@ function mark_signal_consumers(signal, to_status, force_schedule) { for (i = 0; i < length; i++) { const consumer = consumers[i]; const flags = consumer.flags; + const unowned = (flags & UNOWNED) !== 0; + const dirty = (flags & DIRTY) !== 0; if ( - (flags & DIRTY) !== 0 || + (dirty && !unowned) || (!runes && consumer === current_effect) || (!force_schedule && consumer === current_effect) ) { continue; } set_signal_status(consumer, to_status); - if ((flags & CLEAN) !== 0) { + if ((flags & CLEAN) !== 0 || (dirty && unowned)) { if ((consumer.flags & IS_EFFECT) !== 0) { schedule_effect(/** @type {import('./types.js').EffectSignal} */ (consumer), false); } else { diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js new file mode 100644 index 000000000000..d73f3351a30c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js @@ -0,0 +1,45 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + // The component context class instance gets shared between tests, strangely, causing hydration to fail? + skip_if_hydrate: 'permanent', + + async test({ assert, target, component }) { + const btn = target.querySelector('button'); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1, 2]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1, 2, 3]); + + flushSync(() => { + btn?.click(); + }); + + assert.deepEqual(component.log, [ + 0, + 'class trigger false', + 'local trigger false', + 1, + 2, + 3, + 4, + 'class trigger true', + 'local trigger true' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte new file mode 100644 index 000000000000..f9160cd3ea17 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte @@ -0,0 +1,38 @@ + + + + + From c915e68de43377a6329dea4ba240f0386a161af2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 17 Nov 2023 13:00:24 +0000 Subject: [PATCH 2/2] Add comments --- packages/svelte/src/internal/client/runtime.js | 12 +++++++----- .../samples/class-state-derived-unowned/main.svelte | 12 +++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2698612ccc2c..67d1593f41e4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -897,14 +897,16 @@ function mark_signal_consumers(signal, to_status, force_schedule) { const flags = consumer.flags; const unowned = (flags & UNOWNED) !== 0; const dirty = (flags & DIRTY) !== 0; - if ( - (dirty && !unowned) || - (!runes && consumer === current_effect) || - (!force_schedule && consumer === current_effect) - ) { + // We skip any effects that are already dirty (but not unowned). Additionally, we also + // skip if the consumer is the same as the current effect (except if we're not in runes or we + // are in force schedule mode). + if ((dirty && !unowned) || ((!force_schedule || !runes) && consumer === current_effect)) { continue; } set_signal_status(consumer, to_status); + // If the signal is not clean, then skip over it – with the exception of unowned signals that + // are already dirty. Unowned signals might be dirty because they are not captured as part of an + // effect. if ((flags & CLEAN) !== 0 || (dirty && unowned)) { if ((consumer.flags & IS_EFFECT) !== 0) { schedule_effect(/** @type {import('./types.js').EffectSignal} */ (consumer), false); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte index f9160cd3ea17..58b96457dbb6 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte @@ -1,11 +1,11 @@ @@ -23,11 +23,9 @@ log.push(someLogic.someValue); }); $effect(() => { - // Does not trigger log.push('class trigger ' + someLogic.isAboveThree) }); $effect(() => { - // Does Triggers log.push('local trigger ' + localDerived) });