React: Parent's onClick squashed by child's onClick

[chenglou]

Child BigButton's render:

return this.transferPropsTo(<button onClick={this.childHandleClick}>Press</button>);

Parent's render:

return (<BigButton onClick={this.parentHandleClick} /><h1>hello</h1>);

In this case, childHandleClick will apparently squash parentHandleClick, and unless childHandleClick calls this.props.onClick(); inside its body there's no way to trigger parent's click event. This kind of sucks, because each time the child uses a similar event it needs to do a conditional to check if the parent's also employing it.

Also, somewhat related: is it better to attach an event like done above or to do it in componentDidMount? Waiting for 4.0's best practices.

[jordwalke]

Here's an example of what you usually want to do:

    var BigButton = React.createClass({
      childHandleClick: function(e) {
        alert('big button clicked!');
        this.props.onBigButtonClick();
      },
      render: function() {
        return (
          <button onClick={this.childHandleClick}>
              <span />
          </button>
        );
      }
    });

    var Parent = React.createClass({
      parentHandleClick: function() {
          alert('parent clicked');
      },
      render: function() {
        return (
          <BigButton onBigButtonClick={this.parentHandleClick}>
          </BigButton>
        );
      }
    });

So in general, with the current React API, components don't do _anything_ you didn't tell them to do. So in BigButton's render(), you said that the <button />s onClick should be this.childHandleClick. React will not automatically "forward" this.props.onBigButtonClick to the <button /> and there are many good reasons for that.

• Reason 1: What if you didn't want to forward it at all because you don't want to expose that as part of BigButton's API. You should get to decide everything about the contract you offer to your client code - including whether or not you want to accept an onBigButtonClick prop, and what that means.
• Reason 2: What if you wanted to forward it, but not to the <button />, rather to the inner <span />? Any kind of auto-forwarding would result in that click handler getting placed on two different nodes unintentionally.
• Reason 3: You should be in control of how this.props.onBigButtonClick is invoked. You may wish to do it before you alert('big button clicked!') or after. With the current API, you're in complete control.

Regarding having to check for null. I would first ask: Did you intend for this.props.onBigButtonClick to be optional? Or is it required? If it's required, I'd say invoke it as if it's present. If it's not required you can do:

this.props.onBigButtonClick && this.props.onBigButtonClick()

Is there another API you had in mind for this kind of stuff?

[chenglou]

That makes lots of sense, thanks. I don't have any API in mind, I guess I'm still thinking in terms of the old html.

Let me get this straight: say I have a BigInput component (just a wrapper around a normal text input, plus some classes), in the traditional DOM manipulation mentality I'd feel free to register to the input's change event from something else and act upon it, but in React's best practices, I need to specify inside BigInput the whole set of events I should accept (e.g. change, keypress, click, etc.), and basically discard every other DOM event (unless I do a transferPropsTo, but I'm asking about what I should do rather than what's possible)?

That's what prompted me to ask this question. Kind of a small paradigm change here.

[chenglou]

Yeah I know I can do that, but I was wondering if that's the best practice in React, just wanted to confirm what @jordwalke wrote.

As for overriding, classes are the union of both the child and the parent's declarations, and every other attribute I've come across prioritize the child.

[jordwalke]

Regarding best practices: I usually use transferPropsTo if the component is just a dumb wrapper around a DOM node, but also applies its own classnames etc. Other frameworks may decide to automatically forward attributes downward, which has its own initial appeal, but it's the kind of thing that (as a framework default) could cause a lot of confusion (unexpected attributes can end up on low level DOM nodes from ten levels up in the hierarchy).

I would suggest to use transferPropsTo but use it conservatively, for very low level UI components, when you intend for the component you're building to accept the complete set of DOM attributes that a <div /> would have. In other words, we shouldn't use transferPropsTo because it was an easy way to forward the only two attributes we wanted our component API to consist of. We should use transferPropsTo when we truly want our component to implement the complete API of a DOM component (and possibly more).

I'm a fan of building focused components that expose the smallest possible API. It's like making sure your component software doesn't bite off more than it can chew ;)

Disclaimer: Find what works for you. These are just rules of thumb.

[chenglou]

That was very informative, thanks.

[rafaeldwan]

Here's a modernized version of the example given above.

class BigButton extends React.Component {
  childHandleClick = (e) => {
    alert('big button clicked!')
    this.props.onBigButtonClick()
  }
  render() {
    return (
      <button onClick={this.childHandleClick}>
        <span />
      </button>
    )
  }
}

class Parent extends React.Component {
  parentHandleClick = () => {
    alert('parent clicked')
  }
  render() {
    return (
      <BigButton onBigButtonClick={this.parentHandleClick} />
    )
  }
}