diff --git a/.changeset/four-paths-cheer.md b/.changeset/four-paths-cheer.md
new file mode 100644
index 000000000000..54a697a8a42b
--- /dev/null
+++ b/.changeset/four-paths-cheer.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: simplify connection/disconnection logic
diff --git a/.changeset/whole-webs-stick.md b/.changeset/whole-webs-stick.md
new file mode 100644
index 000000000000..fe8b614a019e
--- /dev/null
+++ b/.changeset/whole-webs-stick.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: reconnect deriveds to effect tree when time-travelling
diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js
index c2f7861b78ae..b39afef51682 100644
--- a/packages/svelte/src/internal/client/constants.js
+++ b/packages/svelte/src/internal/client/constants.js
@@ -6,6 +6,13 @@ export const BLOCK_EFFECT = 1 << 4;
export const BRANCH_EFFECT = 1 << 5;
export const ROOT_EFFECT = 1 << 6;
export const BOUNDARY_EFFECT = 1 << 7;
+/**
+ * Indicates that a reaction is connected to an effect root — either it is an effect,
+ * or it is a derived that is depended on by at least one effect. If a derived has
+ * no dependents, we can disconnect it from the graph, allowing it to either be
+ * GC'd or reconnected later if an effect comes to depend on it again
+ */
+export const CONNECTED = 1 << 9;
export const CLEAN = 1 << 10;
export const DIRTY = 1 << 11;
export const MAYBE_DIRTY = 1 << 12;
@@ -26,8 +33,6 @@ export const EFFECT_PRESERVED = 1 << 19;
export const USER_EFFECT = 1 << 20;
// Flags exclusive to deriveds
-export const UNOWNED = 1 << 8;
-export const DISCONNECTED = 1 << 9;
/**
* 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
diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js
index 1eb640ad260c..7e6f3c6f604f 100644
--- a/packages/svelte/src/internal/client/reactivity/deriveds.js
+++ b/packages/svelte/src/internal/client/reactivity/deriveds.js
@@ -9,15 +9,14 @@ import {
EFFECT_PRESERVED,
MAYBE_DIRTY,
STALE_REACTION,
- UNOWNED,
ASYNC,
- WAS_MARKED
+ WAS_MARKED,
+ CONNECTED
} from '#client/constants';
import {
active_reaction,
active_effect,
set_signal_status,
- skip_reaction,
update_reaction,
increment_write_version,
set_active_effect,
@@ -27,7 +26,7 @@ import {
import { equals, safe_equals } from './equality.js';
import * as e from '../errors.js';
import * as w from '../warnings.js';
-import { async_effect, destroy_effect, teardown } from './effects.js';
+import { async_effect, destroy_effect, effect_tracking, teardown } from './effects.js';
import { eager_effects, internal_set, set_eager_effects, source } from './sources.js';
import { get_stack } from '../dev/tracing.js';
import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js';
@@ -61,9 +60,7 @@ export function derived(fn) {
? /** @type {Derived} */ (active_reaction)
: null;
- if (active_effect === null || (parent_derived !== null && (parent_derived.f & UNOWNED) !== 0)) {
- flags |= UNOWNED;
- } else {
+ if (active_effect !== null) {
// Since deriveds are evaluated lazily, any effects created inside them are
// created too late to ensure that the parent effect is added to the tree
active_effect.f |= EFFECT_PRESERVED;
@@ -368,12 +365,16 @@ export function update_derived(derived) {
return;
}
+ // During time traveling we don't want to reset the status so that
+ // traversal of the graph in the other batches still happens
if (batch_values !== null) {
- batch_values.set(derived, derived.v);
+ // only cache the value if we're in a tracking context, otherwise we won't
+ // clear the cache in `mark_reactions` when dependencies are updated
+ if (effect_tracking()) {
+ batch_values.set(derived, derived.v);
+ }
} else {
- var status =
- (skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY : CLEAN;
-
+ var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN;
set_signal_status(derived, status);
}
}
diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js
index 8c4b84438c5b..5d7c0ef871fd 100644
--- a/packages/svelte/src/internal/client/reactivity/effects.js
+++ b/packages/svelte/src/internal/client/reactivity/effects.js
@@ -25,7 +25,6 @@ import {
ROOT_EFFECT,
EFFECT_TRANSPARENT,
DERIVED,
- UNOWNED,
CLEAN,
EAGER_EFFECT,
HEAD_EFFECT,
@@ -33,7 +32,8 @@ import {
EFFECT_PRESERVED,
STALE_REACTION,
USER_EFFECT,
- ASYNC
+ ASYNC,
+ CONNECTED
} from '#client/constants';
import * as e from '../errors.js';
import { DEV } from 'esm-env';
@@ -48,11 +48,11 @@ import { without_reactive_context } from '../dom/elements/bindings/shared.js';
* @param {'$effect' | '$effect.pre' | '$inspect'} rune
*/
export function validate_effect(rune) {
- if (active_effect === null && active_reaction === null) {
- e.effect_orphan(rune);
- }
+ if (active_effect === null) {
+ if (active_reaction === null) {
+ e.effect_orphan(rune);
+ }
- if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0 && active_effect === null) {
e.effect_in_unowned_derived();
}
@@ -103,7 +103,7 @@ function create_effect(type, fn, sync, push = true) {
deps: null,
nodes_start: null,
nodes_end: null,
- f: type | DIRTY,
+ f: type | DIRTY | CONNECTED,
first: null,
fn,
last: null,
diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js
index b480d4155aa9..8ae406b57b30 100644
--- a/packages/svelte/src/internal/client/reactivity/sources.js
+++ b/packages/svelte/src/internal/client/reactivity/sources.js
@@ -23,18 +23,18 @@ import {
DIRTY,
BRANCH_EFFECT,
EAGER_EFFECT,
- UNOWNED,
MAYBE_DIRTY,
BLOCK_EFFECT,
ROOT_EFFECT,
ASYNC,
- WAS_MARKED
+ WAS_MARKED,
+ CONNECTED
} from '#client/constants';
import * as e from '../errors.js';
import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
import { get_stack, tag_proxy } from '../dev/tracing.js';
import { component_context, is_runes } from '../context.js';
-import { Batch, eager_block_effects, schedule_effect } from './batch.js';
+import { Batch, batch_values, eager_block_effects, schedule_effect } from './batch.js';
import { proxy } from '../proxy.js';
import { execute_derived } from './deriveds.js';
@@ -211,7 +211,8 @@ export function internal_set(source, value) {
if ((source.f & DIRTY) !== 0) {
execute_derived(/** @type {Derived} */ (source));
}
- set_signal_status(source, (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY);
+
+ set_signal_status(source, (source.f & CONNECTED) !== 0 ? CLEAN : MAYBE_DIRTY);
}
source.wv = increment_write_version();
@@ -333,9 +334,17 @@ function mark_reactions(signal, status) {
}
if ((flags & DERIVED) !== 0) {
+ var derived = /** @type {Derived} */ (reaction);
+
+ batch_values?.delete(derived);
+
if ((flags & WAS_MARKED) === 0) {
- reaction.f |= WAS_MARKED;
- mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY);
+ // Only connected deriveds can be reliably unmarked right away
+ if (flags & CONNECTED) {
+ reaction.f |= WAS_MARKED;
+ }
+
+ mark_reactions(derived, MAYBE_DIRTY);
}
} else if (not_dirty) {
if ((flags & BLOCK_EFFECT) !== 0) {
diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js
index 76531d33207e..258f6962fa81 100644
--- a/packages/svelte/src/internal/client/runtime.js
+++ b/packages/svelte/src/internal/client/runtime.js
@@ -4,6 +4,7 @@ import { get_descriptors, get_prototype_of, index_of } from '../shared/utils.js'
import {
destroy_block_effect_children,
destroy_effect_children,
+ effect_tracking,
execute_effect_teardown
} from './reactivity/effects.js';
import {
@@ -11,13 +12,12 @@ import {
MAYBE_DIRTY,
CLEAN,
DERIVED,
- UNOWNED,
DESTROYED,
BRANCH_EFFECT,
STATE_SYMBOL,
BLOCK_EFFECT,
ROOT_EFFECT,
- DISCONNECTED,
+ CONNECTED,
REACTION_IS_UPDATING,
STALE_REACTION,
ERROR_VALUE,
@@ -137,10 +137,6 @@ export function set_update_version(value) {
update_version = value;
}
-// If we are working with a get() chain that has no active container,
-// to prevent memory leaks, we skip adding the reaction.
-export let skip_reaction = false;
-
export function increment_write_version() {
return ++write_version;
}
@@ -158,55 +154,18 @@ export function is_dirty(reaction) {
return true;
}
+ if (flags & DERIVED) {
+ reaction.f &= ~WAS_MARKED;
+ }
+
if ((flags & MAYBE_DIRTY) !== 0) {
var dependencies = reaction.deps;
- var is_unowned = (flags & UNOWNED) !== 0;
-
- if (flags & DERIVED) {
- reaction.f &= ~WAS_MARKED;
- }
if (dependencies !== null) {
- var i;
- var dependency;
- var is_disconnected = (flags & DISCONNECTED) !== 0;
- var is_unowned_connected = is_unowned && active_effect !== null && !skip_reaction;
var length = dependencies.length;
- // 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, unless the effect has already been destroyed
- // (which can happen if the derived is read by an async derived)
- if (
- (is_disconnected || is_unowned_connected) &&
- (active_effect === null || (active_effect.f & DESTROYED) === 0)
- ) {
- 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(derived)) {
- (dependency.reactions ??= []).push(derived);
- }
- }
-
- if (is_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;
- }
- }
-
- for (i = 0; i < length; i++) {
- dependency = dependencies[i];
+ for (var i = 0; i < length; i++) {
+ var dependency = dependencies[i];
if (is_dirty(/** @type {Derived} */ (dependency))) {
update_derived(/** @type {Derived} */ (dependency));
@@ -218,9 +177,12 @@ export function is_dirty(reaction) {
}
}
- // Unowned signals should never be marked as clean unless they
- // are used within an active_effect without skip_reaction
- if (!is_unowned || (active_effect !== null && !skip_reaction)) {
+ if (
+ (flags & CONNECTED) !== 0 &&
+ // During time traveling we don't want to reset the status so that
+ // traversal of the graph in the other batches still happens
+ batch_values === null
+ ) {
set_signal_status(reaction, CLEAN);
}
}
@@ -263,7 +225,6 @@ export function update_reaction(reaction) {
var previous_skipped_deps = skipped_deps;
var previous_untracked_writes = untracked_writes;
var previous_reaction = active_reaction;
- var previous_skip_reaction = skip_reaction;
var previous_sources = current_sources;
var previous_component_context = component_context;
var previous_untracking = untracking;
@@ -274,8 +235,6 @@ export function update_reaction(reaction) {
new_deps = /** @type {null | Value[]} */ (null);
skipped_deps = 0;
untracked_writes = null;
- skip_reaction =
- (flags & UNOWNED) !== 0 && (untracking || !is_updating_effect || active_reaction === null);
active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null;
current_sources = null;
@@ -311,12 +270,7 @@ export function update_reaction(reaction) {
reaction.deps = deps = new_deps;
}
- if (
- !skip_reaction ||
- // Deriveds that already have reactions can cleanup, so we still add them as reactions
- ((flags & DERIVED) !== 0 &&
- /** @type {import('#client').Derived} */ (reaction).reactions !== null)
- ) {
+ if (is_updating_effect && effect_tracking() && (reaction.f & CONNECTED) !== 0) {
for (i = skipped_deps; i < deps.length; i++) {
(deps[i].reactions ??= []).push(reaction);
}
@@ -373,7 +327,6 @@ export function update_reaction(reaction) {
skipped_deps = previous_skipped_deps;
untracked_writes = previous_untracked_writes;
active_reaction = previous_reaction;
- skip_reaction = previous_skip_reaction;
current_sources = previous_sources;
set_component_context(previous_component_context);
untracking = previous_untracking;
@@ -415,9 +368,10 @@ function remove_reaction(signal, dependency) {
) {
set_signal_status(dependency, MAYBE_DIRTY);
// If we are working with a derived that is owned by an effect, then mark it as being
- // disconnected.
- if ((dependency.f & (UNOWNED | DISCONNECTED)) === 0) {
- dependency.f ^= DISCONNECTED;
+ // disconnected and remove the mark flag, as it cannot be reliably removed otherwise
+ if ((dependency.f & CONNECTED) !== 0) {
+ dependency.f ^= CONNECTED;
+ dependency.f &= ~WAS_MARKED;
}
// Disconnect any reactions owned by this reaction
destroy_derived_effects(/** @type {Derived} **/ (dependency));
@@ -564,10 +518,7 @@ export function get(signal) {
skipped_deps++;
} else if (new_deps === null) {
new_deps = [signal];
- } else if (!skip_reaction || !new_deps.includes(signal)) {
- // Normally we can push duplicated dependencies to `new_deps`, but if we're inside
- // an unowned derived because skip_reaction is true, then we need to ensure that
- // we don't have duplicates
+ } else if (!new_deps.includes(signal)) {
new_deps.push(signal);
}
}
@@ -585,20 +536,6 @@ export function get(signal) {
}
}
}
- } else if (
- is_derived &&
- /** @type {Derived} */ (signal).deps === null &&
- /** @type {Derived} */ (signal).effects === null
- ) {
- var derived = /** @type {Derived} */ (signal);
- var parent = derived.parent;
-
- if (parent !== null && (parent.f & UNOWNED) === 0) {
- // If the derived is owned by another derived then mark it as unowned
- // as the derived value might have been referenced in a different context
- // since and thus its parent might not be its true owner anymore
- derived.f ^= UNOWNED;
- }
}
if (DEV) {
@@ -657,7 +594,7 @@ export function get(signal) {
}
if (is_derived) {
- derived = /** @type {Derived} */ (signal);
+ var derived = /** @type {Derived} */ (signal);
var value = derived.v;
@@ -684,9 +621,11 @@ export function get(signal) {
if (is_dirty(derived)) {
update_derived(derived);
}
- }
- if (batch_values?.has(signal)) {
+ if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) {
+ reconnect(derived);
+ }
+ } else if (batch_values?.has(signal)) {
return batch_values.get(signal);
}
@@ -697,6 +636,25 @@ export function get(signal) {
return signal.v;
}
+/**
+ * (Re)connect a disconnected derived, so that it is notified
+ * of changes in `mark_reactions`
+ * @param {Derived} derived
+ */
+function reconnect(derived) {
+ if (derived.deps === null) return;
+
+ derived.f ^= CONNECTED;
+
+ for (const dep of derived.deps) {
+ (dep.reactions ??= []).push(derived);
+
+ if ((dep.f & DERIVED) !== 0 && (dep.f & CONNECTED) === 0) {
+ reconnect(/** @type {Derived} */ (dep));
+ }
+ }
+}
+
/** @param {Derived} derived */
function depends_on_old_values(derived) {
if (derived.v === UNINITIALIZED) return true; // we don't know, so assume the worst
diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/Component.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/Component.svelte
new file mode 100644
index 000000000000..200778dc5bc9
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/Component.svelte
@@ -0,0 +1,27 @@
+
diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/_config.js
new file mode 100644
index 000000000000..15bb42074f73
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/_config.js
@@ -0,0 +1,16 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+ async test({ assert, target, logs }) {
+ const button = target.querySelector('button');
+
+ button?.click();
+ await tick();
+ assert.deepEqual(logs, [5]);
+
+ button?.click();
+ await tick();
+ assert.deepEqual(logs, [5, 7]);
+ }
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/main.svelte
new file mode 100644
index 000000000000..bd82e35a3bc3
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/async-derived-in-multiple-effects/main.svelte
@@ -0,0 +1,19 @@
+
+
+
+ {await new Promise((r) => {
+ // long enough for the test to do all its other stuff while this is pending
+ setTimeout(r, 10);
+ })}
+ {#snippet pending()}{/snippet}
+
+
+ count += 1}>{count}
+
+{#if count > 0}
+ {double}
diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js
new file mode 100644
index 000000000000..fc0135623d7a
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js
@@ -0,0 +1,30 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+ async test({ assert, target }) {
+ const button = target.querySelector('button');
+
+ button?.click();
+ await tick();
+
+ assert.htmlEqual(
+ target.innerHTML,
+ `
+ 1
+ 2
+ `
+ );
+
+ button?.click();
+ await tick();
+
+ assert.htmlEqual(
+ target.innerHTML,
+ `
+ 2
+ 4
+ `
+ );
+ }
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte
new file mode 100644
index 000000000000..bd82e35a3bc3
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte
@@ -0,0 +1,19 @@
+
+
+
+ {await new Promise((r) => {
+ // long enough for the test to do all its other stuff while this is pending
+ setTimeout(r, 10);
+ })}
+ {#snippet pending()}{/snippet}
+
+
+ count += 1}>{count}
+
+{#if count > 0}
+