From 641e411cf14bad9489057df9ec4c552ae4dbcab4 Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Fri, 10 May 2024 21:14:11 +0200
Subject: [PATCH] fix: ensure spread events are always added (#11535)

In edge cases it may happen that set_attributes is re-run before the effect is executed. In that case the render effect which initiates this re-run will destroy the inner effect and it will never run. But because next and prev may have the same keys, the event would not get added again and it would get lost. We prevent this by using a root effect.
The added test case doesn't fail for some reason without this fix, but it does fail when you test it out manually, so I still added it.
Found through https://github.com/sveltejs/svelte/issues/10359#issuecomment-2101167524
---
 .changeset/two-brooms-fail.md                 |  5 ++++
 .../client/dom/elements/attributes.js         | 24 +++++++++++++++----
 .../samples/event-spread-rerun/_config.js     | 15 ++++++++++++
 .../samples/event-spread-rerun/main.svelte    |  7 ++++++
 4 files changed, 46 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/two-brooms-fail.md
 create mode 100644 packages/svelte/tests/runtime-runes/samples/event-spread-rerun/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/event-spread-rerun/main.svelte

diff --git a/.changeset/two-brooms-fail.md b/.changeset/two-brooms-fail.md
new file mode 100644
index 000000000000..63ac672fc9cd
--- /dev/null
+++ b/.changeset/two-brooms-fail.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: ensure spread events are added even when rerunning spread immediately
diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js
index 515d952bd463..d7ad037eb9d3 100644
--- a/packages/svelte/src/internal/client/dom/elements/attributes.js
+++ b/packages/svelte/src/internal/client/dom/elements/attributes.js
@@ -4,8 +4,7 @@ import { get_descriptors, get_prototype_of, map_get, map_set } from '../../utils
 import { AttributeAliases, DelegatedEvents, namespace_svg } from '../../../../constants.js';
 import { delegate } from './events.js';
 import { autofocus } from './misc.js';
-import { effect } from '../../reactivity/effects.js';
-import { run } from '../../../shared/utils.js';
+import { effect, effect_root } from '../../reactivity/effects.js';
 import * as w from '../../warnings.js';
 
 /**
@@ -112,7 +111,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
 
 	// @ts-expect-error
 	var attributes = /** @type {Record<string, unknown>} **/ (element.__attributes ??= {});
-	/** @type {Array<() => void>} */
+	/** @type {Array<[string, any, () => void]>} */
 	var events = [];
 
 	for (key in next) {
@@ -145,7 +144,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
 			if (value != null) {
 				if (!delegated) {
 					if (!prev) {
-						events.push(() => element.addEventListener(event_name, value, opts));
+						events.push([key, value, () => element.addEventListener(event_name, value, opts)]);
 					} else {
 						element.addEventListener(event_name, value, opts);
 					}
@@ -193,7 +192,22 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
 	// On the first run, ensure that events are added after bindings so
 	// that their listeners fire after the binding listeners
 	if (!prev) {
-		effect(() => events.forEach(run));
+		// In edge cases it may happen that set_attributes is re-run before the
+		// effect is executed. In that case the render effect which initiates this
+		// re-run will destroy the inner effect and it will never run. But because
+		// next and prev may have the same keys, the event would not get added again
+		// and it would get lost. We prevent this by using a root effect.
+		const destroy_root = effect_root(() => {
+			effect(() => {
+				if (!element.isConnected) return;
+				for (const [key, value, evt] of events) {
+					if (next[key] === value) {
+						evt();
+					}
+				}
+				destroy_root();
+			});
+		});
 	}
 
 	return next;
diff --git a/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/_config.js b/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/_config.js
new file mode 100644
index 000000000000..2b320e0a862c
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/_config.js
@@ -0,0 +1,15 @@
+import { test, ok } from '../../test';
+
+export default test({
+	mode: ['client'],
+
+	async test({ assert, logs, target }) {
+		const input = target.querySelector('input');
+		ok(input);
+
+		input.value = 'foo';
+		await input.dispatchEvent(new Event('input'));
+
+		assert.deepEqual(logs, ['hi']);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/main.svelte
new file mode 100644
index 000000000000..7f480447e39d
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/event-spread-rerun/main.svelte
@@ -0,0 +1,7 @@
+<script>
+	let rest = $state(undefined);
+</script>
+
+<input {...rest} oninput={() => console.log('hi')}>
+<!-- after input and inside template so that attribute spread reruns immediately -->
+{!rest ? (rest = {}) : false}