tests: event-handler-dynamic: add no-op handler

[ghost]

Fixes #4087

[Conduitry]

I think this is hiding a bug, although perhaps just one in jsdom. If a null/falsy event handler is attached to an element, is the intent that triggering the event have no effect @tanhauhau ? It looks like that's what's happening, and it's only jsdom that doesn't like that.

[ghost]

Well, it errors on this part of Svelte generated code:

dispose = [
	listen(button0, "click", /*updateHandler1*/ ctx[3]),
	listen(button1, "click", /*updateHandler2*/ ctx[4]),
	listen(button2, "click", function () {
		/*clickHandler*/ ctx[0].apply(this, arguments); // EXCEPTION HERE
	}),
	listen(button3, "click", /*runTest*/ ctx[5])
];

The linters for VSCode don't pick up the possible undefined variable reference since it does get set to something in a function. I mean we're not gcc-level AST verifying here.

I guess we could make the case for outputting something like this:

// ....
listen(button2, "click", function () {
    /*clickHandler*/ ctx[0] ? ctx[0].apply(this, arguments) : void(0); 				
}),

Here's a repl to play around with:
https://svelte.dev/repl/54f3229b24e0493c9f7faf0f2dae2dfc?version=3.16.3

[ghost]

@Conduitry,

Minimal reproduction case for the exception:
https://svelte.dev/repl/7433e605325d4b4b87ea5522f5a5ff61?version=3.16.3
Without the neverCalled function, it does as you'd expect (i.e no exception).

Going back to 3.12.1, it also does what you'd expect even with the neverCalled function in place.
https://svelte.dev/repl/7433e605325d4b4b87ea5522f5a5ff61?version=3.12.1

I believe this is the bug you're worried it will mask.

[ghost]

Thinking about this on the way home, this can still be merged since the real focus of the test case is the dynamic handler assignment.

I'll make another failing test case for the falsey handler when i come up with a patch for it.

[tanhauhau]

I agree with @Conduitry that this is masking the actual bug.

When the event handler is dynamic, instead of listen/unlisten to new event handler every time whenever it has changed, we listen to a static proxy event handler that will call the actual dynamic event handler.

i think i missed out the case where dynamic handler could be undefined. so maybe i should check it before calling the actual dynamic handler.

and this may pose another issue that I just thought of, what if the dynamic event handler uses event modifier like stopPropagation. Should it propagate when the dynamic event handler is undefined? repl

<script>
	let dynamicClick;
	function divClick() {
		console.log('div clicked')
	}
</script>

<div on:click={divClick}>
	<button on:click|stopPropagation={dynamicClick}>
		dynamic button
	</button>	
</div>

[ghost]

@tanhauhau,

Yes I reached the same conclusion; however, a test case should only test one thing. In this test we are making sure assignment works correctly.

I've opened an issue for underlying bug, but we need another for modifiers as you said.

I feel like fixing the other situations is going to take a bit of time. It would be nice to clear the logs of these exceptions for now so that we aren't distracted by known issues.

[ghost]

Closing in favor of a real fix to the root cause.