chore: simplify batch.apply() (#16945)

* chore: simplify `batch.apply()`

* belt and braces

* note to self

Author: Rich-Harris

.changeset/pretty-llamas-explode.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: simplify `batch.apply()`

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

@@ -44,12 +44,12 @@ export let current_batch = null;
 export let previous_batch = null;
 
 /**
- * When time travelling, we re-evaluate deriveds based on the temporary
- * values of their dependencies rather than their actual values, and cache
- * the results in this map rather than on the deriveds themselves
- * @type {Map<Derived, any> | null}
+ * When time travelling (i.e. working in one batch, while other batches
+ * still have ongoing work), we ignore the real values of affected
+ * signals in favour of their values within the batch
+ * @type {Map<Value, any> | null}
  */
-export let batch_deriveds = null;
+export let batch_values = null;
 
 /** @type {Set<() => void>} */
 export let effect_pending_updates = new Set();
@@ -152,7 +152,7 @@ export class Batch {
 
 		previous_batch = null;
 
-		var revert = Batch.apply(this);
+		this.apply();
 
 		for (const root of root_effects) {
 			this.#traverse_effect_tree(root);
@@ -161,6 +161,10 @@ export class Batch {
 		// if we didn't start any new async work, and no async work
 		// is outstanding from a previous flush, commit
 		if (this.#pending === 0) {
+			// TODO we need this because we commit _then_ flush effects...
+			// maybe there's a way we can reverse the order?
+			var previous_batch_sources = batch_values;
+
 			this.#commit();
 
 			var render_effects = this.#render_effects;
@@ -175,6 +179,7 @@ export class Batch {
 			previous_batch = this;
 			current_batch = null;
 
+			batch_values = previous_batch_sources;
 			flush_queued_effects(render_effects);
 			flush_queued_effects(effects);
 
@@ -187,7 +192,7 @@ export class Batch {
 			this.#defer_effects(this.#block_effects);
 		}
 
-		revert();
+		batch_values = null;
 
 		for (const effect of this.#boundary_async_effects) {
 			update_effect(effect);
@@ -274,6 +279,7 @@ export class Batch {
 		}
 
 		this.current.set(source, source.v);
+		batch_values?.set(source, source.v);
 	}
 
 	activate() {
@@ -282,6 +288,7 @@ export class Batch {
 
 	deactivate() {
 		current_batch = null;
+		batch_values = null;
 	}
 
 	flush() {
@@ -352,14 +359,14 @@ export class Batch {
 
 				if (queued_root_effects.length > 0) {
 					current_batch = batch;
-					const revert = Batch.apply(batch);
+					batch.apply();
 
 					for (const root of queued_root_effects) {
 						batch.#traverse_effect_tree(root);
 					}
 
 					queued_root_effects = [];
-					revert();
+					batch.deactivate();
 				}
 			}
 
@@ -423,49 +430,23 @@ export class Batch {
 		queue_micro_task(task);
 	}
 
-	/**
-	 * @param {Batch} current_batch
-	 */
-	static apply(current_batch) {
-		if (!async_mode_flag || batches.size === 1) {
-			return noop;
-		}
+	apply() {
+		if (!async_mode_flag || batches.size === 1) return;
 
 		// if there are multiple batches, we are 'time travelling' —
-		// we need to undo the changes belonging to any batch
-		// other than the current one
-
-		/** @type {Map<Source, { v: unknown, wv: number }>} */
-		var current_values = new Map();
-		batch_deriveds = new Map();
-
-		for (const [source, current] of current_batch.current) {
-			current_values.set(source, { v: source.v, wv: source.wv });
-			source.v = current;
-		}
+		// we need to override values with the ones in this batch...
+		batch_values = new Map(this.current);
 
+		// ...and undo changes belonging to other batches
 		for (const batch of batches) {
-			if (batch === current_batch) continue;
+			if (batch === this) continue;
 
 			for (const [source, previous] of batch.#previous) {
-				if (!current_values.has(source)) {
-					current_values.set(source, { v: source.v, wv: source.wv });
-					source.v = previous;
+				if (!batch_values.has(source)) {
+					batch_values.set(source, previous);
 				}
 			}
 		}
-
-		return () => {
-			for (const [source, { v, wv }] of current_values) {
-				// reset the source to the current value (unless
-				// it got a newer value as a result of effects running)
-				if (source.wv <= wv) {
-					source.v = v;
-				}
-			}
-
-			batch_deriveds = null;
-		};
 	}
 }
 

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

@@ -33,7 +33,7 @@ import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js';
 import { Boundary } from '../dom/blocks/boundary.js';
 import { component_context } from '../context.js';
 import { UNINITIALIZED } from '../../../constants.js';
-import { batch_deriveds, current_batch } from './batch.js';
+import { batch_values, current_batch } from './batch.js';
 import { unset_context } from './async.js';
 import { deferred } from '../../shared/utils.js';
 
@@ -336,6 +336,8 @@ export function update_derived(derived) {
 	var value = execute_derived(derived);
 
 	if (!derived.equals(value)) {
+		// TODO can we avoid setting `derived.v` when `batch_values !== null`,
+		// without causing the value to be stale later?
 		derived.v = value;
 		derived.wv = increment_write_version();
 	}
@@ -346,8 +348,8 @@ export function update_derived(derived) {
 		return;
 	}
 
-	if (batch_deriveds !== null) {
-		batch_deriveds.set(derived, derived.v);
+	if (batch_values !== null) {
+		batch_values.set(derived, derived.v);
 	} else {
 		var status =
 			(skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY : CLEAN;

packages/svelte/src/internal/client/runtime.js

@@ -42,7 +42,7 @@ import {
 	set_dev_stack
 } from './context.js';
 import * as w from './warnings.js';
-import { Batch, batch_deriveds, flushSync, schedule_effect } from './reactivity/batch.js';
+import { Batch, batch_values, flushSync, schedule_effect } from './reactivity/batch.js';
 import { handle_error } from './error-handling.js';
 import { UNINITIALIZED } from '../../constants.js';
 import { captured_signals } from './legacy.js';
@@ -671,15 +671,19 @@ export function get(signal) {
 	} else if (is_derived) {
 		derived = /** @type {Derived} */ (signal);
 
-		if (batch_deriveds?.has(derived)) {
-			return batch_deriveds.get(derived);
+		if (batch_values?.has(derived)) {
+			return batch_values.get(derived);
 		}
 
 		if (is_dirty(derived)) {
 			update_derived(derived);
 		}
 	}
 
+	if (batch_values?.has(signal)) {
+		return batch_values.get(signal);
+	}
+
 	if ((signal.f & ERROR_VALUE) !== 0) {
 		throw signal.v;
 	}