Skip to content

Commit 641e411

Browse files
authored
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 #10359 (comment)
1 parent 31f8fea commit 641e411

File tree

4 files changed

+46
-5
lines changed

4 files changed

+46
-5
lines changed

.changeset/two-brooms-fail.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: ensure spread events are added even when rerunning spread immediately

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import { get_descriptors, get_prototype_of, map_get, map_set } from '../../utils
44
import { AttributeAliases, DelegatedEvents, namespace_svg } from '../../../../constants.js';
55
import { delegate } from './events.js';
66
import { autofocus } from './misc.js';
7-
import { effect } from '../../reactivity/effects.js';
8-
import { run } from '../../../shared/utils.js';
7+
import { effect, effect_root } from '../../reactivity/effects.js';
98
import * as w from '../../warnings.js';
109

1110
/**
@@ -112,7 +111,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
112111

113112
// @ts-expect-error
114113
var attributes = /** @type {Record<string, unknown>} **/ (element.__attributes ??= {});
115-
/** @type {Array<() => void>} */
114+
/** @type {Array<[string, any, () => void]>} */
116115
var events = [];
117116

118117
for (key in next) {
@@ -145,7 +144,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
145144
if (value != null) {
146145
if (!delegated) {
147146
if (!prev) {
148-
events.push(() => element.addEventListener(event_name, value, opts));
147+
events.push([key, value, () => element.addEventListener(event_name, value, opts)]);
149148
} else {
150149
element.addEventListener(event_name, value, opts);
151150
}
@@ -193,7 +192,22 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
193192
// On the first run, ensure that events are added after bindings so
194193
// that their listeners fire after the binding listeners
195194
if (!prev) {
196-
effect(() => events.forEach(run));
195+
// In edge cases it may happen that set_attributes is re-run before the
196+
// effect is executed. In that case the render effect which initiates this
197+
// re-run will destroy the inner effect and it will never run. But because
198+
// next and prev may have the same keys, the event would not get added again
199+
// and it would get lost. We prevent this by using a root effect.
200+
const destroy_root = effect_root(() => {
201+
effect(() => {
202+
if (!element.isConnected) return;
203+
for (const [key, value, evt] of events) {
204+
if (next[key] === value) {
205+
evt();
206+
}
207+
}
208+
destroy_root();
209+
});
210+
});
197211
}
198212

199213
return next;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { test, ok } from '../../test';
2+
3+
export default test({
4+
mode: ['client'],
5+
6+
async test({ assert, logs, target }) {
7+
const input = target.querySelector('input');
8+
ok(input);
9+
10+
input.value = 'foo';
11+
await input.dispatchEvent(new Event('input'));
12+
13+
assert.deepEqual(logs, ['hi']);
14+
}
15+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script>
2+
let rest = $state(undefined);
3+
</script>
4+
5+
<input {...rest} oninput={() => console.log('hi')}>
6+
<!-- after input and inside template so that attribute spread reruns immediately -->
7+
{!rest ? (rest = {}) : false}

0 commit comments

Comments
 (0)