language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

[svelte2tsx] EventHandler.currentTarget typed incorrectly

Open leumasme opened this issue 3 years ago • 0 comments

Describe the bug

The file svelte2tsx/svelte-jsx.d.ts#L348 defines EventHandler as

type EventHandler<E extends Event = Event, T extends EventTarget = HTMLElement> =
  (event: E & { currentTarget: EventTarget & T}) => any;

It therefore guarantees that currentTarget is present and even narrows it down to the right element type - for comparison, the original lib.dom.d.ts just types it as

readonly currentTarget: EventTarget | null;

It however gets problematic when we notice that currentTarget isn't always guaranteed to be there - MDN docs:

Note: The value of event.currentTarget is only available while the event is being handled. If you console.log() the event object, storing it in a variable, and then look for the currentTarget key in the console, its value will be null.

This means that, even if currentTarget was there at the beginning of the event handler call, it will disappear if execution ever leaves the event handler - for example, from awaiting a fetch request, using a setTimeout or when saving the event into a variable to later use it outside the event handler.

Thus, this code will first log the element and then null, despite its type not allowing null:

console.log(e.currentTarget);
setTimeout(()=> console.log(e.currentTarget), 0);

This may lead to unexpected crashes - just add an await fetch before you access some property of e.currentTarget and suddenly you get a crash, even though typescript says you're fine.

The problem here is that I don't believe it is possible to truly correctly type this in typescript - a property disappearing when execution leaves the task is quite specific, so the only way to safely type this may be to always make null a valid value, just like the lib.dom.d.ts define it.

Reproduction

<input
    on:keydown="{(e) => {
      console.log(e.currentTarget);
      setTimeout(() => console.log(e.currentTarget), 0);
    }}"
  />

Expected behaviour

The types should not guarantee event.currentTarget to be non-null when this can not actually be guaranteed, and the code

<input
    on:keydown="{(e) => {
      console.log(e.currentTarget.value);
      setTimeout(() => console.log(e.currentTarget.value), 0);
    }}"
  />

should not compile as it will crash.

System Info

  • OS: Windows 10
  • IDE: VSCode

Which package is the issue about?

svelte2tsx

Additional Information, eg. Screenshots

No response

leumasme avatar Aug 20 '22 13:08 leumasme