chore: remove UNOWNED flag (#17105)

Fixes #17024
Fixes #17049 (comment) (and therefore everything that was still buggy in that issue I think)

* chore: remove unowned check when calling `e.effect_in_unowned_derived`

* WIP

* all non-unit tests passing

* tidy

* WIP

* WIP

* WIP

* note to self

* fix

* fix

* hmm maybe not

* try this

* simplify

* remove skip_reaction

* docs

* add changeset, in case this results in changed behaviour

* Update packages/svelte/src/internal/client/reactivity/effects.js

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* fix #17024

* fix comment

* revert

* fix

* dry

* changeset

* fix WAS_MARKED logic

* failing test (that uncovered other unrelated bug) + fix

* fix: delete from batch_values on updates (#17115)

* fix: delete from batch_values on updates

This fixes a bug where a derived would still show its old value even after it was indirectly updated again within the same batch. This can for example happen by reading a derived on an effect, then writing to a source in that same effect that makes the derived update, and then read the derived value in a sibling effect - it still shows the old value without the fix.

The fix is to _delete_ the value from batch_values, as it's now the newest value across all batches. In order to not prevent breakage on other batches we have to leave the status of deriveds as-is, i.e. within is_dirty and update_derived we cannot update its status. That might be a bit more inefficient as you now have to traverse the graph for each `get` of that derived (it's a bit like they are all disconnected) but we can always optimize that later if need be.

Another advantage of this fix is that we can get rid of duplicate logic we had to add about unmarking and reconnecting deriveds, because that logic was only needed for the case where deriveds are read after they are updated, which now no longer hits that if-branch

* keep derived cache, but clear it in mark_reactions (#17116)

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>

Author: Rich-Harris

.changeset/four-paths-cheer.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: simplify connection/disconnection logic

.changeset/whole-webs-stick.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: reconnect deriveds to effect tree when time-travelling

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

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);
 	}
 }

packages/svelte/src/internal/client/reactivity/effects.js

@@ -25,15 +25,15 @@ import {
 	ROOT_EFFECT,
 	EFFECT_TRANSPARENT,
 	DERIVED,
-	UNOWNED,
 	CLEAN,
 	EAGER_EFFECT,
 	HEAD_EFFECT,
 	MAYBE_DIRTY,
 	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,

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) {