diff --git a/.changeset/nasty-pigs-lay.md b/.changeset/nasty-pigs-lay.md new file mode 100644 index 0000000000..64ef72103a --- /dev/null +++ b/.changeset/nasty-pigs-lay.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: when re-connecting unowned deriveds, remove their unowned flag diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 60b55970e6..59a7ed0f16 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,18 +1,9 @@ /** @import { Derived, Effect } from '#client' */ import { DEV } from 'esm-env'; -import { - CLEAN, - DERIVED, - DESTROYED, - DIRTY, - EFFECT_HAS_DERIVED, - MAYBE_DIRTY, - UNOWNED -} from '../constants.js'; +import { CLEAN, DERIVED, DIRTY, EFFECT_HAS_DERIVED, MAYBE_DIRTY, UNOWNED } from '../constants.js'; import { active_reaction, active_effect, - remove_reactions, set_signal_status, skip_reaction, update_reaction, diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index d157642d5d..5a3b30281f 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -10,7 +10,15 @@ import { import { get_descriptor, is_function } from '../../shared/utils.js'; import { mutable_source, set, source, update } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; -import { active_effect, get, captured_signals, set_active_effect, untrack } from '../runtime.js'; +import { + active_effect, + get, + captured_signals, + set_active_effect, + untrack, + active_reaction, + set_active_reaction +} from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; import { @@ -241,26 +249,6 @@ export function spread_props(...props) { return new Proxy({ props }, spread_props_handler); } -/** - * @template T - * @param {() => T} fn - * @returns {T} - */ -function with_parent_branch(fn) { - var effect = active_effect; - var previous_effect = active_effect; - - while (effect !== null && (effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0) { - effect = effect.parent; - } - try { - set_active_effect(effect); - return fn(); - } finally { - set_active_effect(previous_effect); - } -} - /** * This function is responsible for synchronizing a possibly bound prop with the inner component state. * It is used whenever the compiler sees that the component writes to the prop, or when it has a default prop_value. @@ -335,8 +323,8 @@ export function prop(props, key, flags, fallback) { } else { // Svelte 4 did not trigger updates when a primitive value was updated to the same value. // Replicate that behavior through using a derived - var derived_getter = with_parent_branch(() => - (immutable ? derived : derived_safe_equal)(() => /** @type {V} */ (props[key])) + var derived_getter = (immutable ? derived : derived_safe_equal)( + () => /** @type {V} */ (props[key]) ); derived_getter.f |= LEGACY_DERIVED_PROP; getter = () => { @@ -380,21 +368,19 @@ export function prop(props, key, flags, fallback) { // The derived returns the current value. The underlying mutable // source is written to from various places to persist this value. var inner_current_value = mutable_source(prop_value); - var current_value = with_parent_branch(() => - derived(() => { - var parent_value = getter(); - var child_value = get(inner_current_value); - - if (from_child) { - from_child = false; - was_from_child = true; - return child_value; - } + var current_value = derived(() => { + var parent_value = getter(); + var child_value = get(inner_current_value); + + if (from_child) { + from_child = false; + was_from_child = true; + return child_value; + } - was_from_child = false; - return (inner_current_value.v = parent_value); - }) - ); + was_from_child = false; + return (inner_current_value.v = parent_value); + }); if (!immutable) current_value.equals = safe_equals; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 30f14b7356..8b0f84268c 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -184,19 +184,28 @@ export function check_dirtiness(reaction) { // If we are working with a disconnected or an unowned signal that is now connected (due to an active effect) // then we need to re-connect the reaction to the dependency if (is_disconnected || is_unowned_connected) { + var derived = /** @type {Derived} */ (reaction); + var parent = derived.parent; + for (i = 0; i < length; i++) { dependency = dependencies[i]; // We always re-add all reactions (even duplicates) if the derived was // previously disconnected, however we don't if it was unowned as we // de-duplicate dependencies in that case - if (is_disconnected || !dependency?.reactions?.includes(reaction)) { - (dependency.reactions ??= []).push(reaction); + if (is_disconnected || !dependency?.reactions?.includes(derived)) { + (dependency.reactions ??= []).push(derived); } } if (is_disconnected) { - reaction.f ^= DISCONNECTED; + derived.f ^= DISCONNECTED; + } + // If the unowned derived is now fully connected to the graph again (it's unowned and reconnected, has a parent + // and the parent is not unowned), then we can mark it as connected again, removing the need for the unowned + // flag + if (is_unowned_connected && parent !== null && (parent.f & UNOWNED) === 0) { + derived.f ^= UNOWNED; } } diff --git a/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/_config.js new file mode 100644 index 0000000000..8cd4af0548 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/_config.js @@ -0,0 +1,25 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + let [btn1, btn2] = target.querySelectorAll('button'); + + btn1?.click(); + flushSync(); + + btn2?.click(); + flushSync(); + + btn1?.click(); + flushSync(); + + btn1?.click(); + flushSync(); + + assert.htmlEqual( + target.innerHTML, + `\n3\n\n1` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/main.svelte new file mode 100644 index 0000000000..48d4f5fd0b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-unowned-12/main.svelte @@ -0,0 +1,18 @@ + + + {linked.current} + {count} diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 46209ede81..046f833e0e 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -224,20 +224,75 @@ describe('signals', () => { }; }); - test('effects correctly handle unowned derived values that do not change', () => { - const log: number[] = []; + test('https://perf.js.hyoo.ru/#!bench=9h2as6_u0mfnn #2', () => { + let res: number[] = []; - let count = state(0); - const read = () => { - const x = derived(() => ({ count: $.get(count) })); - return $.get(x); + const numbers = Array.from({ length: 2 }, (_, i) => i); + const fib = (n: number): number => (n < 2 ? 1 : fib(n - 1) + fib(n - 2)); + const hard = (n: number, l: string) => n + fib(16); + + const A = state(0); + const B = state(0); + + return () => { + const C = derived(() => ($.get(A) % 2) + ($.get(B) % 2)); + const D = derived(() => numbers.map((i) => i + ($.get(A) % 2) - ($.get(B) % 2))); + const E = derived(() => hard($.get(C) + $.get(A) + $.get(D)[0]!, 'E')); + const F = derived(() => hard($.get(D)[0]! && $.get(B), 'F')); + const G = derived(() => $.get(C) + ($.get(C) || $.get(E) % 2) + $.get(D)[0]! + $.get(F)); + + const destroy = effect_root(() => { + effect(() => { + res.push(hard($.get(G), 'H')); + }); + effect(() => { + res.push($.get(G)); + }); + effect(() => { + res.push(hard($.get(F), 'J')); + }); + }); + + flushSync(); + + let i = 2; + while (--i) { + res.length = 0; + set(B, 1); + set(A, 1 + i * 2); + flushSync(); + + set(A, 2 + i * 2); + set(B, 2); + flushSync(); + + assert.equal(res.length, 4); + assert.deepEqual(res, [3198, 1601, 3195, 1598]); + } + + destroy(); + assert(A.reactions === null); + assert(B.reactions === null); }; - const derivedCount = derived(() => read().count); - user_effect(() => { - log.push($.get(derivedCount)); - }); + }); + + test('effects correctly handle unowned derived values that do not change', () => { + const log: number[] = []; return () => { + let count = state(0); + const read = () => { + const x = derived(() => ({ count: $.get(count) })); + return $.get(x); + }; + const derivedCount = derived(() => read().count); + + const destroy = effect_root(() => { + user_effect(() => { + log.push($.get(derivedCount)); + }); + }); + flushSync(() => set(count, 1)); // Ensure we're not leaking consumers assert.deepEqual(count.reactions?.length, 1); @@ -248,6 +303,8 @@ describe('signals', () => { // Ensure we're not leaking consumers assert.deepEqual(count.reactions?.length, 1); assert.deepEqual(log, [0, 1, 2, 3]); + + destroy(); }; }); @@ -343,25 +400,69 @@ describe('signals', () => { }; }); - let some_state = state({}); - let some_deps = derived(() => { - return [$.get(some_state)]; - }); - test('two effects with an unowned derived that has some dependencies', () => { const log: Array> = []; - render_effect(() => { - log.push($.get(some_deps)); - }); + return () => { + let some_state = state({}); + let some_deps = derived(() => { + return [$.get(some_state)]; + }); + let destroy2: any; - render_effect(() => { - log.push($.get(some_deps)); - }); + const destroy = effect_root(() => { + render_effect(() => { + $.untrack(() => { + log.push($.get(some_deps)); + }); + }); - return () => { + destroy2 = effect_root(() => { + render_effect(() => { + log.push($.get(some_deps)); + }); + + render_effect(() => { + log.push($.get(some_deps)); + }); + }); + }); + + set(some_state, {}); + flushSync(); + + assert.deepEqual(log, [[{}], [{}], [{}], [{}], [{}]]); + + destroy2(); + + set(some_state, {}); flushSync(); - assert.deepEqual(log, [[{}], [{}]]); + + assert.deepEqual(log, [[{}], [{}], [{}], [{}], [{}]]); + + log.length = 0; + + const destroy3 = effect_root(() => { + render_effect(() => { + $.untrack(() => { + log.push($.get(some_deps)); + }); + log.push($.get(some_deps)); + }); + }); + + set(some_state, {}); + flushSync(); + + assert.deepEqual(log, [[{}], [{}], [{}], [{}]]); + + destroy3(); + + assert(some_state.reactions === null); + + destroy(); + + assert(some_state.reactions === null); }; });