jsx-handler-names, stateless components and object destructuring

[renato]

I'm seeing something related to #346 in my eslint errors:

I solved it by changing the code, but shouldn't it work considering input is just a prop?

[ljharb]

Try const handleChange = input.onChange; first.

[renato]

@ljharb I did solve it by changing the code as I said but the point is: that code shouldn't yield an error, should it?

The same object destructuring works without any trick in normal components.

[ljharb]

Ah, fair point. If the same code (with const { input } = this.props; and then using input.onChange) passes in a non-SFC, then there’s definitely a bug.

The bug might be that the SFC warns, or that the stateful component doesn’t warn, but either way they should behave the same.

[renato]

My mistake: the message happens with a stateful component too.

Actually, it happens even if not destructuring the object: onChange={props.input.onChange} and onChange={this.props.input.onChange}.

In other words, the rule always accepts a props.onEvent but never a nested props.nested.onEvent, and it's the same when destructuring.

https://github.com/yannickcr/eslint-plugin-react/blob/4f3fc51e32467afbeccc4954765f4312e8e76edf/lib/rules/jsx-handler-names.js#L39

I think it's a bug because if props.onChange is acceptable, why props.input.onChange wouldn't be?

[ljharb]

It shouldn't be acceptable in general because flat props are way better than nested ones :-)

However, I agree that it should be equally acceptable to this rule.

[acherkashin]

@ljharb Any updates on this issue?

[ljharb]

@acherkashin no; but a PR with failing tests would be very helpful.

[ljharb]

It seems like this can be covered by setting eventHandlerPropPrefix to false. Happy to reopen if not.

[riteshjagga]

I don't understand how setting eventHandlerPropPrefix to false fixes this issue.

AFAIK, eventHandlerPropPrefix applies to the

• prop name i.e. before the =
• and also when a handler is assigned as it is from the function props i.e. after the =

<MyComponent onChange={props.onChange} />

What if I want to code like this

const MyInput = ({onChange}) => {
 ...
  return <input type='text' onChange={onChange} />
}

[ljharb]

@riteshjagga what's the issue with doing that? (besides that components shouldn't be arrow functions)

[riteshjagga]

Upon using this i.e. destructuring onChange from props,

<input type='text' onChange={onChange} />

I get this issue, which should not be reported

ESLint: Handler function for `onChange` prop key must be a camelCase name beginning with 'handle' only(react/jsx-handler-names)

If I set eventHandlerPropPrefix: false (what you suggested), these cases are treated valid and eslint doesn't report that event handlers should begin with handle prefix

<input type='text' onChange={changeHandler} />
OR
<input type='text' onChange={textIsBeingChanged} />

---

For the besides part, as far as what I've read, this is more of a stylistic preference which is project specific. Please let me know if there is any other reason.

[ljharb]

It's indeed stylistic, and if you prefer on you should configure it that way.

That said, if setting it to false stops reporting on those two examples but is reporting on onChange, that's a bug - could you please file a new issue?

[riteshjagga]

It is not reporting onChange either and it seems that it doesn't report at all. So, I'm not sure if a bug should be opened per what you said but you can let me know again after going through this observation.

I tried to inspect jsx-handler-names code to see what is happening:

• Setting eventHandlerPropPrefix: false, sets the eventHandlerPropPrefix to null - Line 97
• This then sets the PROP_EVENT_HANDLER_REGEX to null - Line 104
• This then sets propIsEventHandler to null - Line 140
• And then neither of the if conditions executes - Line 144 and Line 157

Please let me know what do you think?

[ljharb]

Right, i expect them all not to warn if it’s set to false. If you want the prefix “on” you’d need to set it to that.

[riteshjagga]

Sorry, I'm confused.

i expect them all not to warn if it’s set to false>

It seems this is desired

It seems like this can be covered by setting eventHandlerPropPrefix to false.

This is what you suggested above and closed the bug and I took it as a solution to what is reported.

So both these look contradictory. Am I wrong in understanding the conversation?

---

Also if you can let me know if a new bug needs to open for the code situation mentioned in this link.

[ljharb]

Maybe I'm confused :-) are you saying that you want it to warn on onChange={onChange}, or you want it not to warn? Which one are you unable to achieve?

[riteshjagga]

I do not want onChange={onChange} to be warned because it comes from destructuring the props.

but I want onChange={changeHandler} to be warned because it doesn't use handle prefix and doesn't come from the props too.

[ljharb]

The rule definitely doesn't - and isn't intended to - differentiate based on whether it comes from props or not. If the prop is named onChange then you can rename it when you destructure it to handleChange, for example.

[riteshjagga]

OK. Thank you!

[Bro3Simon]

I believe this rule doesn't work with destructuring because when you destructure you are creating a local variable.

This produces and error.

export function Component({onClick}) {
  return (
    <Button
      onClick={onClick}
    >
      Click Me
    </Button>
  );
}

This works fine.

export function Component(props) {
  return (
    <Button
      onClick={props.onClick}
    >
      Click Me
    </Button>
  );
}

If you destructure you will unfortunately have to rename on destructure in order for it to work.

export function Component({onClick: handleClick}) {
  return (
    <Button
      onClick={handleClick}
    >
      Click Me
    </Button>
  );
}

Configuration

    "react/jsx-handler-names": [
      "error",
      {
        "checkInlineFunction": true,
        "checkLocalVariables": true
      }
    ],

[ljharb]

Yes, forcing you to rename on destructure is the intended consequence of the rule here.