more

packages/svelte/src/internal/client/dom/blocks/branches.js

@@ -110,14 +110,16 @@ export class BranchManager {
 			}
 		}
 
-		for (const [b, k] of this.#batches) {
+		this.#batches.delete(batch);
-			this.#batches.delete(b);
 
-			if (b === batch) {
+		for (const [b, k] of this.#batches) {
+			if (b.id > batch.id) {
 				// keep values for newer batches
-				break;
+				continue;
 			}
 
+			this.#batches.delete(b);
+
 			const offscreen = this.#offscreen.get(k);
 
 			if (offscreen) {

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

@@ -114,28 +114,19 @@ export class Batch {
 	/** True as soon as `#process` was called */
 	started = false;
 
-	// TODO temporary
-	get next() {
-		return this.#next;
-	}
-	// TODO temporary
-	get prev() {
-		return this.#prev;
-	}
-
 	linked = true;
 
 	/** @type {Map<Effect, number>} */
 	stale_effects = new Map();
 
 	/** @type {Set<Effect>} */
-	effects_ran = new Set();
+	seen_effects = new Set();
 
 	/** @type {Batch | null} */
-	#prev = null;
+	prev = null;
 
 	/** @type {Batch | null} */
-	#next = null;
+	next = null;
 
 	/** @type {Set<Batch>} */
 	dependent = new Set();
@@ -270,8 +261,30 @@ export class Batch {
 		if (last_batch === null) {
 			first_batch = last_batch = this;
 		} else {
-			last_batch.#next = this;
+			// Put the new batch before the first forked batch
-			this.#prev = last_batch;
+			let batch = first_batch;
+			while (batch && !batch.is_fork) {
+				batch = batch.next;
+			}
+
+			if (batch) {
+				const prev = batch.prev;
+				this.next = batch;
+				batch.prev = this;
+				if (prev) {
+					prev.next = this;
+					this.prev = prev;
+				} else {
+					first_batch = this;
+				}
+				while (batch) {
+					batch.id = uid++;
+					batch = batch.next;
+				}
+			} else {
+				last_batch.next = this;
+				this.prev = last_batch;
+			}
 		}
 
 		last_batch = this;
@@ -587,10 +600,12 @@ export class Batch {
 					effects.push(effect);
 				} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
 					render_effects.push(effect);
-				} else if (is_dirty(effect)) {
+				} else {
-					this.effects_ran.add(effect);
+					this.seen_effects.add(effect);
-					if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect);
+					if (is_dirty(effect)) {
-					update_effect(effect);
+						// if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect);
+						update_effect(effect);
+					}
 				}
 
 				var child = effect.first;
@@ -617,7 +632,7 @@ export class Batch {
 	#find_earlier_batch() {
 		if (this.is_eager) return null;
 
-		var batch = this.#prev;
+		var batch = this.prev;
 
 		while (batch !== null) {
 			if (!batch.is_fork) {
@@ -635,7 +650,7 @@ export class Batch {
 				}
 			}
 
-			batch = batch.#prev;
+			batch = batch.prev;
 		}
 
 		return null;
@@ -663,8 +678,16 @@ export class Batch {
 			var flags = reaction.f;
 
 			if ((flags & DERIVED) !== 0) {
-				if (this.mark(/** @type {Derived} */ (reaction), MAYBE_DIRTY, not_yet)) {
+				var derived = /** @type {Derived} */ (reaction);
-					set_signal_status(/** @type {Derived} */ (reaction), status);
+
+				// if (
+				// 	this.current.has(derived) &&
+				// 	(/** @type {any} */ (this.current.get(derived))[0]) === derived.v
+				// ) {
+
+				// }
+				if (this.mark(derived, MAYBE_DIRTY, not_yet)) {
+					set_signal_status(derived, status);
 					marked = true;
 				}
 			} else {
@@ -672,10 +695,10 @@ export class Batch {
 
 				if (
 					not_yet
-						? !this.effects_ran.has(effect) &&
+						? !this.seen_effects.has(effect) &&
 							!this.#dirty_effects.has(effect) &&
 							!this.#maybe_dirty_effects.has(effect)
-						: flags & (ASYNC | BLOCK_EFFECT) && this.effects_ran.has(effect)
+						: flags & (ASYNC | BLOCK_EFFECT) && this.seen_effects.has(effect)
 				) {
 					this.#maybe_dirty_effects.delete(effect);
 					set_signal_status(effect, status);
@@ -831,7 +854,7 @@ export class Batch {
 			wv_values?.set(source, wv);
 		}
 
-		let batch = this.#next;
+		let batch = this.next;
 		let is_latest_value = !this.is_fork;
 		while (batch) {
 			if (source.f & ASYNC) {
@@ -850,22 +873,23 @@ export class Batch {
 				}
 			}
 			if (
-				!is_latest_value ||
+				!batch.is_fork &&
-				batch.current.has(source) ||
+				(!is_latest_value ||
-				// Check derived's dependencies for outdated values. We only have to check one
+					batch.current.has(source) ||
-				// level because is_dirty etc will execute the top-most deriveds first, whose result
+					// Check derived's dependencies for outdated values. We only have to check one
-				// the later deriveds can use to make a decision ("oh this derived's value is different to what I cached")
+					// level because is_dirty etc will execute the top-most deriveds first, whose result
-				((source.f & DERIVED) !== 0 &&
+					// the later deriveds can use to make a decision ("oh this derived's value is different to what I cached")
-					/** @type {Derived} */ (source).deps?.some(
+					((source.f & DERIVED) !== 0 &&
-						(d) =>
+						/** @type {Derived} */ (source).deps?.some(
-							/** @type {Batch} */ (batch).current.has(d) ||
+							(d) =>
-							(this.current.has(d) &&
+								/** @type {Batch} */ (batch).current.has(d) ||
-								/** @type {[any, boolean, number]} */ (this.current.get(d))[0] !== d.v)
+								(this.current.has(d) &&
-					))
+									/** @type {[any, boolean, number]} */ (this.current.get(d))[0] !== d.v)
+						)))
 			) {
 				is_latest_value = false;
 			}
-			batch = batch.#next;
+			batch = batch.next;
 		}
 
 		if (is_latest_value) {
@@ -878,29 +902,66 @@ export class Batch {
 			if (batch.id < this.id && batch.current.has(source)) {
 				this.dependent.add(batch);
 			}
-			if (
+
-				batch.is_fork &&
+			if (batch.is_fork && is_latest_value) {
-				is_latest_value &&
+				const current = batch.current.get(source);
-				((!batch.current.has(source) && !is_derived) ||
+				batch.current.delete(source);
-					/** @type {[any, boolean, number]} */ (batch.current.get(source))[0] !== value) &&
+
-				((source.f & ASYNC) === 0 ||
+				if (![...batch.current.values()].some((v) => !v[1])) {
-					!depends_on(
+					batch.discard();
-						source.e,
+				} else {
-						[...batch.current.keys()].filter((s) => !this.current.has(s)),
+					if (current) batch.current.set(source, current);
-						new Map()
+					if (
-					))
+						!is_derived &&
-			) {
+						(!current || current[0] !== value) &&
-				batch.current.set(source, [value, is_derived, wv]);
+						((source.f & ASYNC) === 0 ||
-				const b = batch;
+							!depends_on(
-				queue_micro_task(() => {
+								source.e,
-					if (b.mark(source, DIRTY)) {
+								[...batch.current.keys()].filter((s) => !this.current.has(s)),
-						b.flush();
+								new Map()
+							))
+					) {
+						batch.current.delete(source);
+						const b = batch;
+						queue_micro_task(() => {
+							if (b.mark(source, DIRTY)) {
+								b.flush();
+							}
+						});
 					}
-				});
+				}
 			}
-			batch = batch.#next;
+			batch = batch.next;
 		}
 
+		// batch = first_batch;
+		// while (batch) {
+		// 	if (batch.id < this.id && batch.current.has(source)) {
+		// 		this.dependent.add(batch);
+		// 	}
+		// 	if (
+		// 		batch.is_fork &&
+		// 		is_latest_value &&
+		// 		((!batch.current.has(source) && !is_derived) ||
+		// 			/** @type {[any, boolean, number]} */ (batch.current.get(source))[0] !== value) &&
+		// 		((source.f & ASYNC) === 0 ||
+		// 			!depends_on(
+		// 				source.e,
+		// 				[...batch.current.keys()].filter((s) => !this.current.has(s)),
+		// 				new Map()
+		// 			))
+		// 	) {
+		// 		batch.current.set(source, [value, is_derived, wv]);
+		// 		const b = batch;
+		// 		queue_micro_task(() => {
+		// 			if (b.mark(source, DIRTY)) {
+		// 				b.flush();
+		// 			}
+		// 		});
+		// 	}
+		// 	batch = batch.#next;
+		// }
+
 		// if (!this.is_fork) {
 		// 	let is_latest_value = true;
 		// 	batch = this.#next;
@@ -982,7 +1043,7 @@ export class Batch {
 		// in other words, we re-run block/async effects with the newly
 		// committed state, unless the batch in question has a more
 		// recent value for a given source
-		for (let batch = first_batch; batch !== null; batch = batch.#next) {
+		for (let batch = first_batch; batch !== null; batch = batch.next) {
 			var is_earlier = batch.id < this.id;
 
 			/** @type {Source[]} */
@@ -1201,8 +1262,8 @@ export class Batch {
 		return current_batch;
 	}
 
-	apply(include_later = false) {
+	apply(include_earlier = false) {
-		if (!async_mode_flag || (!this.is_fork && this.#prev === null && this.#next === null)) {
+		if (!async_mode_flag || (!this.is_fork && this.prev === null && this.next === null)) {
 			batch_values = null;
 			wv_values = null;
 			return;
@@ -1218,7 +1279,7 @@ export class Batch {
 			wv_values.set(source, wv);
 		}
 
-		for (let batch = first_batch; batch !== null; batch = batch.#next) {
+		for (let batch = first_batch; batch !== null; batch = batch.next) {
 			if (batch === this) continue;
 
 			if (batch.id < this.id) {
@@ -1229,7 +1290,7 @@ export class Batch {
 
 			if (batch.is_fork) continue;
 
-			if (batch.id > this.id || include_later || this.is_eager) {
+			if (batch.id > this.id || include_earlier || this.is_eager) {
 				for (const [source, value] of batch.previous) {
 					if (!batch_values.has(source)) {
 						batch_values.set(source, value);
@@ -1241,7 +1302,7 @@ export class Batch {
 		return;
 
 		// ...and undo changes belonging to other batches unless they intersect
-		for (let batch = first_batch; batch !== null; batch = batch.#next) {
+		for (let batch = first_batch; batch !== null; batch = batch.next) {
 			if (batch === this || batch.is_fork) continue;
 
 			// If two batches intersect, the latter batch will be merged into the earlier batch,
@@ -1302,19 +1363,19 @@ export class Batch {
 		// running it multiple times to not corrupt the linked list
 		if (!this.linked) return;
 
-		var prev = this.#prev;
+		var prev = this.prev;
-		var next = this.#next;
+		var next = this.next;
 
 		if (prev === null) {
 			first_batch = next;
 		} else {
-			prev.#next = next;
+			prev.next = next;
 		}
 
 		if (next === null) {
 			last_batch = prev;
 		} else {
-			next.#prev = prev;
+			next.prev = prev;
 		}
 
 		this.linked = false;
@@ -1735,8 +1796,22 @@ export function fork(fn) {
 
 			batch.is_fork = false;
 
+			// Reorder the batches such that there's no forks before this one
+			while (batch.prev && batch.prev.is_fork) {
+				let prev = /** @type {Batch} */ (batch.prev);
+				const id = batch.id;
+				batch.id = prev.id;
+				prev.id = id;
+				prev.next = batch.next;
+				batch.prev = prev.prev;
+				if (prev.prev) {
+					prev.prev.next = batch;
+				}
+				prev.prev = batch;
+			}
+
 			// apply changes and update write versions so deriveds see the change
-			for (var [source, [value, is_derived, wv]] of batch.current) {
+			for (var [source, content] of batch.current) {
 				// TODO this if-block tries to accomodate the fact that this value might be obsoleted by a subsequent batch already;
 				// but has false positives (i.e. values not updated when they should). Needs a better mechanism
 				// maybe current has a fourth entry, "outdated" boolean, and later batches set it for earlier ones?
@@ -1745,12 +1820,15 @@ export function fork(fn) {
 				// TODO we need to ensure that the version bumps happen "in order", e.g. in case of source1->derived2 we need to bump S last
 				// }
 
-				if (!is_derived) {
+				source.v = content[0];
-					source.v = value;
+
-					source.wv = increment_write_version();
+				if (!content[1]) {
+					// TODO what about already-outdated async sources?
+					// is there a situation where they can appear here?
+					content[2] = source.wv = increment_write_version();
 					// batch.mark(source, ...) TODO re-maybe-dirty- everything?
 					// dirty those effects the fork did not see yet, e.g. because a later batch created new branches
-					batch.mark(source, DIRTY, true); // TODO probably better to only DIRTY on first non-seen derived
+					batch.mark(source, MAYBE_DIRTY, true);
 				}
 			}
 
@@ -1771,6 +1849,7 @@ export function fork(fn) {
 				flush_eager_effects();
 			});
 
+			// TODO reuse batch.capture() logic here (maybe we can just call it?)
 			let next_batch = batch.next;
 			while (next_batch) {
 				for (const [source, [, is_derived]] of batch.current) {

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

@@ -274,7 +274,6 @@ export function async_derived(fn, label, location) {
 					});
 				}
 
-				debugger;
 				internal_set(signal, value);
 			}
 

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

@@ -371,7 +371,7 @@ export function legacy_pre_effect_reset() {
  */
 export function async_effect(fn) {
 	const effect = create_effect(ASYNC | EFFECT_PRESERVED, fn);
-	current_batch?.effects_ran.add(effect);
+	current_batch?.seen_effects.add(effect);
 	return effect;
 }
 
@@ -419,7 +419,7 @@ export function block(fn, flags = 0) {
 	if (DEV) {
 		effect.dev_stack = dev_stack;
 	}
-	current_batch?.effects_ran.add(effect);
+	current_batch?.seen_effects.add(effect);
 	return effect;
 }
 

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

@@ -495,37 +495,47 @@ export function update_effect(effect) {
 		let is_latest_value = true;
 		// Can be falsy inside flush_eager_effects
 		if (own_batch) {
-			is_latest_value = !own_batch.is_fork;
+			is_latest_value =
-			var batch = own_batch.next;
+				!own_batch.is_fork &&
-			while (batch) {
+				(!effect.deps?.length ||
-				if (
+					!effect.deps.some((d) => {
-					batch.started && // when flushing user effects writes sources which creates a new batch, then ignore that one
+						return (
-					(!is_latest_value ||
+							batch_values &&
-						// Check derived's dependencies for outdated values. We only have to check one
+							batch_values.has(d) &&
-						// level because is_dirty etc will execute the top-most deriveds first, whose result
+							(!own_batch?.current.has(d) ||
-						// the later deriveds can use to make a decision ("oh this derived's value is different to what I cached")
+								/** @type {any} */ (own_batch.current.get(d))[0] !== d.v)
-						effect.deps?.some((d) => {
+						);
-							var other_current = /** @type {Batch} */ (batch).current;
+					}));
-							var own_current = /** @type {Batch} */ (own_batch).current;
+			// var batch = own_batch.next;
-							// const y =
+			// while (batch) {
-							// 	other_current.has(d) ||
+			// 	if (
-							// 	(own_current.has(d) &&
+			// 		batch.started && // when flushing user effects writes sources which creates a new batch, then ignore that one
-							// 		/** @type {[any, boolean, number]} */ (own_current.get(d))[0] !== d.v);
+			// 		(!is_latest_value ||
-							// if (y) debugger;
+			// 			// Check derived's dependencies for outdated values. We only have to check one
-							// const x =
+			// 			// level because is_dirty etc will execute the top-most deriveds first, whose result
-							// 	other_current.has(d) &&
+			// 			// the later deriveds can use to make a decision ("oh this derived's value is different to what I cached")
-							// 	(!own_current.has(d) ||
+			// 			effect.deps?.some((d) => {
-							// 		/** @type {[any, boolean, number]} */ (own_current.get(d))[0] !==
+			// 				var other_current = /** @type {Batch} */ (batch).current;
-							// 			/** @type {[any, boolean, number]} */ (other_current.get(d))[0]);
+			// 				var own_current = /** @type {Batch} */ (own_batch).current;
-							// if (x) debugger;
+			// 				// const y =
-							const z = (own_current.get(d)?.[2] ?? d.wv) != d.wv;
+			// 				// 	other_current.has(d) ||
-							return z;
+			// 				// 	(own_current.has(d) &&
-						}))
+			// 				// 		/** @type {[any, boolean, number]} */ (own_current.get(d))[0] !== d.v);
-				) {
+			// 				// if (y) debugger;
-					is_latest_value = false;
+			// 				// const x =
-				}
+			// 				// 	other_current.has(d) &&
-				batch = batch.next;
+			// 				// 	(!own_current.has(d) ||
-			}
+			// 				// 		/** @type {[any, boolean, number]} */ (own_current.get(d))[0] !==
+			// 				// 			/** @type {[any, boolean, number]} */ (other_current.get(d))[0]);
+			// 				// if (x) debugger;
+			// 				const z = (own_current.get(d)?.[2] ?? d.wv) != d.wv;
+			// 				return z;
+			// 			}))
+			// 	) {
+			// 		is_latest_value = false;
+			// 	}
+			// 	batch = batch.next;
+			// }
 		}
 
 		if (is_latest_value) {

packages/svelte/tests/runtime-runes/samples/async-derived-stale-read/_config.js

@@ -0,0 +1,22 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		await tick();
+
+		const [a, b, log, resolve] = target.querySelectorAll('button');
+
+		a.click();
+		await tick();
+		b.click();
+		await tick();
+		resolve.click();
+		await tick();
+		assert.htmlEqual(target.querySelector('p')?.innerHTML ?? '', 'd is 1');
+
+		resolve.click();
+		await tick();
+		assert.equal(target.querySelector('p'), null);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-derived-stale-read/main.svelte

@@ -0,0 +1,27 @@
+<script>
+	let a = $state(0);
+	let b = $state(0);
+
+	let d = $derived(a + b);
+
+	let queued = [];
+	function push(v) {
+		if (!v) return v;
+		return new Promise((resolve) => {
+			queued.push(() => resolve(v));
+		});
+	}
+
+	$effect(() => console.log(d));
+</script>
+
+<button onclick={() => a++}>a++</button>
+<button onclick={() => b++}>b++</button>
+<button onclick={() => console.log(d)}>log d</button>
+<button onclick={() => queued.shift()?.()}>resolve</button>
+
+{#if await push(a + b)}
+	{#if d === 1}
+		<p>d is {d}</p>
+	{/if}
+{/if}

packages/svelte/tests/runtime-runes/samples/async-fork-effect-overlap-1/_config.js

@@ -0,0 +1,38 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		await tick();
+		const [x, y, shift, pop, commit] = target.querySelectorAll('button');
+		const [p] = target.querySelectorAll('p');
+		logs.length = 0;
+
+		x.click();
+		await tick();
+		assert.deepEqual(logs, ['called with 1,0']);
+		logs.length = 0;
+
+		y.click();
+		await tick();
+		assert.deepEqual(logs, ['called with 0,1', 'called with 1,1']); // if 'called with 1,1' happens a few button clicks later that would also be ok
+		assert.htmlEqual(p.innerHTML, '0');
+		logs.length = 0;
+
+		pop.click(); // the rerunning fork
+		await tick();
+		pop.click(); // the first real world run
+		await tick();
+		assert.deepEqual(logs, []);
+		assert.htmlEqual(p.innerHTML, '1');
+		logs.length = 0;
+
+		commit.click();
+		await tick();
+		assert.deepEqual(logs, []);
+		assert.htmlEqual(p.innerHTML, '2');
+
+		pop.click();
+		await tick();
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-fork-effect-overlap-1/main.svelte

@@ -0,0 +1,23 @@
+<script>
+	import { fork } from 'svelte';
+
+	let x = $state(0);
+	let y = $state(0);
+	let f;
+
+	const deferred = [];
+
+	function delay(_, value) {
+		if (!value) return value;
+		return new Promise((resolve) => deferred.push(() => resolve(value)));
+	}
+ 
+</script>
+
+<button onclick={() => {f = fork(() => x++)}}>x</button>
+<button onclick={() => y++}>y</button>
+<button onclick={() => deferred.shift()?.()}>shift</button>
+<button onclick={() => deferred.pop()?.()}>pop</button>
+<button onclick={() => f.commit()}>commit</button>
+
+<p>{await delay(console.log('called with ' + x + ',' + y), x + y)}</p>

packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/_config.js

@@ -0,0 +1,18 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		const [change, commit] = target.querySelectorAll('button');
+
+		assert.deepEqual(logs, ['hi']);
+
+		change.click();
+		await tick();
+		assert.deepEqual(logs, ['hi', 'hi']);
+
+		commit.click();
+		await tick();
+		assert.deepEqual(logs, ['hi', 'hi']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { fork } from 'svelte';
+
+	let x = $state(1);
+	let y = $derived.by(() => {console.log('hi'); return x % 2});
+	let f;
+</script>
+
+<button onclick={() => { f = fork(() => x = 3)}}>change</button>
+<button onclick={() => f.commit()}>commit</button>
+
+{#if y}hi{/if}

packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-1/_config.js

@@ -22,7 +22,7 @@ export default test({
 		`
 		);
 
-		commit.click();
+		commit.click(); // puts fork (x) behind y so it has to wait on y first
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
@@ -36,7 +36,7 @@ export default test({
 		`
 		);
 
-		shift.click();
+		shift.click(); // ... which is why nothing happens yet on first shift
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
@@ -46,13 +46,6 @@ export default test({
 			<button>shift</button>
 			<button>pop</button>
 			<button>commit</button>
-			universe
-			universe
-			"universe"
-			universe
-			universe
-			universe
-			"universe"
 			<hr>
 		`
 		);