more

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

@@ -117,6 +117,9 @@ export class Batch {
 
 	linked = true;
 
+	/** @type {Set<Effect>} */
+	effects_ran = new Set();
+
 	/** @type {Batch | null} */
 	#prev = null;
 
@@ -144,6 +147,14 @@ export class Batch {
 	 */
 	previous = new Map();
 
+	/**
+	 * TODO run this on fork commit, put in all the effects we have decided we need to rerun,
+	 * it's a map so that we can delete entries if later batches have runs in it; though
+	 * how do we know that this no longer counts for batch 3 but still for batch 2?
+	 * @type {Map<Effect, () => void>}
+	 */
+	on_fork_commit = new Map();
+
 	/**
 	 * When the batch is committed (and the DOM is updated), we need to remove old branches
 	 * and append new ones by calling the functions added inside (if/each/key/etc) blocks
@@ -530,6 +541,7 @@ export class Batch {
 				} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
 					render_effects.push(effect);
 				} else if (is_dirty(effect)) {
+					this.effects_ran.add(effect);
 					if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect);
 					update_effect(effect);
 				}
@@ -582,6 +594,53 @@ export class Batch {
 		return null;
 	}
 
+	/**
+	 * Mark all reactive trees leading to block/async effects that (indirectly) depend on `value`
+	 * @param {Value} value
+	 * @param {number} status
+	 * @param {boolean} not_yet - whether to mark effects that have not yet run, as opposed to those that have already run
+	 */
+	mark(value, status, not_yet = false) {
+		var reactions = value.reactions;
+		if (reactions === null) return false;
+		// skip if value is derived and is neither dirty nor maybe dirty. transitive
+		// deriveds (a derived depending on another derived) are only MAYBE_DIRTY, so
+		// we must continue traversing them to reach the effects that depend on them
+		// if ((value.f & DERIVED) !== 0 && (value.f & (DIRTY | MAYBE_DIRTY)) === 0) {
+		// 	return;
+		// }
+
+		let marked = false;
+
+		for (const reaction of reactions) {
+			var flags = reaction.f;
+
+			if ((flags & DERIVED) !== 0) {
+				if (this.mark(/** @type {Derived} */ (reaction), MAYBE_DIRTY, not_yet)) {
+					set_signal_status(/** @type {Derived} */ (reaction), status);
+					marked = true;
+				}
+			} else {
+				var effect = /** @type {Effect} */ (reaction);
+
+				if (
+					not_yet
+						? !this.effects_ran.has(effect) &&
+							!this.#dirty_effects.has(effect) &&
+							!this.#maybe_dirty_effects.has(effect)
+						: flags & (ASYNC | BLOCK_EFFECT) && this.effects_ran.has(effect)
+				) {
+					this.#maybe_dirty_effects.delete(effect);
+					set_signal_status(effect, status);
+					this.schedule(effect);
+					marked = true;
+				}
+			}
+		}
+
+		return marked;
+	}
+
 	/**
 	 * @param {Batch} batch
 	 */
@@ -591,6 +650,9 @@ export class Batch {
 				this.previous.set(source, batch.previous.get(source));
 			}
 
+			if (this.current.get(source)?.[0] !== value[0]) {
+				// this.mark(source, DIRTY);
+			}
 			this.current.set(source, value);
 		}
 
@@ -599,6 +661,31 @@ export class Batch {
 			if (d) deferred.promise.then(d.resolve).catch(d.reject);
 		}
 
+		for (const c of batch.#commit_callbacks) {
+			this.oncommit(() => c(batch));
+		}
+
+		for (const c of batch.#discard_callbacks) {
+			this.ondiscard(() => c(batch));
+		}
+
+		for (const [s, v] of batch.#skipped_branches) {
+			this.#skipped_branches.set(s, v);
+			this.#unskipped_branches.delete(s);
+		}
+
+		for (const s of batch.#unskipped_branches) {
+			const v = this.#skipped_branches.get(s);
+			// TODO i do wonder at this point if it's less code / easier / more robust to do what mark() below does
+			// instead and just rerun all the block effects. Though it will certainly overrun some blocks, potentially
+			// with bad consequences for e.g. each blocks (could generate a new array etc etc).
+			if (v) {
+				v.d = v.d.filter((e) => !batch.async_deriveds.has(e));
+				v.m = v.m.filter((e) => !batch.async_deriveds.has(e));
+			}
+			this.unskip_effect(s);
+		}
+
 		// Clear them or else those that are still pending might get rejected on discard (after merged-into batch is done).
 		// This can happen when batch Y merged into X and Y has a pending boundary and therefore still-pending async deriveds inside.
 		batch.async_deriveds.clear();
@@ -633,7 +720,9 @@ export class Batch {
 					var effect = /** @type {Effect} */ (reaction);
 
 					// TODO this overfires e.g. for async-state-new-branch-fork-5 where it reruns
-					// the Child async effects with "world" after commit+first resolve.
+					// the Child async effects with "world" after commit+first resolve; generally
+					// overfires everywhere where the merged effect has async deriveds that are not
+					// in the earlier batch, which it definitely should not rerun.
 					if (flags & (ASYNC | BLOCK_EFFECT) && !this.async_deriveds.has(effect)) {
 						this.#maybe_dirty_effects.delete(effect);
 						set_signal_status(effect, DIRTY);
@@ -643,9 +732,9 @@ export class Batch {
 			}
 		};
 
-		for (const source of this.current.keys()) {
+		// for (const source of this.current.keys()) {
-			mark(source);
+		// 	mark(source);
-		}
+		// }
 
 		this.oncommit(() => batch.discard());
 		batch.#unlink();
@@ -690,18 +779,41 @@ export class Batch {
 			batch = batch.#prev;
 		}
 
-		if (!this.is_fork) {
+		let is_latest_value = true;
-			let is_latest_value = true;
+		batch = this.#next;
-			batch = this.#next;
+		while (batch) {
-			while (batch) {
+			if (source.f & ASYNC) {
-				if (batch.current.has(source)) {
+				const b = batch;
-					is_latest_value = false;
+				const run = () => {
-					break;
+					if (b.mark(source, DIRTY)) {
+						b.flush();
+					}
+				};
+				if (this.is_fork) {
+					// this.on_fork_commit.set({}, run); // TODO
+				} else {
+					queue_micro_task(run);
 				}
-				batch = batch.#next;
 			}
-			if (is_latest_value) source.v = value;
+			if (batch.current.has(source)) {
+				is_latest_value = false;
+			}
+			batch = batch.#next;
 		}
+		if (is_latest_value && !this.is_fork) source.v = value;
+
+		// if (!this.is_fork) {
+		// 	let is_latest_value = true;
+		// 	batch = this.#next;
+		// 	while (batch) {
+		// 		if (batch.current.has(source)) {
+		// 			is_latest_value = false;
+		// 			break;
+		// 		}
+		// 		batch = batch.#next;
+		// 	}
+		// 	if (is_latest_value) source.v = value;
+		// }
 	}
 
 	activate() {
@@ -1505,6 +1617,8 @@ export function fork(fn) {
 			for (var [source, [value]] of batch.current) {
 				source.v = value;
 				source.wv = increment_write_version();
+				// 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
 			}
 
 			// trigger any `$state.eager(...)` expressions with the new state.
@@ -1524,6 +1638,18 @@ export function fork(fn) {
 				flush_eager_effects();
 			});
 
+			let next_batch = batch.next;
+			while (next_batch) {
+				for (const [source, [, is_derived]] of batch.current) {
+					if (!is_derived && !next_batch.current.has(source)) {
+						if (next_batch.mark(source, DIRTY)) {
+							next_batch.flush();
+						}
+					}
+				}
+				next_batch = next_batch.next;
+			}
+
 			// let next_batch = batch.next;
 			// while (next_batch) {
 			// 	for (const [effect] of batch.async_deriveds) {
@@ -1541,6 +1667,10 @@ export function fork(fn) {
 			// 	next_batch = next_batch.next;
 			// }
 
+			// for (const run of batch.on_fork_commit.values()) {
+			// 	run();
+			// }
+
 			batch.flush();
 			await settled;
 		},

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

@@ -181,21 +181,21 @@ export function async_derived(fn, label, location) {
 
 		var batch = /** @type {Batch} */ (current_batch);
 
-		let next_batch = batch.next;
+		// let next_batch = batch.next;
-		while (next_batch) {
+		// while (next_batch) {
-			if (next_batch.async_deriveds.has(effect)) {
+		// 	if (next_batch.async_deriveds.has(effect)) {
-				next_batch.dependent.add(batch);
+		// 		next_batch.dependent.add(batch);
-				if (!next_batch.is_fork) {
+		// 		if (!next_batch.is_fork) {
-					set_signal_status(effect, DIRTY); // TODO ideally we can find out if we really need to rerun or if all dependencies' values are equal
+		// 			set_signal_status(effect, DIRTY); // TODO ideally we can find out if we really need to rerun or if all dependencies' values are equal
-					// TODO same for block effects; ideally one mechanism for both
+		// 			// TODO same for block effects; ideally one mechanism for both
-					next_batch.schedule(effect);
+		// 			next_batch.schedule(effect);
-					const b = next_batch;
+		// 			const b = next_batch;
-					queue_micro_task(() => b.flush());
+		// 			queue_micro_task(() => b.flush());
-				}
+		// 		}
-				break; // TODO break correct? Don't we need to do the rerun for all of them?
+		// 		break; // TODO break correct? Don't we need to do the rerun for all of them?
-			}
+		// 	}
-			next_batch = next_batch.next;
+		// 	next_batch = next_batch.next;
-		}
+		// }
 
 		let prev = batch.prev;
 		while (prev) {
@@ -283,11 +283,9 @@ export function async_derived(fn, label, location) {
 		}
 	});
 
-	if (DEV) {
+	// Besides prod-logic this also helps in DEV to let this be printed
-		// add a flag that lets this be printed as a derived
+	// as a derived when using `$inspect.trace()`
-		// when using `$inspect.trace()`
+	signal.f |= ASYNC;
-		signal.f |= ASYNC;
-	}
 
 	return new Promise((fulfil) => {
 		/** @param {Promise<V>} p */

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

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

packages/svelte/tests/runtime-runes/samples/async-merge-no-unnecessary-rerun/_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 [a, b, pop] = target.querySelectorAll('button');
+
+		a.click();
+		await tick();
+		assert.deepEqual(logs, ['a+b 1']);
+		logs.length = 0;
+
+		b.click();
+		await tick();
+		assert.deepEqual(logs, ['a+b 2', 'b 1']);
+		logs.length = 0;
+
+		pop.click();
+		await tick();
+		assert.deepEqual(logs, []);
+
+		pop.click();
+		await tick();
+		// At this point the two batches are merged, and no async effects should be rerun
+		assert.deepEqual(logs, []);
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a</button>
+			<button>b</button>
+			<button>pop</button>
+			1 + 1 = 2 | 1
+		`
+		);
+	}
+});

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

@@ -0,0 +1,18 @@
+<script>
+	let a = $state(0);
+	let b = $state(0);
+
+	const queue = [];
+
+	function push(from, v) {
+		if (!v) return v;
+		console.log(from + ' ' + v);
+		return new Promise(r => queue.push(() => r(v)));
+	}
+</script>
+
+<button onclick={() => a++}>a</button>
+<button onclick={() => b++}>b</button>
+<button onclick={() => queue.pop()?.()}>pop</button>
+
+{a} + {b} = {await push('a+b', a + b)} | {await push('b', b)}

packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-2/_config.js

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
 	async test({ assert, target }) {
 		await tick();
 		const [a_b, a_c, b_d, shift, pop] = target.querySelectorAll('button');

packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-3/_config.js

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
 	async test({ assert, target }) {
 		await tick();
 		const [a_b, a_c, b_d, shift, pop] = target.querySelectorAll('button');
@@ -63,12 +62,12 @@ export default test({
 		`
 		);
 
-		pop.click(); // second b resolved, still pending: [b, a]
+		pop.click(); // second b resolved, still pending: [b, a] but b is now obsolete
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
 			`
-			a 0 | b 0 | c 0 | d 0
+			a 1 | b 2 | c 0 | d 1
 			<button>a and b</button>
 			<button>a and c</button>
 			<button>b and d</button>
@@ -79,18 +78,6 @@ export default test({
 
 		shift.click(); // first b resolved, first + last batch settled, still pending: [a]
 		await tick();
-		assert.htmlEqual(
-			target.innerHTML,
-			`
-			a 1 | b 2 | c 0 | d 1
-			<button>a and b</button>
-			<button>a and c</button>
-			<button>b and d</button>
-			<button>shift</button>
-			<button>pop</button>
-		`
-		);
-
 		shift.click(); // all resolved
 		await tick();
 		assert.htmlEqual(

packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-4/_config.js

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
 	async test({ assert, target }) {
 		await tick();
 		const [a_b, a_c, b_d, shift, pop] = target.querySelectorAll('button');
@@ -63,12 +62,12 @@ export default test({
 		`
 		);
 
-		pop.click(); // second b resolved, still pending: [b, a]
+		pop.click(); // second b resolved, still pending: [b, a] with b obsolete so can render some already
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
 			`
-			a 0 | b 0 | c 0 | d 0
+			a 1 | b 2 | c 0 | d 1
 			<button>a and b</button>
 			<button>a and c</button>
 			<button>b and d</button>
@@ -78,22 +77,6 @@ export default test({
 		);
 
 		pop.click(); // second a resolved, first a/b now obsolete
-		// TODO would be nice to show final result here already, right now it doesn't because
-		// we have no handle on the already resolved first a anymore
-		await tick();
-		assert.htmlEqual(
-			target.innerHTML,
-			`
-			a 0 | b 0 | c 0 | d 0
-			<button>a and b</button>
-			<button>a and c</button>
-			<button>b and d</button>
-			<button>shift</button>
-			<button>pop</button>
-		`
-		);
-
-		shift.click(); // queue empty
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,

packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-7/_config.js

@@ -32,7 +32,7 @@ export default test({
 		`
 		);
 
-		shift.click(); // schedules second step of first batch and schedules rerun of second batch
+		shift.click(); // schedules second step of first batch and schedules rerun of second batch which can use updated values
 		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
@@ -45,36 +45,8 @@ export default test({
 		`
 		);
 
-		// how it's on main
-
 		pop.click();
 		await tick();
-		assert.htmlEqual(
-			target.innerHTML,
-			`
-			a 1 | b 2 | c 0 | d 2
-			<button>a++</button>
-			<button>c++</button>
-			<button>shift</button>
-			<button>pop</button>
-		`
-		);
-
-		shift.click(); // obsolete second batch promise (already rejected)
-		await tick();
-		assert.htmlEqual(
-			target.innerHTML,
-			`
-			a 1 | b 2 | c 0 | d 2
-			<button>a++</button>
-			<button>c++</button>
-			<button>shift</button>
-			<button>pop</button>
-		`
-		);
-
-		shift.click(); // first batch resolves
-		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
 			`
@@ -85,45 +57,5 @@ export default test({
 			<button>pop</button>
 		`
 		);
-
-		// how it's on https://github.com/sveltejs/svelte/pull/17971
-		// pop.click(); // second batch resolves but knows it needs to wait on first batch
-		// await tick();
-		// assert.htmlEqual(
-		// 	target.innerHTML,
-		// 	`
-		// 	a 0 | b 0 | c 0 | d 0
-		// 	<button>a++</button>
-		// 	<button>c++</button>
-		// 	<button>shift</button>
-		// 	<button>pop</button>
-		// `
-		// );
-
-		// shift.click(); // obsolete second batch promise (already rejected)
-		// await tick();
-		// assert.htmlEqual(
-		// 	target.innerHTML,
-		// 	`
-		// 	a 0 | b 0 | c 0 | d 0
-		// 	<button>a++</button>
-		// 	<button>c++</button>
-		// 	<button>shift</button>
-		// 	<button>pop</button>
-		// `
-		// );
-
-		// shift.click(); // first batch resolves, with it second can now resolve as well
-		// await tick();
-		// assert.htmlEqual(
-		// 	target.innerHTML,
-		// 	`
-		// 	a 1 | b 2 | c 1 | d 3
-		// 	<button>a++</button>
-		// 	<button>c++</button>
-		// 	<button>shift</button>
-		// 	<button>pop</button>
-		// `
-		// );
 	}
 });

packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-fork-3/_config.js

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true,
 	async test({ assert, target }) {
 		await tick();
 		const [a_b_fork, a_c, b_d, shift, pop, commit] = target.querySelectorAll('button');

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

@@ -2,8 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO this one is tricky
-
 	async test({ assert, target }) {
 		const [increment, a, b] = target.querySelectorAll('button');
 
@@ -31,27 +29,27 @@ export default test({
 
 		a.click();
 		await tick();
-
 		assert.htmlEqual(
 			target.innerHTML,
 			`
 				<button>increment</button>
 				<button>shift a</button>
 				<button>shift b</button>
-				<p>a: 0</p>
+				<p>a: 2</p>
 			`
 		);
 
 		b.click();
 		await tick();
-
+		b.click();
+		await tick();
 		assert.htmlEqual(
 			target.innerHTML,
 			`
 				<button>increment</button>
 				<button>shift a</button>
 				<button>shift b</button>
-				<p>b: 0</p
+				<p>b: 3</p
 			`
 		);
 	}

packages/svelte/tests/runtime-runes/samples/async-state-block-overlap/_config.js

@@ -0,0 +1,55 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+
+		const [a, b, _, pop] = target.querySelectorAll('button');
+
+		a.click();
+		await tick();
+
+		b.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a</button>
+			<button>b</button>
+			<button>shift</button>
+			<button>pop</button>
+			0 0 bye
+		`
+		);
+
+		pop.click();
+		await tick();
+		// The later batch finishes first but should not commit; it overlaps with the earlier batch through a block.
+		// If we were to commit this batch we would briefly see an if block's truthy branch, which is wrong considering
+		// the "timeline applied in order" principle.
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a</button>
+			<button>b</button>
+			<button>shift</button>
+			<button>pop</button>
+			0 0 bye
+		`
+		);
+
+		pop.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a</button>
+			<button>b</button>
+			<button>shift</button>
+			<button>pop</button>
+			1 1 bye
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-state-block-overlap/main.svelte

@@ -0,0 +1,25 @@
+<script>
+	let a = $state(0);
+	let b = $state(0);
+
+	const queue = [];
+
+	function push(v) {
+		if (!v) return v;
+		return new Promise(r => queue.push(() => r(v)));
+	}
+</script>
+
+<button onclick={() => a++}>a</button>
+<button onclick={() => b++}>b</button>
+<button onclick={() => queue.shift()?.()}>shift</button>
+<button onclick={() => queue.pop()?.()}>pop</button>
+
+{await push(a)}
+{await push(b)}
+
+{#if a < b}
+	hello
+{:else}
+	bye
+{/if}

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

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
 	async test({ assert, target }) {
 		const [x, y, shift, pop, commit] = target.querySelectorAll('button');
 

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

@@ -2,7 +2,6 @@ import { tick } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
 	async test({ assert, target }) {
 		const [x, y, resolve, commit] = target.querySelectorAll('button');