From fe51cde1fabcfef7702e9fd9d81feb7579879165 Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <trueadm@users.noreply.github.com>
Date: Thu, 23 May 2024 16:42:59 +0100
Subject: [PATCH] breaking: event handlers + bindings now yield effect updates
 (#11706)

* breaking: delegated event handlers now yield effect updates

* tweak

* refactor

* refactor

* yield binding change events

* handle input event bindings

* more bindings

* more bindings

* more tests

* more tests

* address feedback

* address feedback
---
 .changeset/popular-cameras-tie.md             |  5 ++
 packages/svelte/src/constants.js              |  1 +
 .../client/dom/elements/bindings/input.js     | 11 +++--
 .../client/dom/elements/bindings/media.js     | 25 ++++++----
 .../client/dom/elements/bindings/navigator.js |  3 +-
 .../client/dom/elements/bindings/select.js    |  4 +-
 .../client/dom/elements/bindings/size.js      |  9 ++--
 .../client/dom/elements/bindings/this.js      | 16 ++++---
 .../client/dom/elements/bindings/window.js    |  7 +--
 .../internal/client/dom/elements/events.js    |  5 +-
 .../svelte/src/internal/client/runtime.js     | 47 +++++++++++++++++--
 .../svelte/tests/runtime-legacy/shared.ts     |  2 +
 12 files changed, 97 insertions(+), 38 deletions(-)
 create mode 100644 .changeset/popular-cameras-tie.md

diff --git a/.changeset/popular-cameras-tie.md b/.changeset/popular-cameras-tie.md
new file mode 100644
index 0000000000..4235a01801
--- /dev/null
+++ b/.changeset/popular-cameras-tie.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+breaking: event handlers + bindings now yield effect updates
diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js
index 2d98a8eb19..4df2f33ef6 100644
--- a/packages/svelte/src/constants.js
+++ b/packages/svelte/src/constants.js
@@ -36,6 +36,7 @@ export const RawTextElements = ['textarea', 'script', 'style', 'title'];
 export const DelegatedEvents = [
 	'beforeinput',
 	'click',
+	'change',
 	'dblclick',
 	'contextmenu',
 	'focusin',
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js
index 1196a60339..7c099a47e2 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js
@@ -4,6 +4,7 @@ import { stringify } from '../../../render.js';
 import { listen_to_event_and_reset_event } from './shared.js';
 import * as e from '../../../errors.js';
 import { get_proxied_value, is } from '../../../proxy.js';
+import { yield_updates } from '../../../runtime.js';
 
 /**
  * @param {HTMLInputElement} input
@@ -18,7 +19,7 @@ export function bind_value(input, get_value, update) {
 			e.bind_invalid_checkbox_value();
 		}
 
-		update(is_numberlike_input(input) ? to_number(input.value) : input.value);
+		yield_updates(() => update(is_numberlike_input(input) ? to_number(input.value) : input.value));
 	});
 
 	render_effect(() => {
@@ -84,10 +85,10 @@ export function bind_group(inputs, group_index, input, get_value, update) {
 				value = get_binding_group_value(binding_group, value, input.checked);
 			}
 
-			update(value);
+			yield_updates(() => update(value));
 		},
 		// TODO better default value handling
-		() => update(is_checkbox ? [] : null)
+		() => yield_updates(() => update(is_checkbox ? [] : null))
 	);
 
 	render_effect(() => {
@@ -128,7 +129,7 @@ export function bind_group(inputs, group_index, input, get_value, update) {
 export function bind_checked(input, get_value, update) {
 	listen_to_event_and_reset_event(input, 'change', () => {
 		var value = input.checked;
-		update(value);
+		yield_updates(() => update(value));
 	});
 
 	if (get_value() == undefined) {
@@ -187,7 +188,7 @@ function to_number(value) {
  */
 export function bind_files(input, get_value, update) {
 	listen_to_event_and_reset_event(input, 'change', () => {
-		update(input.files);
+		yield_updates(() => update(input.files));
 	});
 	render_effect(() => {
 		input.files = get_value();
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/media.js b/packages/svelte/src/internal/client/dom/elements/bindings/media.js
index 56606168c8..c47ee7a99c 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/media.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/media.js
@@ -1,6 +1,7 @@
 import { hydrating } from '../../hydration.js';
 import { render_effect, effect } from '../../../reactivity/effects.js';
 import { listen } from './shared.js';
+import { yield_updates } from '../../../runtime.js';
 
 /** @param {TimeRanges} ranges */
 function time_ranges_to_array(ranges) {
@@ -35,7 +36,7 @@ export function bind_current_time(media, get_value, update) {
 		}
 
 		updating = true;
-		update(media.currentTime);
+		yield_updates(() => update(media.currentTime));
 	};
 
 	raf_id = requestAnimationFrame(callback);
@@ -60,7 +61,9 @@ export function bind_current_time(media, get_value, update) {
  * @param {(array: Array<{ start: number; end: number }>) => void} update
  */
 export function bind_buffered(media, update) {
-	listen(media, ['loadedmetadata', 'progress'], () => update(time_ranges_to_array(media.buffered)));
+	listen(media, ['loadedmetadata', 'progress'], () =>
+		yield_updates(() => update(time_ranges_to_array(media.buffered)))
+	);
 }
 
 /**
@@ -76,7 +79,9 @@ export function bind_seekable(media, update) {
  * @param {(array: Array<{ start: number; end: number }>) => void} update
  */
 export function bind_played(media, update) {
-	listen(media, ['timeupdate'], () => update(time_ranges_to_array(media.played)));
+	listen(media, ['timeupdate'], () =>
+		yield_updates(() => update(time_ranges_to_array(media.played)))
+	);
 }
 
 /**
@@ -84,7 +89,7 @@ export function bind_played(media, update) {
  * @param {(seeking: boolean) => void} update
  */
 export function bind_seeking(media, update) {
-	listen(media, ['seeking', 'seeked'], () => update(media.seeking));
+	listen(media, ['seeking', 'seeked'], () => yield_updates(() => update(media.seeking)));
 }
 
 /**
@@ -92,7 +97,7 @@ export function bind_seeking(media, update) {
  * @param {(seeking: boolean) => void} update
  */
 export function bind_ended(media, update) {
-	listen(media, ['timeupdate', 'ended'], () => update(media.ended));
+	listen(media, ['timeupdate', 'ended'], () => yield_updates(() => update(media.ended)));
 }
 
 /**
@@ -103,7 +108,7 @@ export function bind_ready_state(media, update) {
 	listen(
 		media,
 		['loadedmetadata', 'loadeddata', 'canplay', 'canplaythrough', 'playing', 'waiting', 'emptied'],
-		() => update(media.readyState)
+		() => yield_updates(() => update(media.readyState))
 	);
 }
 
@@ -127,7 +132,7 @@ export function bind_playback_rate(media, get_value, update) {
 		}
 
 		listen(media, ['ratechange'], () => {
-			if (!updating) update(media.playbackRate);
+			if (!updating) yield_updates(() => update(media.playbackRate));
 			updating = false;
 		});
 	});
@@ -145,7 +150,7 @@ export function bind_paused(media, get_value, update) {
 	var callback = () => {
 		if (paused !== media.paused) {
 			paused = media.paused;
-			update((paused = media.paused));
+			yield_updates(() => update((paused = media.paused)));
 		}
 	};
 
@@ -170,7 +175,7 @@ export function bind_paused(media, get_value, update) {
 					media.pause();
 				} else {
 					media.play().catch(() => {
-						update((paused = true));
+						yield_updates(() => update((paused = true)));
 					});
 				}
 			};
@@ -234,7 +239,7 @@ export function bind_muted(media, get_value, update) {
 
 	var callback = () => {
 		updating = true;
-		update(media.muted);
+		yield_updates(() => update(media.muted));
 	};
 
 	if (get_value() == null) {
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/navigator.js b/packages/svelte/src/internal/client/dom/elements/bindings/navigator.js
index a5d505400c..28d6bf9926 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/navigator.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/navigator.js
@@ -1,3 +1,4 @@
+import { yield_updates } from '../../../runtime.js';
 import { listen } from './shared.js';
 
 /**
@@ -6,6 +7,6 @@ import { listen } from './shared.js';
  */
 export function bind_online(update) {
 	listen(window, ['online', 'offline'], () => {
-		update(navigator.onLine);
+		yield_updates(() => update(navigator.onLine));
 	});
 }
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js
index 533e8e9af9..624c77abb0 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js
@@ -1,6 +1,6 @@
 import { effect } from '../../../reactivity/effects.js';
 import { listen_to_event_and_reset_event } from './shared.js';
-import { untrack } from '../../../runtime.js';
+import { untrack, yield_updates } from '../../../runtime.js';
 import { is } from '../../../proxy.js';
 
 /**
@@ -90,7 +90,7 @@ export function bind_select_value(select, get_value, update) {
 			value = selected_option && get_option_value(selected_option);
 		}
 
-		update(value);
+		yield_updates(() => update(value));
 	});
 
 	// Needs to be an effect, not a render_effect, so that in case of each loops the logic runs after the each block has updated
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/size.js b/packages/svelte/src/internal/client/dom/elements/bindings/size.js
index afbba473aa..771975d7a3 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/size.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/size.js
@@ -1,5 +1,5 @@
 import { effect, render_effect } from '../../../reactivity/effects.js';
-import { untrack } from '../../../runtime.js';
+import { untrack, yield_updates } from '../../../runtime.js';
 
 /**
  * Resize observer singleton.
@@ -88,7 +88,10 @@ export function bind_resize_observer(element, type, update) {
 				? resize_observer_border_box
 				: resize_observer_device_pixel_content_box;
 
-	var unsub = observer.observe(element, /** @param {any} entry */ (entry) => update(entry[type]));
+	var unsub = observer.observe(
+		element,
+		/** @param {any} entry */ (entry) => yield_updates(() => update(entry[type]))
+	);
 	render_effect(() => unsub);
 }
 
@@ -101,7 +104,7 @@ export function bind_element_size(element, type, update) {
 	var unsub = resize_observer_border_box.observe(element, () => update(element[type]));
 
 	effect(() => {
-		untrack(() => update(element[type]));
+		yield_updates(() => untrack(() => update(element[type])));
 		return unsub;
 	});
 }
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/this.js b/packages/svelte/src/internal/client/dom/elements/bindings/this.js
index de78858b60..125ead6cd2 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/this.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/this.js
@@ -1,6 +1,6 @@
 import { STATE_SYMBOL } from '../../../constants.js';
 import { effect, render_effect } from '../../../reactivity/effects.js';
-import { untrack } from '../../../runtime.js';
+import { untrack, yield_updates } from '../../../runtime.js';
 import { queue_micro_task } from '../../task.js';
 
 /**
@@ -37,12 +37,14 @@ export function bind_this(element_or_component, update, get_value, get_parts) {
 
 			untrack(() => {
 				if (element_or_component !== get_value(...parts)) {
-					update(element_or_component, ...parts);
-					// If this is an effect rerun (cause: each block context changes), then nullfiy the binding at
-					// the previous position if it isn't already taken over by a different effect.
-					if (old_parts && is_bound_this(get_value(...old_parts), element_or_component)) {
-						update(null, ...old_parts);
-					}
+					yield_updates(() => {
+						update(element_or_component, ...parts);
+						// If this is an effect rerun (cause: each block context changes), then nullfiy the binding at
+						// the previous position if it isn't already taken over by a different effect.
+						if (old_parts && is_bound_this(get_value(...old_parts), element_or_component)) {
+							update(null, ...old_parts);
+						}
+					});
 				}
 			});
 		});
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/window.js b/packages/svelte/src/internal/client/dom/elements/bindings/window.js
index 7f3962ca04..608f1ffbd2 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/window.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/window.js
@@ -1,4 +1,5 @@
 import { effect, render_effect } from '../../../reactivity/effects.js';
+import { yield_updates } from '../../../runtime.js';
 import { listen } from './shared.js';
 
 /**
@@ -15,7 +16,7 @@ export function bind_window_scroll(type, get_value, update) {
 		clearTimeout(timeout);
 		timeout = setTimeout(clear, 100); // TODO use scrollend event if supported (or when supported everywhere?)
 
-		update(window[is_scrolling_x ? 'scrollX' : 'scrollY']);
+		yield_updates(() => update(window[is_scrolling_x ? 'scrollX' : 'scrollY']));
 	};
 
 	addEventListener('scroll', target_handler, {
@@ -53,7 +54,7 @@ export function bind_window_scroll(type, get_value, update) {
 	effect(() => {
 		var value = window[is_scrolling_x ? 'scrollX' : 'scrollY'];
 		if (value === 0) {
-			update(value);
+			yield_updates(() => update(value));
 		}
 	});
 
@@ -69,5 +70,5 @@ export function bind_window_scroll(type, get_value, update) {
  * @param {(size: number) => void} update
  */
 export function bind_window_size(type, update) {
-	listen(window, ['resize'], () => update(window[type]));
+	listen(window, ['resize'], () => yield_updates(() => update(window[type])));
 }
diff --git a/packages/svelte/src/internal/client/dom/elements/events.js b/packages/svelte/src/internal/client/dom/elements/events.js
index 1479a00bf5..81285acb30 100644
--- a/packages/svelte/src/internal/client/dom/elements/events.js
+++ b/packages/svelte/src/internal/client/dom/elements/events.js
@@ -1,5 +1,6 @@
 import { render_effect } from '../../reactivity/effects.js';
 import { all_registered_events, root_event_handles } from '../../render.js';
+import { yield_updates } from '../../runtime.js';
 import { define_property, is_array } from '../../utils.js';
 import { hydrating } from '../hydration.js';
 import { queue_micro_task } from '../task.js';
@@ -47,7 +48,7 @@ export function create_event(event_name, dom, handler, options) {
 			handle_event_propagation(dom, event);
 		}
 		if (!event.cancelBubble) {
-			return handler.call(this, event);
+			return yield_updates(() => handler.call(this, event));
 		}
 	}
 
@@ -203,7 +204,7 @@ export function handle_event_propagation(handler_element, event) {
 	}
 
 	try {
-		next(current_target);
+		yield_updates(() => next(/** @type {Element} */ (current_target)));
 	} finally {
 		// @ts-expect-error is used above
 		event.__root = handler_element;
diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js
index 1149b1074d..e70cbf3083 100644
--- a/packages/svelte/src/internal/client/runtime.js
+++ b/packages/svelte/src/internal/client/runtime.js
@@ -28,6 +28,7 @@ import { lifecycle_outside_component } from '../shared/errors.js';
 
 const FLUSH_MICROTASK = 0;
 const FLUSH_SYNC = 1;
+export const FLUSH_YIELD = 2;
 
 // Used for DEV time error handling
 /** @param {WeakSet<Error>} value */
@@ -36,6 +37,8 @@ const handled_errors = new WeakSet();
 let current_scheduler_mode = FLUSH_MICROTASK;
 // Used for handling scheduling
 let is_micro_task_queued = false;
+let is_yield_task_queued = false;
+
 export let is_flushing_effect = false;
 export let is_destroying_effect = false;
 
@@ -521,13 +524,17 @@ function infinite_loop_guard() {
  * @returns {void}
  */
 function flush_queued_root_effects(root_effects) {
+	const length = root_effects.length;
+	if (length === 0) {
+		return;
+	}
 	infinite_loop_guard();
 
 	var previously_flushing_effect = is_flushing_effect;
 	is_flushing_effect = true;
 
 	try {
-		for (var i = 0; i < root_effects.length; i++) {
+		for (var i = 0; i < length; i++) {
 			var effect = root_effects[i];
 
 			// When working with custom elements, the root effects might not have a root
@@ -563,19 +570,31 @@ function flush_queued_effects(effects) {
 	}
 }
 
-function process_microtask() {
+function process_deferred() {
 	is_micro_task_queued = false;
+	is_yield_task_queued = false;
 	if (flush_count > 101) {
 		return;
 	}
 	const previous_queued_root_effects = current_queued_root_effects;
 	current_queued_root_effects = [];
 	flush_queued_root_effects(previous_queued_root_effects);
-	if (!is_micro_task_queued) {
+	if (!is_micro_task_queued && !is_yield_task_queued) {
 		flush_count = 0;
 	}
 }
 
+async function yield_tick() {
+	// TODO: replace this with scheduler.yield when it becomes standard
+	await new Promise((fulfil) => {
+		requestAnimationFrame(() => {
+			setTimeout(fulfil, 0);
+		});
+		// In case of being within background tab, the rAF won't fire
+		setTimeout(fulfil, 100);
+	});
+}
+
 /**
  * @param {import('#client').Effect} signal
  * @returns {void}
@@ -584,7 +603,12 @@ export function schedule_effect(signal) {
 	if (current_scheduler_mode === FLUSH_MICROTASK) {
 		if (!is_micro_task_queued) {
 			is_micro_task_queued = true;
-			queueMicrotask(process_microtask);
+			queueMicrotask(process_deferred);
+		}
+	} else if (current_scheduler_mode === FLUSH_YIELD) {
+		if (!is_yield_task_queued) {
+			is_yield_task_queued = true;
+			yield_tick().then(process_deferred);
 		}
 	}
 
@@ -684,6 +708,19 @@ function process_effects(effect, collected_effects) {
 	}
 }
 
+/**
+ * @param {{ (): void; (): any; }} fn
+ */
+export function yield_updates(fn) {
+	const previous_scheduler_mode = current_scheduler_mode;
+	try {
+		current_scheduler_mode = FLUSH_YIELD;
+		return fn();
+	} finally {
+		current_scheduler_mode = previous_scheduler_mode;
+	}
+}
+
 /**
  * Internal version of `flushSync` with the option to not flush previous effects.
  * Returns the result of the passed function, if given.
@@ -729,7 +766,7 @@ export function flush_sync(fn, flush_previous = true) {
  * @returns {Promise<void>}
  */
 export async function tick() {
-	await Promise.resolve();
+	await yield_tick();
 	// By calling flush_sync we guarantee that any pending state changes are applied after one tick.
 	// TODO look into whether we can make flushing subsequent updates synchronously in the future.
 	flush_sync();
diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts
index 6f3a0d9048..8d3ea09e45 100644
--- a/packages/svelte/tests/runtime-legacy/shared.ts
+++ b/packages/svelte/tests/runtime-legacy/shared.ts
@@ -223,6 +223,8 @@ async function run_test_variant(
 			e.preventDefault();
 		});
 
+		globalThis.requestAnimationFrame = globalThis.setTimeout;
+
 		let mod = await import(`${cwd}/_output/client/main.svelte.js`);
 
 		const target = window.document.querySelector('main') as HTMLElement;