currentTarget value always is null in Event object

[logvi]

Inferno 1.6.0+:
currentTarget value always is null in Event object. I have prepared example component:

const clickHandler = e => {
  console.log(e, e.currentTarget.dataset);
};

function TestComponent() {
  return (
    <div onClick={clickHandler} data-test={1}>
      <span>Click me!</span>
    </div>
  );
}

export default TestComponent;

Expected behaviour: currentTarget should be div
It's working properly in Inferno 1.4.0

[MichaelRando]

aka #874

[Havunen]

It was removed there, if we want to make it back we need to support different environments. linkEvent should replace this.

[TrySound]

@Havunen currentTarget is not replacable with linkEvent. There're different purposes in this. What the heck you remove something and publish as minor?

[trueadm]

We should test if we can set currentTarget using a try/catch.

[Havunen]

Uh-huh... That is quite a hack :/

[jmhdev]

@TrySound @trueadm @Havunen Is there a reason not to just use the undelegated onclick instead (just for cases where you need access to currentTarget)?

[trueadm]

@jmhdev you can already do that, just use onclick without camel-casing and you get the native browser version. @Havunen try/catch is no longer slow with Ignition/TurboFan in Chrome 58.

[MichaelRando]

I switched to preact over this. Maintaining React-compatibility is greater value than having to support infernojs breaking differences in the event system from React's. If you could support the React event API and make your special case optimizations opt-in instead of mandatory, it would be better for my use.

[trueadm]

@MichaelRando I'm confused with this one – Preact doesn't support synthetic events? You can back out of them by using lowercase event names, so it would work like Preact.

Unless you're mixing Inferno up with inferno-compat?

You know, it's actually somewhat insulting when people in the OSS community say this to maintainers – it's a bit of a kick in the teeth. We are only humans, we need help and support just as much as you. If something is wrong, please help us out with PRs or issues and work together. Saying to us that we're doing it wrong and that you're going somewhere else just makes maintainers stop working on OSS.

[MichaelRando]

@trueadm So in preact-compat, without synthetic events, my code does not change between React and preact versions. I can create various build targets and performance tests and compare the two. Because w/o synthetic events, the API itself is compatible with React's.

Using lowercase event names is not React-API interchangeable.

As far as I've read, inferno-compat does not include changes to event processing?

[trueadm]

@MichaelRando Is there any chance you can help us with this?

[MichaelRando]

It would be my pleasure to help out with respect to testing the platform and debating design decisions. I'm not likely to contribute pull requests since I'm not a contributor here and my work isn't open source compatible.

I think the problem is only partially emulating the React event api.

One major difference between Inferno and React is that Inferno does not rename events or change how they work by default. Inferno only specifies that events should be camel cased, rather than lower case. Lower case events will bypass Inferno's event system in favour of using the native event system supplied by the browse

Maybe the native events should map to camelcase (preact) or maybe currentTarget and the React list of event handlers should be synthesized. (react) https://facebook.github.io/react/docs/events.html

[trueadm]

@MichaelRando I'm happy for you to contribute code here if you can in free time? That's pretty much what the entire Inferno team does anyway. We're not paid for any of this.

I think there are two things:

• fixing currentTarget in our synthetic events so it aligns with React/Preact
• discuss if we should still be using delegated events by default

Personally, I'd like to rip out the event system entirely and have it as it's own thing. I'm pushing for this to happen on React too, but that's a far harder problem and maybe one to happen in the future. What the API for that would look like is tricky, maybe defined in the component or maybe more explicit? Maybe with functional stateful components too?

import { onClick } from 'inferno-events';

const clickButton = onClick.delegate((event, props, state, setState) => {
  setState({ counter: state.counter + 1 }}
})

function MyComponent(props, state, context) {
  return <button events={ [ clickButton ] }>Click me { state.counter }</button>
}

[MichaelRando]

There's

• fixing currentTarget, which should be as easy as putting it back and not sweating Safari7
• touch events Touch events not supported? #1001
• and then the conversation about delegated events or no.

I don't think the event system is detachable; I presume synthetic events fall out of using nodes that are not DOMElements and the reason it works for preact is that it doesn't go that far.

My recommendation is to get currentTarget back, and when a bug like #1001 pops up, add the synthetic version event to the master list instead of recommending a hybrid mix of native and non-native event types.

[trueadm]

#1001 was a non issue as far as I was aware, I couldn't create the issue locally. We can fix currentTarget with a try/catch for Safari.

The reason we support both delegated and not is because the usage of one or the other can have big performance impacts depending on how they are going to be used.

[Havunen]

Due to high demand from community for currentTarget we will re-introduce it in next release.

[Havunen]

New version 3.0.2 released. Closing