Merge branch 'main' into each-block-pending

.changeset/upset-parts-throw.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: rebase batches after process, not during

packages/svelte/CHANGELOG.md

@@ -1,5 +1,11 @@
 # svelte
 
+## 5.53.10
+
+### Patch Changes
+
+- fix: re-process batch if new root effects were scheduled ([#17895](https://github.com/sveltejs/svelte/pull/17895))
+
 ## 5.53.9
 
 ### Patch Changes

packages/svelte/package.json

@@ -2,7 +2,7 @@
   "name": "svelte",
   "description": "Cybernetically enhanced web apps",
   "license": "MIT",
-  "version": "5.53.9",
+  "version": "5.53.10",
   "type": "module",
   "types": "./types/index.d.ts",
   "engines": {

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

@@ -249,6 +249,10 @@ export class Batch {
 				reset_branch(e, t);
 			}
 		} else {
+			if (this.#pending === 0) {
+				batches.delete(this);
+			}
+
 			// clear effects. Those that are still needed will be rescheduled through unskipping the skipped branches.
 			this.#dirty_effects.clear();
 			this.#maybe_dirty_effects.clear();
@@ -262,15 +266,19 @@ export class Batch {
 			flush_queued_effects(effects);
 			previous_batch = null;
 
-			if (this.#pending === 0) {
-				this.#commit();
-			}
-
 			this.#deferred?.resolve();
 		}
 
 		var next_batch = /** @type {Batch | null} */ (/** @type {unknown} */ (current_batch));
 
+		// Edge case: During traversal new branches might create effects that run immediately and set state,
+		// causing an effect and therefore a root to be scheduled again. We need to traverse the current batch
+		// once more in that case - most of the time this will just clean up dirty branches.
+		if (this.#roots.length > 0) {
+			const batch = (next_batch ??= this);
+			batch.#roots.push(...this.#roots.filter((r) => !batch.#roots.includes(r)));
+		}
+
 		if (next_batch !== null) {
 			batches.add(next_batch);
 
@@ -282,6 +290,10 @@ export class Batch {
 
 			next_batch.#process();
 		}
+
+		if (!batches.has(this)) {
+			this.#commit();
+		}
 	}
 
 	/**
@@ -425,74 +437,59 @@ 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
-		if (batches.size > 1) {
-			this.previous.clear();
-
-			var previous_batch = current_batch;
-			var previous_batch_values = batch_values;
-			var is_earlier = true;
-
-			for (const batch of batches) {
-				if (batch === this) {
-					is_earlier = false;
-					continue;
+		for (const batch of batches) {
+			var is_earlier = batch.id < this.id;
+
+			/** @type {Source[]} */
+			var sources = [];
+
+			for (const [source, value] of this.current) {
+				if (batch.current.has(source)) {
+					if (is_earlier && value !== batch.current.get(source)) {
+						// bring the value up to date
+						batch.current.set(source, value);
+					} else {
+						// same value or later batch has more recent value,
+						// no need to re-run these effects
+						continue;
+					}
 				}
 
-				/** @type {Source[]} */
-				const sources = [];
-
-				for (const [source, value] of this.current) {
-					if (batch.current.has(source)) {
-						if (is_earlier && value !== batch.current.get(source)) {
-							// bring the value up to date
-							batch.current.set(source, value);
-						} else {
-							// same value or later batch has more recent value,
-							// no need to re-run these effects
-							continue;
-						}
-					}
+				sources.push(source);
+			}
 
-					sources.push(source);
-				}
+			if (sources.length === 0) {
+				continue;
+			}
 
-				if (sources.length === 0) {
-					continue;
-				}
+			// Re-run async/block effects that depend on distinct values changed in both batches
+			var others = [...batch.current.keys()].filter((s) => !this.current.has(s));
+			if (others.length > 0) {
+				batch.activate();
 
-				// Re-run async/block effects that depend on distinct values changed in both batches
-				const others = [...batch.current.keys()].filter((s) => !this.current.has(s));
-				if (others.length > 0) {
-					batch.activate();
-
-					/** @type {Set<Value>} */
-					const marked = new Set();
-					/** @type {Map<Reaction, boolean>} */
-					const checked = new Map();
-					for (const source of sources) {
-						mark_effects(source, others, marked, checked);
-					}
+				/** @type {Set<Value>} */
+				var marked = new Set();
 
-					if (batch.#roots.length > 0) {
-						batch.apply();
+				/** @type {Map<Reaction, boolean>} */
+				var checked = new Map();
 
-						for (const root of batch.#roots) {
-							batch.#traverse(root, [], []);
-						}
+				for (var source of sources) {
+					mark_effects(source, others, marked, checked);
+				}
+
+				if (batch.#roots.length > 0) {
+					batch.apply();
 
-						// TODO do we need to do anything with the dummy effect arrays?
+					for (var root of batch.#roots) {
+						batch.#traverse(root, [], []);
 					}
 
-					batch.deactivate();
+					// TODO do we need to do anything with the dummy effect arrays?
 				}
-			}
 
-			current_batch = previous_batch;
-			batch_values = previous_batch_values;
+				batch.deactivate();
+			}
 		}
-
-		this.#skipped_branches.clear();
-		batches.delete(this);
 	}
 
 	/**
@@ -559,7 +556,10 @@ export class Batch {
 	}
 
 	apply() {
-		if (!async_mode_flag || (!this.is_fork && batches.size === 1)) return;
+		if (!async_mode_flag || (!this.is_fork && batches.size === 1)) {
+			batch_values = null;
+			return;
+		}
 
 		// if there are multiple batches, we are 'time travelling' —
 		// we need to override values with the ones in this batch...

packages/svelte/src/version.js

@@ -4,5 +4,5 @@
  * The current version, as set in package.json.
  * @type {string}
  */
-export const VERSION = '5.53.9';
+export const VERSION = '5.53.10';
 export const PUBLIC_VERSION = '5';

packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/_config.js

@@ -0,0 +1,46 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		const [open, close, increment] = target.querySelectorAll('button');
+
+		open.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>0</button>
+				<div>open (width: 42)</div>
+			`
+		);
+
+		increment.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>1</button>
+				<div>open (width: 42)</div>
+			`
+		);
+
+		close.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>1</button>
+				<div>closed</div>
+			`
+		);
+
+		assert.deepEqual(logs, ['effect ran']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/main.svelte

@@ -0,0 +1,37 @@
+<script module>
+	let active = $state(false);
+	let panelWidth = $state(null);
+
+	const store = {
+		get active() { return active; },
+		open() { active = true; },
+		close() { active = false; },
+		// This getter lazily writes $state on first read
+		get panelWidth() {
+			if (panelWidth === null) panelWidth = 42;
+
+			$effect(() => {
+				console.log('effect ran');
+			});
+
+			return panelWidth;
+		}
+	};
+</script>
+
+<script>
+	let counter = $state(0);
+</script>
+
+<button onclick={() => store.open()}>Open</button>
+<button onclick={() => store.close()}>Close</button>
+<button onclick={() => counter++}>{counter}</button>
+
+<div>
+	{#if store.active}
+		open (width: {store.panelWidth})
+	{:else}
+		closed
+	{/if}
+</div>
+ 

packages/svelte/tests/runtime-runes/samples/new-branch-reschedule/_config.js

@@ -0,0 +1,44 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const [open, close, increment] = target.querySelectorAll('button');
+
+		open.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>0</button>
+				<div>open (width: 42)</div>
+			`
+		);
+
+		increment.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>1</button>
+				<div>open (width: 42)</div>
+			`
+		);
+
+		close.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>Open</button>
+				<button>Close</button>
+				<button>1</button>
+				<div>closed</div>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/new-branch-reschedule/main.svelte

@@ -0,0 +1,31 @@
+<script module>
+	let active = $state(false);
+	let panelWidth = $state(null);
+
+	const store = {
+		get active() { return active; },
+		open() { active = true; },
+		close() { active = false; },
+		// This getter lazily writes $state on first read
+		get panelWidth() {
+			if (panelWidth === null) panelWidth = 42;
+			return panelWidth;
+		}
+	};
+</script>
+
+<script>
+	let counter = $state(0);
+</script>
+
+<button onclick={() => store.open()}>Open</button>
+<button onclick={() => store.close()}>Close</button>
+<button onclick={() => counter++}>{counter}</button>
+
+<div>
+	{#if store.active}
+		open (width: {store.panelWidth})
+	{:else}
+		closed
+	{/if}
+</div>