diff --git a/.changeset/grumpy-pens-press.md b/.changeset/grumpy-pens-press.md new file mode 100644 index 0000000000..0ba87ab0c5 --- /dev/null +++ b/.changeset/grumpy-pens-press.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove `WAS_MARKED` flag in favor of `Set` diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a3ad988ba1..ebb73369c2 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -44,14 +44,6 @@ export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; export const EFFECT_OFFSCREEN = 1 << 25; -// Flags exclusive to deriveds -/** - * Tells that we marked this derived and its reactions as visited during the "mark as (maybe) dirty"-phase. - * Will be lifted during execution of the derived and during checking its dirty state (both are necessary - * because a derived might be checked but not executed). - */ -export const WAS_MARKED = 1 << 16; - // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; export const ASYNC = 1 << 22; diff --git a/packages/svelte/src/internal/client/dev/debug.js b/packages/svelte/src/internal/client/dev/debug.js index 83cc510ae2..2513d15c97 100644 --- a/packages/svelte/src/internal/client/dev/debug.js +++ b/packages/svelte/src/internal/client/dev/debug.js @@ -15,7 +15,6 @@ import { MAYBE_DIRTY, RENDER_EFFECT, ROOT_EFFECT, - WAS_MARKED, MANAGED_EFFECT } from '#client/constants'; import { snapshot } from '../../shared/clone.js'; @@ -204,7 +203,6 @@ export function log_reactions(signal) { if ((flags & DIRTY) !== 0) names.push('DIRTY'); if ((flags & MAYBE_DIRTY) !== 0) names.push('MAYBE_DIRTY'); if ((flags & CONNECTED) !== 0) names.push('CONNECTED'); - if ((flags & WAS_MARKED) !== 0) names.push('WAS_MARKED'); if ((flags & INERT) !== 0) names.push('INERT'); if ((flags & DESTROYED) !== 0) names.push('DESTROYED'); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 5af51449ad..92490da442 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -9,7 +9,6 @@ import { EFFECT_PRESERVED, STALE_REACTION, ASYNC, - WAS_MARKED, DESTROYED, CLEAN, REACTION_RAN, @@ -362,7 +361,6 @@ export function execute_derived(derived) { stack.push(derived); - derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { @@ -372,7 +370,6 @@ export function execute_derived(derived) { } } else { try { - derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 9235c5f673..a6680c4d10 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -25,9 +25,7 @@ import { MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, - ASYNC, - WAS_MARKED, - CONNECTED + ASYNC } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -171,6 +169,15 @@ export function set(source, value, should_proxy = false) { return internal_set(source, new_value, legacy_updates); } +/** + * A set of signals we have already seen while traversing in mark_reactions. + * Not always set to balance the common case of sources only having a couple + * of (transitive) dependencies (where always creating a Set would be bad for perf) + * with the edge case of extremely deep or wide dependency arrays with cycles. + * @type {Set | null} + */ +var seen = null; + /** * @template V * @param {Source} source @@ -234,7 +241,8 @@ export function internal_set(source, value, updated_during_traversal = null) { // For debugging, in case you want to know which reactions are being scheduled: // log_reactions(source); - mark_reactions(source, DIRTY, updated_during_traversal); + seen = null; + mark_reactions(source, DIRTY, updated_during_traversal, 0); // 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 @@ -321,15 +329,23 @@ export function increment(source) { * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY * @param {Effect[] | null} updated_during_traversal + * @param {number} depth * @returns {void} */ -function mark_reactions(signal, status, updated_during_traversal) { +function mark_reactions(signal, status, updated_during_traversal, depth) { var reactions = signal.reactions; if (reactions === null) return; var runes = is_runes(); var length = reactions.length; + if (length > 100 || depth > 100) seen = new Set(); + + if (seen !== null) { + if (seen.has(signal)) return; + seen.add(signal); + } + for (var i = 0; i < length; i++) { var reaction = reactions[i]; var flags = reaction.f; @@ -354,15 +370,7 @@ function mark_reactions(signal, status, updated_during_traversal) { var derived = /** @type {Derived} */ (reaction); batch_values?.delete(derived); - - if ((flags & WAS_MARKED) === 0) { - // Only connected deriveds can be reliably unmarked right away - if (flags & CONNECTED) { - reaction.f |= WAS_MARKED; - } - - mark_reactions(derived, MAYBE_DIRTY, updated_during_traversal); - } + mark_reactions(derived, MAYBE_DIRTY, updated_during_traversal, depth + 1); } else if (not_dirty) { var effect = /** @type {Effect} */ (reaction); diff --git a/packages/svelte/src/internal/client/reactivity/utils.js b/packages/svelte/src/internal/client/reactivity/utils.js index 0d27cb8b84..3280f2372e 100644 --- a/packages/svelte/src/internal/client/reactivity/utils.js +++ b/packages/svelte/src/internal/client/reactivity/utils.js @@ -1,24 +1,7 @@ -/** @import { Derived, Effect, Value } from '#client' */ -import { CLEAN, DERIVED, DIRTY, MAYBE_DIRTY, WAS_MARKED } from '#client/constants'; +/** @import { Effect } from '#client' */ +import { CLEAN, DIRTY, MAYBE_DIRTY } from '#client/constants'; import { set_signal_status } from './status.js'; -/** - * @param {Value[] | null} deps - */ -function clear_marked(deps) { - if (deps === null) return; - - for (const dep of deps) { - if ((dep.f & DERIVED) === 0 || (dep.f & WAS_MARKED) === 0) { - continue; - } - - dep.f ^= WAS_MARKED; - - clear_marked(/** @type {Derived} */ (dep).deps); - } -} - /** * @param {Effect} effect * @param {Set} dirty_effects @@ -31,10 +14,6 @@ export function defer_effect(effect, dirty_effects, maybe_dirty_effects) { maybe_dirty_effects.add(effect); } - // Since we're not executing these effects now, we need to clear any WAS_MARKED flags - // so that other batches can correctly reach these effects during their own traversal - clear_marked(effect.deps); - // mark as clean so they get scheduled if they depend on pending async state set_signal_status(effect, CLEAN); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 033afe98dc..d18ba19916 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -21,7 +21,6 @@ import { REACTION_IS_UPDATING, STALE_REACTION, ERROR_VALUE, - WAS_MARKED, MANAGED_EFFECT, REACTION_RAN } from './constants.js'; @@ -160,10 +159,6 @@ export function is_dirty(reaction) { return true; } - if (flags & DERIVED) { - reaction.f &= ~WAS_MARKED; - } - if ((flags & MAYBE_DIRTY) !== 0) { var dependencies = /** @type {Value[]} */ (reaction.deps); var length = dependencies.length; @@ -392,11 +387,8 @@ function remove_reaction(signal, dependency) { ) { var derived = /** @type {Derived} */ (dependency); - // If we are working with a derived that is owned by an effect, then mark it as being - // disconnected and remove the mark flag, as it cannot be reliably removed otherwise if ((derived.f & CONNECTED) !== 0) { derived.f ^= CONNECTED; - derived.f &= ~WAS_MARKED; } // In a fork it's possible that a derived is executed and gets reactions, then commits, but is diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte new file mode 100644 index 0000000000..9771762c0b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js new file mode 100644 index 0000000000..60985cdd12 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js @@ -0,0 +1,29 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [show, hide] = target.querySelectorAll('button'); + + hide.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + + ` + ); + + show.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + +
visible
+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte new file mode 100644 index 0000000000..3359a6305b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte @@ -0,0 +1,14 @@ + + + + +{#if visible2} + +
visible
+{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js new file mode 100644 index 0000000000..f3f8c33dbe --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js @@ -0,0 +1,13 @@ +class RawStore { + values = $state.raw({ visible: true }); + + get(key) { + return this.values[key]; + } + + set(key, value) { + this.values = { ...this.values, [key]: value }; + } +} + +export const store = new RawStore(); \ No newline at end of file