`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event #11206

madacol · 2024-04-17T13:41:40Z

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

The text was updated successfully, but these errors were encountered:

trueadm · 2024-04-17T18:33:04Z

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.

madacol · 2024-04-17T20:02:21Z

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)

qupig · 2024-04-17T22:50:11Z

@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>

RaiVaibhav · 2024-04-18T07:39:03Z

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?

brunnerh · 2024-04-18T07:49:50Z

@trueadm
How relevant is this to performance?
I think this might be quite pernicious (and documentation usually will not cut it).

dummdidumm · 2024-04-18T07:59:50Z

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.

trueadm · 2024-04-18T08:28:47Z

@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.

madacol · 2024-04-18T09:44:45Z

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 a setTimeout (including when the Promise resolves synchronously)
Changes to <body>

Tested with

REPL

<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

REPL

<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 await and .then() where the Promise resolves asynchronously, also inside a setTimeout, 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

trueadm · 2024-04-18T09:55:13Z

@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.

qupig · 2024-04-18T14:53:05Z

@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.

Conduitry added the documentation label Apr 17, 2024

This comment was marked as outdated.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event #11206

`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event #11206

madacol commented Apr 17, 2024 •

edited

trueadm commented Apr 17, 2024

madacol commented Apr 17, 2024 •

edited

qupig commented Apr 17, 2024 •

edited

RaiVaibhav commented Apr 18, 2024 •

edited

brunnerh commented Apr 18, 2024

dummdidumm commented Apr 18, 2024 •

edited

This comment was marked as outdated.

trueadm commented Apr 18, 2024

madacol commented Apr 18, 2024 •

edited

trueadm commented Apr 18, 2024

qupig commented Apr 18, 2024

event.currentTarget wrongly changes reference to the <body> after an await, inside an onclick event #11206

event.currentTarget wrongly changes reference to the <body> after an await, inside an onclick event #11206

Comments

madacol commented Apr 17, 2024 • edited

Describe the bug

Reproduction

Logs

System Info

Severity

trueadm commented Apr 17, 2024

madacol commented Apr 17, 2024 • edited

qupig commented Apr 17, 2024 • edited

RaiVaibhav commented Apr 18, 2024 • edited

brunnerh commented Apr 18, 2024

dummdidumm commented Apr 18, 2024 • edited

This comment was marked as outdated.

trueadm commented Apr 18, 2024

madacol commented Apr 18, 2024 • edited

Svelte 5 onclick

Svelte 5 on:click

Svelte 4 on:click and in vanilla JS onclick

trueadm commented Apr 18, 2024

qupig commented Apr 18, 2024

`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event #11206

`event.currentTarget` wrongly changes reference to the <body> after an `await`, inside an onclick event #11206

madacol commented Apr 17, 2024 •

edited

madacol commented Apr 17, 2024 •

edited

qupig commented Apr 17, 2024 •

edited

RaiVaibhav commented Apr 18, 2024 •

edited

dummdidumm commented Apr 18, 2024 •

edited

madacol commented Apr 18, 2024 •

edited

Svelte 5 `onclick`

Svelte 5 `on:click`

Svelte 4 `on:click` and in vanilla JS `onclick`