fix: further improvements to effect scheduling and flushing (#10971)

* fix: improve effect scheduling

* fix: further improvements to effect scheduling and flushin

* add test

* simplify

* simplify

* lint

* fix e2e tests

* fix e2e tests

* simplify

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

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

Co-authored-by: Rich Harris <[email protected]>

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

Co-authored-by: Rich Harris <[email protected]>

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

Co-authored-by: Rich Harris <[email protected]>

* style tweak

---------

Co-authored-by: Rich Harris <[email protected]>

Author: trueadm

.changeset/brown-houses-obey.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: further improvements to effect scheduling and flushing

packages/svelte/src/internal/client/dom/elements/attributes.js

@@ -68,7 +68,11 @@ export function set_xlink_attribute(dom, attribute, value) {
  */
 export function set_custom_element_data(node, prop, value) {
 	if (prop in node) {
-		node[prop] = typeof node[prop] === 'boolean' && value === '' ? true : value;
+		var curr_val = node[prop];
+		var next_val = typeof curr_val === 'boolean' && value === '' ? true : value;
+		if (typeof curr_val !== 'object' || curr_val !== next_val) {
+			node[prop] = next_val;
+		}
 	} else {
 		set_attribute(node, prop, value);
 	}

packages/svelte/src/internal/client/dom/legacy/lifecycle.js

@@ -1,3 +1,4 @@
+import { CLEAN } from '../../constants.js';
 import { run, run_all } from '../../../shared/utils.js';
 import { user_pre_effect, user_effect } from '../../reactivity/effects.js';
 import {
@@ -27,7 +28,7 @@ export function init() {
 			// are batched up with the current run. Avoids for example child components rerunning when they're
 			// now hidden because beforeUpdate did set an if block to false.
 			const parent = current_effect?.parent;
-			if (parent != null) {
+			if (parent != null && (parent.f & CLEAN) === 0) {
 				flush_local_render_effects(parent);
 			}
 		});

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

@@ -424,8 +424,7 @@ function infinite_loop_guard() {
 function flush_queued_root_effects(root_effects) {
 	for (var i = 0; i < root_effects.length; i++) {
 		var signal = root_effects[i];
-		var effects = get_nested_effects(signal, RENDER_EFFECT | EFFECT);
-		flush_queued_effects(effects);
+		flush_nested_effects(signal, RENDER_EFFECT | EFFECT);
 	}
 }
 
@@ -438,24 +437,13 @@ function flush_queued_effects(effects) {
 	if (length === 0) return;
 
 	infinite_loop_guard();
-	var previously_flushing_effect = is_flushing_effect;
-	is_flushing_effect = true;
-
-	try {
-		for (var i = 0; i < length; i++) {
-			var signal = effects[i];
+	for (var i = 0; i < length; i++) {
+		var effect = effects[i];
 
-			if ((signal.f & (DESTROYED | INERT)) === 0) {
-				if (check_dirtiness(signal)) {
-					execute_effect(signal);
-				}
-			}
+		if ((effect.f & (DESTROYED | INERT)) === 0 && check_dirtiness(effect)) {
+			execute_effect(effect);
 		}
-	} finally {
-		is_flushing_effect = previously_flushing_effect;
 	}
-
-	effects.length = 0;
 }
 
 function process_microtask() {
@@ -512,77 +500,57 @@ export function schedule_effect(signal) {
  * @param {import('./types.js').Effect} effect
  * @param {number} filter_flags
  * @param {boolean} shallow
- * @param {import('./types.js').Effect[]} collected_render
  * @param {import('./types.js').Effect[]} collected_user
  * @returns {void}
  */
-function recursively_collect_effects(
-	effect,
-	filter_flags,
-	shallow,
-	collected_render,
-	collected_user
-) {
+function recursively_process_effects(effect, filter_flags, shallow, collected_user) {
 	var effects = effect.effects;
 	if (effects === null) return;
 
-	var render = [];
 	var user = [];
 
 	for (var i = 0; i < effects.length; i++) {
 		var child = effects[i];
 		var flags = child.f;
+		var is_inactive = (flags & (DESTROYED | INERT)) !== 0;
+		if (is_inactive) continue;
 		var is_branch = flags & BRANCH_EFFECT;
+		var is_clean = (flags & CLEAN) !== 0;
 
 		if (is_branch) {
 			// Skip this branch if it's clean
-			if ((flags & CLEAN) !== 0) continue;
+			if (is_clean) continue;
 			set_signal_status(child, CLEAN);
 		}
 
 		if ((flags & RENDER_EFFECT) !== 0) {
 			if (is_branch) {
 				if (shallow) continue;
-				recursively_collect_effects(child, filter_flags, false, collected_render, collected_user);
+				recursively_process_effects(child, filter_flags, false, collected_user);
 			} else {
-				render.push(child);
+				if (check_dirtiness(child)) {
+					execute_effect(child);
+				}
+				recursively_process_effects(child, filter_flags, false, collected_user);
 			}
 		} else if ((flags & EFFECT) !== 0) {
-			if (is_branch) {
+			if (is_branch || is_clean) {
 				if (shallow) continue;
-				recursively_collect_effects(child, filter_flags, false, collected_render, collected_user);
+				recursively_process_effects(child, filter_flags, false, collected_user);
 			} else {
 				user.push(child);
 			}
 		}
 	}
 
-	if (render.length > 0) {
-		if ((filter_flags & RENDER_EFFECT) !== 0) {
-			collected_render.push(...render);
-		}
-
-		if (!shallow) {
-			for (i = 0; i < render.length; i++) {
-				recursively_collect_effects(
-					render[i],
-					filter_flags,
-					false,
-					collected_render,
-					collected_user
-				);
-			}
-		}
-	}
-
 	if (user.length > 0) {
 		if ((filter_flags & EFFECT) !== 0) {
 			collected_user.push(...user);
 		}
 
 		if (!shallow) {
 			for (i = 0; i < user.length; i++) {
-				recursively_collect_effects(user[i], filter_flags, false, collected_render, collected_user);
+				recursively_process_effects(user[i], filter_flags, false, collected_user);
 			}
 		}
 	}
@@ -597,35 +565,36 @@ function recursively_collect_effects(
  * @param {import('./types.js').Effect} effect
  * @param {number} filter_flags
  * @param {boolean} [shallow]
- * @returns {import('./types.js').Effect[]}
+ * @returns {void}
  */
-function get_nested_effects(effect, filter_flags, shallow = false) {
-	/**
-	 * @type {import("./types.js").Effect[]}
-	 */
-	var render_effects = [];
-	/**
-	 * @type {import("./types.js").Effect[]}
-	 */
+function flush_nested_effects(effect, filter_flags, shallow = false) {
+	/** @type {import('#client').Effect[]} */
 	var user_effects = [];
-	// When working with custom-elements, the root effects might not have a root
-	if (effect.effects === null && (effect.f & BRANCH_EFFECT) === 0) {
-		return [effect];
+
+	var previously_flushing_effect = is_flushing_effect;
+	is_flushing_effect = true;
+
+	try {
+		// When working with custom elements, the root effects might not have a root
+		if (effect.effects === null && (effect.f & BRANCH_EFFECT) === 0) {
+			flush_queued_effects([effect]);
+		} else {
+			recursively_process_effects(effect, filter_flags, shallow, user_effects);
+			flush_queued_effects(user_effects);
+		}
+	} finally {
+		is_flushing_effect = previously_flushing_effect;
 	}
-	recursively_collect_effects(effect, filter_flags, shallow, render_effects, user_effects);
-	return [...render_effects, ...user_effects];
 }
 
 /**
  * @param {import('./types.js').Effect} effect
  * @returns {void}
  */
 export function flush_local_render_effects(effect) {
-	/**
-	 * @type {import("./types.js").Effect[]}
-	 */
-	var render_effects = get_nested_effects(effect, RENDER_EFFECT, true);
-	flush_queued_effects(render_effects);
+	// We are entering a new flush sequence, so ensure counter is reset.
+	flush_count = 0;
+	flush_nested_effects(effect, RENDER_EFFECT, true);
 }
 
 /**

packages/svelte/tests/runtime-legacy/samples/before-render-chain-2/Item.svelte

@@ -0,0 +1,24 @@
+<script>
+	import {beforeUpdate} from 'svelte';
+
+	let count1 = 0;
+	let count2 = 0;
+
+	function increaseCount1() {
+		count1++;
+	}
+
+	$: if (count2 < 10) {
+		increaseCount1();
+	}
+
+	$: if (count1 < 10) {
+		count2++;
+	}
+
+	beforeUpdate(() => {
+		// We don't need to do anything
+	});
+</script>
+
+<button on:click={() => count1++}>{count1} / {count2}</button>

packages/svelte/tests/runtime-legacy/samples/before-render-chain-2/_config.js

@@ -0,0 +1,11 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, component, target }) {
+		const [btn1] = target.querySelectorAll('button');
+		flushSync(() => {
+			// This test would result in an infinite loop, so if this doesn't error, then the test is working.
+		});
+	}
+});

packages/svelte/tests/runtime-legacy/samples/before-render-chain-2/main.svelte

@@ -0,0 +1,9 @@
+<script>
+	import Item from './Item.svelte';
+
+	var items = Array(1000);
+</script>
+
+{#each items as item}
+	<Item />
+{/each}