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

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

@@ -286,12 +286,11 @@ export class Batch {
 			this.previous.set(source, value);
 		}
 
+		// Don't save errors in `batch_values`, or they won't be thrown in `runtime.js#get`
 		if ((source.f & ERROR_VALUE) === 0) {
 			this.current.set(source, source.v);
+			batch_values?.set(source, source.v);
 		}
-
-		// The value is now the newest known value for this source, therefore remove it from batch_values
-		batch_values?.delete(source);
 	}
 
 	activate() {

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

@@ -26,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';
@@ -368,17 +368,11 @@ export function update_derived(derived) {
 	// 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) {
-		// Delete the value as the current one is now the latest.
+		// only cache the value if we're in a tracking context, otherwise we won't
-		// Deleting instead of updating handles the case where a derived
+		// clear the cache in `mark_reactions` when dependencies are updated
-		// is subsequently indirectly updated in the same batch — without
+		if (effect_tracking()) {
-		// deleting here we would incorrectly get the old value from `batch_values`
+			batch_values.set(derived, derived.v);
-		// instead of recomputing it. The one drawback is that it's now a bit
+		}
-		// more inefficient to get the value of that derived again in the same batch,
-		// as it has to check is_dirty all the way up the graph all the time.
-		// TODO if that turns out to be a performance problem, we could try
-		// to save the current status of the derived in a map and restore it
-		// before leaving the batch.
-		batch_values.delete(derived);
 	} else {
 		var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN;
 		set_signal_status(derived, status);

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

@@ -34,7 +34,7 @@ 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';
 
@@ -334,12 +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) {
 				// Only connected deriveds can be reliably unmarked right away
 				if (flags & CONNECTED) {
 					reaction.f |= WAS_MARKED;
 				}
-				mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY);
+
+				mark_reactions(derived, MAYBE_DIRTY);
 			}
 		} else if (not_dirty) {
 			if ((flags & BLOCK_EFFECT) !== 0) {