From e703a62b77c9de45e886d8a7f59bd0db658318f9 Mon Sep 17 00:00:00 2001
From: Micah Zoltu <micah@zoltu.net>
Date: Wed, 1 Feb 2023 16:48:42 +0800
Subject: [PATCH] Changes `EventHandler<...>` to have a `this` of type `void`.
 (#3867)

* Changes `EventHandler<...>` to have a `this` of type `void`.

Originally, `this` was set to `E['currentTarget']` which, based on the MDN doc linked in #2166, certainly sounds like the correct thing to do.  A couple years later it was changed to `never` in #3147 with no commentary on the PR or commit indicating why.  My guess is that this was changed so arrow functions would work, which I believe are permanently bound and cannot be rebound by the caller.

I believe that `never` is wrong because this code doesn't compile:
```ts
type TargetedEvent<Target extends EventTarget = EventTarget, TypedEvent extends Event = Event> = Omit<TypedEvent, 'currentTarget'> & { readonly currentTarget: Target }
interface EventHandler<E extends TargetedEvent> {
	(this: never, event: E): void
}
declare const apple: EventHandler<TargetedEvent<HTMLElement, Event>>
declare const event: TargetedEvent<HTMLElement, Event>
apple(event) // error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'never'.
```
Changing to `void` resolves this error and should work everywhere I believe.

I removed the comment since it no longer applies with this being `never` or `void`.  It appears that it was just forgotten when the switch from `E['currentTarget']` to `never` was made.

* Removes incorrectly passing test.

The type signature's intent is to make it clear to users that they cannot rely on `this` being the element in callbacks, and they should use `event.currentTarget` instead.  This test was testing that the user could unwisely ignore that.

---------

Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>
---
 src/jsx.d.ts               | 6 +-----
 test/ts/Component-test.tsx | 6 ------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/jsx.d.ts b/src/jsx.d.ts
index 2fb6813776..ef3b0fcd9c 100644
--- a/src/jsx.d.ts
+++ b/src/jsx.d.ts
@@ -418,11 +418,7 @@ export namespace JSXInternal {
 	>;
 
 	export interface EventHandler<E extends TargetedEvent> {
-		/**
-		 * The `this` keyword always points to the DOM element the event handler
-		 * was invoked on. See: https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Event_handlers#Event_handlers_parameters_this_binding_and_the_return_value
-		 */
-		(this: never, event: E): void;
+		(this: void, event: E): void;
 	}
 
 	export type AnimationEventHandler<Target extends EventTarget> = EventHandler<
diff --git a/test/ts/Component-test.tsx b/test/ts/Component-test.tsx
index b037219531..f923fcd37e 100644
--- a/test/ts/Component-test.tsx
+++ b/test/ts/Component-test.tsx
@@ -2,12 +2,6 @@ import 'mocha';
 import { expect } from 'chai';
 import { createElement, Component, RenderableProps, Fragment } from '../../';
 
-// Test `this` binding on event handlers
-function onHandler(this: HTMLInputElement, event: any) {
-	return this.value;
-}
-const foo = <input onChange={onHandler} />;
-
 export class ContextComponent extends Component<{ foo: string }> {
 	getChildContext() {
 		return { something: 2 };