svelte
svelte copied to clipboard
sveltejs
`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event
Describe the bug
<button onclick={async event => {
console.log(event.currentTarget); // <button>
await new Promise((resolve)=>resolve())
console.log(event.currentTarget); // <body>
}}>Click me</button>
The first event.currentTarget correctly references the button, but after awaiting the promise (or in a then() ), it changes reference to the body
Problem disappears if I change the onclick to the old on:click
Reproduction
https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA52Qvw6CMBDGX-XSCRIDO1oS4ws4uKlDLSdpLD3SXjGE8O6WiE5OTvfvu--Xu0ncjcUgqvMknOpQVGLf92IjeOyXIgxoGVMdKHq9dHa3yEwOyGlr9ENOKoxOAw7oGGQN08VdWJMLDKtSvoeFjt6neFK-Rd5-ZGSxsNRmPzQgpVxN8i2UJazsetlVT2UYHD7h6KkzAbPMY3IbMJf1mmV5_h-GmjFB5rk-LDdCh7vyi07P6Kgxd4ONqNhHnK_zC1Gr0X5HAQAA
Logs
No response
System Info
not relevant
Severity
annoyance
There's not much we can do about this. We can list this as a breaking change, but the event object is shared along the listeners to improve performance. I thought we might be able to detect an async closure and bail-out but you could pass that along anywhere with the event object.
Yeah, I thought this was just a bug of svelte. I didn't realized it was almost standard behaviour (in firefox event.currentTarget goes to null in a setTimeout)
@madacol I don't know what happened to on:click in svelte5, but in svelte4, the behavior also exists.
@trueadm So I don't think there's a "breaking change" there from svelte4.
The question instead is why the on:click behavior is different in svelte5 and svelte4?
Try the following code in the Svelte4 REPL and Svelte5 REPL:
<button on:click={async event => {
const button = event.currentTarget;
console.log(event.currentTarget === button); // true
await new Promise((resolve) => setTimeout(resolve, 100));
console.log(event.currentTarget === button); // false
}}>Click me</button>
Hey @Conduitry this is actually a bug?, two things I noticed after I went through the code.
(For now we will just compare v4 and v5(which includes v4 behind the hood))
on:Click : I use on:Click then it's adding a signaling event i.e.,
$.event(
"click",
button_1,
async (event) => {
debugger;
const button = event.currentTarget;
console.log(event.currentTarget === button); // true
await new Promise((resolve) => setTimeout(resolve, 100));
debugger;
console.log(event.currentTarget === button); // false
},
false
);
which means the currentTarget is button and the param dom is button and after await its uses the same event object with same currentTarget
function event(event_name, dom, handler, capture, passive)
onclick: this handled entirely in a very different way i.e.,
const event_handle = (events) => {
for (let i = 0; i < events.length; i++) {
const event_name = events[i];
if (!registered_events.has(event_name)) {
registered_events.add(event_name);
// Add the event listener to both the container and the document.
// The container listener ensures we catch events from within in case
// the outer content stops propagation of the event.
target.addEventListener(
event_name,
bound_event_listener,
PassiveDelegatedEvents.includes(event_name)
? {
passive: true
}
: undefined
);
// The document listener ensures we catch events that originate from elements that were
// manually moved outside of the container (e.g. via manual portals).
document.addEventListener(
event_name,
bound_document_event_listener,
PassiveDelegatedEvents.includes(event_name)
? {
passive: true
}
: undefined
);
}
}
};
Here two event listeners are added one in the root, which can be body and another is dom, now first listnere got called but in this case the handler_element is not button its body or root div, and so svelte, internally assign the currentTarget to button,
Till now on the first check i.e., event.currentTarget === button it will say true but actually is not bubbled more like captured form the root of the app.
Now await is called in between 2nd listener is activated and now the handler element is dom means the current object is now
changed, so on the second check it will say false i.e., event.currentTarget === button
Just wanted to understand, is making changes in the event object, will not create more bugs?
@trueadm How relevant is this to performance? I think this might be quite pernicious (and documentation usually will not cut it).
What other frameworks do, and what we could also offer, is a way to opt out of event delegation through a compiler option. I'm not sure that this bug right here warrants changing anything though - especially considering that other frameworks like React and Solid have the same gotcha.
Isn't supporting the v4 methodology is better? and make the on:click: same as onClick, I don't understand why rethinking is needed here, is js size is the issue?
@brunnerh Event delegation has a significant impact on performance in real world applications. React has an event. persist() that will generate a static event object. We could do something the same but with an imported helper that we document.
For the record, these are the differences I have noticed about how and when event.currentTarget changes reference:
Svelte 5 onclick
- Changes after any
await,.then(), or inside asetTimeout(including when the Promise resolves synchronously) - Changes to
<body>
Tested with
<button onclick={async event => {
console.log(event.currentTarget); // <button>
await new Promise((resolve)=>resolve())
console.log(event.currentTarget); // <body>
}}>Click me</button>
Svelte 5 on:click
never changes reference
Tested with
<button on:click={async event => {
console.log(event.currentTarget); // <button>
await new Promise((resolve)=>setTimeout(resolve,1000))
console.log(event.currentTarget); // <button>
}}>Click me</button>
Svelte 4 on:click and in vanilla JS onclick
- Changes reference after an operation that runs asynchronously (after an
awaitand.then()where the Promise resolves asynchronously, also inside asetTimeout, etc) - Changes to
null
E.g it changes when doing
await new Promise((resolve)=>setTimeout(resolve,1000))
but not on
await new Promise((resolve)=>resolve())
Tested with
<button on:click={async event => {
console.log(event.currentTarget); // <button>
await new Promise((resolve)=>setTimeout(resolve,0))
console.log(event.currentTarget); // null
}}>Click me</button>
and
<!DOCTYPE html>
<html>
<body>
<button>Click me</button>
<script>
document.querySelector('button').addEventListener('click', async event => {
console.log(event.currentTarget);
await new Promise((resolve)=>setTimeout(resolve, 0))
console.log(event.currentTarget);
})
</script>
</body>
</html>
Tested in Firefox, and some in chromium
@madacol Yes, as I mentioned above, this is to be expected when using onclick in Svelte 5. We're doing our own event delegation for improved memory/performance (like many other frameworks do) and sharing the event object between listeners is an optimization. If you use on:click we don't do this in Svelte 5 as we opt out of event delegation to keep parity with Svelte 4.
@madacol
when the Promise resolves synchronously ???
No, there is no synchronous resolves async function.
What you see is that the entire handler is delayed asynchronously.
This is what await actually does. You can never predict the execution order and timing of async functions.
What we need to know is that according to the documentation, currentTarget always points to the element to which the event is currently propagated in the bubbling order.
So yes, when the async function finished before the event bubbles up to the next element, it behaves like currentTarget has not changed, which is why I gave it an explicit delay with setTimeout.