diff --git a/.changeset/hot-kangaroos-invite.md b/.changeset/hot-kangaroos-invite.md new file mode 100644 index 0000000000..df21009329 --- /dev/null +++ b/.changeset/hot-kangaroos-invite.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: refactor internal signal dependency heuristic diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 27d82bb0a4..42b13ac584 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -39,7 +39,7 @@ export function template(content, flags) { return hydrate_node; } - if (!node) { + if (node === undefined) { node = create_fragment_from_html(has_start ? content : '' + content); if (!is_fragment) node = /** @type {Node} */ (get_first_child(node)); } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 68a309cbda..cd9a9f9171 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -375,8 +375,10 @@ export function destroy_effect(effect, remove_dom = true) { remove_reactions(effect, 0); set_signal_status(effect, DESTROYED); - if (effect.transitions) { - for (const transition of effect.transitions) { + var transitions = effect.transitions; + + if (transitions !== null) { + for (const transition of transitions) { transition.stop(); } } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 3425d6dadc..86427a7509 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -139,8 +139,10 @@ function mark_reactions(signal, status) { if (reactions === null) return; var runes = is_runes(); + var length = reactions.length; - for (var reaction of reactions) { + for (var i = 0; i < length; i++) { + var reaction = reactions[i]; var flags = reaction.f; // Skip any effects that are already dirty diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 4a53a70043..1e347c011f 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -9,7 +9,7 @@ export interface Signal { export interface Value extends Signal { /** Signals that read from this signal */ - reactions: null | Set; + reactions: null | Reaction[]; /** Equality function */ equals: Equals; /** The latest value for this signal */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 6a32095b6d..4e2a2ee031 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -169,7 +169,7 @@ export function check_dirtiness(reaction) { if ((flags & DISCONNECTED) !== 0) { for (i = 0; i < dependencies.length; i++) { - (dependencies[i].reactions ??= new Set()).add(reaction); + (dependencies[i].reactions ??= []).push(reaction); } reaction.f ^= DISCONNECTED; @@ -188,11 +188,11 @@ export function check_dirtiness(reaction) { if (is_unowned) { // TODO is there a more logical place to do this work? - if (!current_skip_reaction && !dependency?.reactions?.has(reaction)) { + if (!current_skip_reaction && !dependency?.reactions?.includes(reaction)) { // If we are working with an unowned signal as part of an effect (due to !current_skip_reaction) // and the version hasn't changed, we still need to check that this reaction // if linked to the dependency source – otherwise future updates will not be caught. - (dependency.reactions ??= new Set()).add(reaction); + (dependency.reactions ??= []).push(reaction); } } } @@ -291,26 +291,9 @@ export function update_reaction(reaction) { var deps = reaction.deps; if (new_deps !== null) { - var dependency; var i; - if (deps !== null) { - /** All dependencies of the reaction, including those tracked on the previous run */ - var array = skipped_deps === 0 ? new_deps : deps.slice(0, skipped_deps).concat(new_deps); - - // If we have more than 16 elements in the array then use a Set for faster performance - // TODO: evaluate if we should always just use a Set or not here? - var set = array.length > 16 ? new Set(array) : null; - - // Remove dependencies that should no longer be tracked - for (i = skipped_deps; i < deps.length; i++) { - dependency = deps[i]; - - if (set !== null ? !set.has(dependency) : !array.includes(dependency)) { - remove_reaction(reaction, dependency); - } - } - } + remove_reactions(reaction, skipped_deps); if (deps !== null && skipped_deps > 0) { deps.length = skipped_deps + new_deps.length; @@ -323,7 +306,7 @@ export function update_reaction(reaction) { if (!current_skip_reaction) { for (i = skipped_deps; i < deps.length; i++) { - (deps[i].reactions ??= new Set()).add(reaction); + (deps[i].reactions ??= []).push(reaction); } } } else if (deps !== null && skipped_deps < deps.length) { @@ -350,9 +333,16 @@ export function update_reaction(reaction) { function remove_reaction(signal, dependency) { let reactions = dependency.reactions; if (reactions !== null) { - reactions.delete(signal); - if (reactions.size === 0) { - reactions = dependency.reactions = null; + var index = reactions.indexOf(signal); + if (index !== -1) { + var new_length = reactions.length - 1; + if (new_length === 0) { + reactions = dependency.reactions = null; + } else { + // Swap with last element and then remove. + reactions[index] = reactions[new_length]; + reactions.pop(); + } } } // If the derived has no reactions, then we can disconnect it from the graph, @@ -377,19 +367,8 @@ export function remove_reactions(signal, start_index) { var dependencies = signal.deps; if (dependencies === null) return; - var active_dependencies = start_index === 0 ? null : dependencies.slice(0, start_index); - var seen = new Set(); - for (var i = start_index; i < dependencies.length; i++) { - var dependency = dependencies[i]; - - if (seen.has(dependency)) continue; - seen.add(dependency); - - // Avoid removing a reaction if we know that it is active (start_index will not be 0) - if (active_dependencies === null || !active_dependencies.includes(dependency)) { - remove_reaction(signal, dependency); - } + remove_reaction(signal, dependencies[i]); } } @@ -726,16 +705,10 @@ export function get(signal) { // rather than updating `new_deps`, which creates GC cost if (new_deps === null && deps !== null && deps[skipped_deps] === signal) { skipped_deps++; - } - - // Otherwise, create or push to `new_deps`, but only if this - // dependency wasn't the last one that was accessed - else if (deps === null || skipped_deps === 0 || deps[skipped_deps - 1] !== signal) { - if (new_deps === null) { - new_deps = [signal]; - } else if (new_deps[new_deps.length - 1] !== signal) { - new_deps.push(signal); - } + } else if (new_deps === null) { + new_deps = [signal]; + } else { + new_deps.push(signal); } if ( diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 7a68e92bc7..270b5125a4 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -236,13 +236,13 @@ describe('signals', () => { return () => { flushSync(() => set(count, 1)); // Ensure we're not leaking consumers - assert.deepEqual(count.reactions?.size, 1); + assert.deepEqual(count.reactions?.length, 1); flushSync(() => set(count, 2)); // Ensure we're not leaking consumers - assert.deepEqual(count.reactions?.size, 1); + assert.deepEqual(count.reactions?.length, 1); flushSync(() => set(count, 3)); // Ensure we're not leaking consumers - assert.deepEqual(count.reactions?.size, 1); + assert.deepEqual(count.reactions?.length, 1); assert.deepEqual(log, [0, 1, 2, 3]); }; }); @@ -304,8 +304,8 @@ describe('signals', () => { flushSync(() => set(count, 4)); flushSync(() => set(count, 0)); // Ensure we're not leaking consumers - assert.deepEqual(count.reactions?.size, 1); - assert.deepEqual(calc.reactions?.size, 1); + assert.deepEqual(count.reactions?.length, 2); + assert.deepEqual(calc.reactions?.length, 1); assert.deepEqual(log, [0, 2, 'limit', 0]); destroy(); // Ensure we're not leaking consumers @@ -484,7 +484,7 @@ describe('signals', () => { return () => { flushSync(); assert.equal(a?.deps?.length, 1); - assert.equal(state?.reactions?.size, 1); + assert.equal(state?.reactions?.length, 1); destroy(); assert.equal(a?.deps?.length, 1); assert.equal(state?.reactions, null); @@ -658,12 +658,12 @@ describe('signals', () => { }); assert.equal($.get(d), 0); - assert.equal(count.reactions?.size, 1); + assert.equal(count.reactions?.length, 1); assert.equal(d.deps?.length, 1); set(count, 1); assert.equal($.get(d), 2); - assert.equal(count.reactions?.size, 1); + assert.equal(count.reactions?.length, 1); assert.equal(d.deps?.length, 1); destroy();