Make React.SyntheticEvent.target generic over T, not React.SyntheticEvent.currentTarget

[Emm]

Currently, the Typescript 2.0 signature for React's SyntheticEvent looks like:

interface SyntheticEvent<T> {
    currentTarget: EventTarget & T;
    target: EventTarget;
}

According to commit a13fa7a, target was originally EventTarget & T, but this change was (apparently) accidentally reverted during a merge in commit 5607f54.

The current signature breaks a lot of pre-2.0 code (as you can imagine, React code is replete with event handlers) for little to no benefit (since most code relies on event.target, so this means still relying on pre-2.0 code like (event.target as any).value, since event.target is not generic.

I'll be happy to provide a patch, but I'd like to be sure SyntheticEvent.target was not de-generified on purpose.

[ghost]

This is probably an mistake made during merging. Please make a PR.

[bogusfocused]

see #11041, #10784 and many more. Is a mystery why it is not fixed even after merge.

[bbenezech]

You cannot always tell target's type at compile time. Making it generic is of little value.
target is the origin of the event (which no one really cares about, it might be a span inside a link, for example)
currentTarget is the element that has the event handler attached to, which you should very much care about and type accordingly if you attached a dataset or other attributes to it, and intend to access at runtime.

Relying on target instead of currentTarget is a beginner's mistake that will bite them sooner than latter.

For example:

  handleTabClick = (e: React.SyntheticEvent<HTMLLinkElement>) => {
    e.preventDefault()
    // !!! e.target.dataset is empty if user clicked the <span>, not the <a> !!!
    // !!! how can you type e.target? User might also have clicked the <a> if it has padding !!!
    // you have to use currentTarget, and it is the element that you know the type for sure
    if (e.currentTarget.dataset['closable'] === 'true') {
      this.close()
    }
  }

render() {
  return <a onClick={this.handleTabClick} data-closable="true">
      <span className="fatty">Click me!</span>
   </a>
}

This code used to compile, as it should.

With the 'fix' merged, e.currentTarget is not typable (I can't access its dataset without a type error) and e.target is now typed as an HTMLLinkElement, when in fact there is no way we can be sure about it. (I may have clicked the span inside.

Theoretically we could add a second generic to SyntheticEvent, to type target, when user is certain about it (no children to the event's handler, or they all have the same type). But it is of little value and really misleading (because it only applies to particular cases where using casting would make a lot more sense).
In particular, if you have no children, you can use currentTarget.

[bbenezech]

Plus it is completely wrong, on a DOMAttributes<SomeType>, you get a onClick: MouseEventHandler<SomeType>, then get EventHandler<MouseEvent<SomeType>> with

    interface EventHandler<E extends SyntheticEvent<any>> {
        (event: E): void;
    }

You absolutely cannot type target differently from currentTarget. onClick expects an event handler that has handling event (aka currentTarget) as the generic. It cannot be something else, it won't compile.

[tkrotoff]

@bbenezech

handleTabClick = (e: React.SyntheticEvent<HTMLLinkElement>) => { ... }

I suggest instead:

handleTabClick = (e: React.FormEvent<HTMLLinkElement>) => { ... }

You also have ClipboardEvent<T>, CompositionEvent<T>, DragEvent<T>, FocusEvent<T>, KeyboardEvent<T>...
SyntheticEvent is the low level thing.

[ghost]

This was changed back. See #12239.

[devinrhode2]

I think EventTarget could at least have a few basic dom properties, no? Could it be typed to HTMLElement or at least Element?

[hontas]

Just ran into an issue with this. Our case is that we have a handler higher up in the DOM hierarchy that should perform an action if the event was not dispatched on a child of an element that has a certain class.

Our code is roughly as follows:

const onArticleClick = (event: React.MouseEvent) => {
  if (event.target.closest('.prevent-on-article-click-handler')) {
    return false
  }
  // do things...
} 

Looking at the documentation in MDN for target and currentTarget I can't see why target cannot be considered an Element for a click event?

We could of course use another approach and call event.stopPropagation() instead but there's nothing wrong with the implementation, and since target is not typed as an Element we are forced to cast it or use any.

The reasoning above by @bbenezech has it's points but it's not contradicting that target could/should default to Element, even if a user that did not know the difference was using the "wrong" one.

Happy for some feedback if I misunderstood or have reached some wrong conclusion.

The way this seems to be intended at helping users use the mostly correct property is causing us issues because the types are not mirroring what the actual DOM is giving us, and I think that typing to help rather than typing to be correct may be the wrong approach here.

I'm not proposing that target should be typed as EventTarget & T rather that it be type as EventTarget & Element (since it makes no sense having it as generic) or at least be passed down from the specific event where it makes sense that it should be an Element

[carl0s-sa]

I'm not proposing that target should be typed as EventTarget & T rather that it be type as EventTarget & Element (since it makes no sense having it as generic) or at least be passed down from the specific event where it makes sense that it should be an Element

Any plans to make this happen? Just ran into @hontas's case. I can add a PR for it if there's none already