Skip to content

Instantly share code, notes, and snippets.

@heygrady
Last active July 20, 2021 16:36
Show Gist options
  • Save heygrady/5169e555314503bba1e28b64f0d236d8 to your computer and use it in GitHub Desktop.
Save heygrady/5169e555314503bba1e28b64f0d236d8 to your computer and use it in GitHub Desktop.
Binding handlers

Binding handlers

Binding specific values to events is a common occurrence in a React application. Let's kick this off with a common example. Here we have a <Button /> that calls a click handler with a specific argument: entity. There are several things wrong here.

Note: we will be refactoring this example as we go along.

  1. We're binding a function in the render method
  2. We're binding two props together in the child component
  3. We're breaking with several established naming conventions
  4. We're using a class component when a functional component will suffice

Notice that we're creating boundHandler to ensure that handleClick is called with entity as its first argument. How could this be improved?

import React, { Component } from 'react'
import PropTypes from 'prop-types'

class Button extends Component {
  render() {
    const { entity, handleClick } = this.props
    // ❌ Avoid binding a function in the render method
    const boundHandler = () => handleClick(entity)
    return (
      <button onClick={boundHandler}>Destroy!</button>
    )
  }
}
Button.propTypes = {
  entity: PropTypes.object,
  handleClick: PropTypes.func.isRequired,
}
export default Button

Avoid binding a function in the render method

Let's fix the first problem. We want to avoid binding in the render method. Notice that the boundHandler function is recreated on every render. While it's reasonably fast to create functions, we're creating a downstream issue: the child props will constantly change.

Here's the proper solution to the render binding problem. We create a new class method (onClick) and bind it in the constructor. This way the this.onClick function will be the same for the lifetime of the <Button /> instance β€” no matter how many times it re-renders.

import React, { Component } from 'react'
import PropTypes from 'prop-types'

class Button extends Component {
  constructor(props) {
    super(props)
    // βœ… Prefer binding class methods in the constructor
    this.onClick = this.onClick.bind(this)
  }

  onClick() {
    const { entity, handleClick } = this.props
    handleClick(entity)
  }

  render() {
    return (
      <button onClick={this.onClick}>Destroy!</button>
    )
  }
}
Button.propTypes = {
  entity: PropTypes.object,
  handleClick: PropTypes.func.isRequired,
}
export default Button

Why does it matter if the function changes?

For the simple <Button /> example above... it doesn't matter. This won't actually be a performance concern as written because native elements like <button /> render incredibly fast no matter how often their props change.

But let's take a quick peek at a more complex example. Below we're replacing our super-fast <button /> element with a <ComplexSlowMonster /> component that is very slow to render. Thankfully we're relying on shallow props in ComplexSlowMonster; so it will only re-render if its props change.

Can you spot the issue?

Whoops! By passing a fresh boundHandler each time we're causing ComplexSlowMonster to re-render constantly. The fix above sidesteps this issue by binding the handler in the constructor, ensuring the function never changes.

import React, { Component } from 'react'
import PropTypes from 'prop-types'

import ComplexSlowMonster from 'components/ComplexSlowMonster'

class Button extends Component {
  render() {
    const { entity, handleClick } = this.props
    const boundHandler = () => handleClick(entity)
    return (
      // 🀦 Whoops! We force a re-render every time our props change
      <ComplexSlowMonster onClick={boundHandler}>Destroy!</ComplexSlowMonster>
    )
  }
}
Button.propTypes = {
  entity: PropTypes.object,
  handleClick: PropTypes.func.isRequired,
}
export default Button

Avoid binding two props together in the child component

This one is more subtle and it's harder to devise a work-around. Often this refactor means editing the parent component. The problem here is that any two props that exist in a component also exist in the parent component. So... if these props need bound together, why not do it there?

If you look closely, you can see that the this.onClick function is doing work that doesn't belong in the <Button /> component.

Why does <Button /> care about entity?!?

onClick() {
  // ❌ Avoid binding two props together in the child component
  const { entity, handleClick } = this.props
  handleClick(entity)
}

So, let's simplify things a bit.

  1. Prefer to push "work" upstream. Avoid doing work in a child component that could be done by the parent
  2. We only really care about onClick, so make that the only prop
  3. Notice that onClick is no longer required. The button works just fine if it is undefined
  4. Notice that we rename handleClick to onClick
import React, { Component } from 'react'
import PropTypes from 'prop-types'

class Button extends Component {
  render() {
    // βœ… Prefer to bind in the parent
    const { onClick } = this.props
    return (
      <button onClick={onClick}>Destroy!</button>
    )
  }
}
Button.propTypes = {
  // πŸŽ‰ Yay! no need to force isRequired
  onClick: PropTypes.func,
}
export default Button

Prefer to name props to match their use

Notice above that we renamed handleClick to onClick, that was on purpose. While handleClick is perfectly descriptive, it's not following the naming convention that already exists natively in HTML and within React. What do you call a click handler in HTML? onclick. What do you call a click handler in React? onClick. What did we name our click handler? handleClick?!?

It's beneficial to keep prop names as close to the standard React prop names as possible. In our case we're passing a click handler into a React component. It makes sense in context to call it onClick. Similarly, if we needed to pass in a custom CSS class name prop... we should call it className.

This naming scheme makes our components feel like native components. Our <Button /> simply barfs back a <button />. We should try to maintain as transparent an interface as possible; and consistent naming helps with that.

Avoid class components; prefer pure functional components

Notice that we've effectively removed everything from our component except for the render method. Let's finish this off and use a pure functional component instead.

Why? Because. Pure functional components enforce rigid discipline. They can only do one thing (render props) so you are forced to do all of the other work somewhere else.

  1. Pure functional components are easier to type (way less code)
  2. Their restrictive nature pressures you to move "work" out of the component
  3. Functional components aren't any faster than class components, they might even (technically) be slower
import React from 'react'
import PropTypes from 'prop-types'

// βœ… Prefer pure functional components
const Button = ({ onClick }) => (
  <button onClick={onClick}>Destroy!</button>
)

Button.propTypes = {
  onClick: PropTypes.func,
}
export default Button

In order to complete this solution we need to refactor our Parent component.

Refactoring up the component tree

We've successfully simplified our <Button /> component but we've also changed it's interface. Now we need to update all the places where it's used.

Let's fix the problem by looking at a few different scenarios.

Scenario 1: What if entity is coming from redux?

In this scenario, we're actually getting the entity from the redux store. This is a pretty common case. Below we're imagining that there's some specific entity that is defined in the redux store that our Button will be manipulating.

<ButtonContainer /> before refactoring

Remember our original <Button /> from the beginning of this article? Let's imagine that we have a <ButtonContainer /> that is passing in entity and handleClick.

Can you spot the issue?

We're effectively using <Button /> to move a value from mapStateToProps to mapDispatchToProps. That's a little silly!

import { connect } from 'redux'

import { selectEntity } from 'modules/foo/selectors'
import { destroyEntity } from 'modules/foo/actions'

import Button from './Button'

const mapStateToProps = (state) => ({
  // ❌ Avoid using the component to pass values to mapDispatchToProps
  entity: selectEntity(state)
})

const mapDispatchToProps = (dispatch) => ({
  handleClick: (entity) => {
    dispatch(destroyEntity(entity))
  }
})
const ButtonContainer = connect(mapStateToProps, mapDispatchToProps)(Button)
export default ButtonContainer

<ButtonContainer /> after refactoring (use an inline thunk)

This might seem a little weird at first but, if our project is like most projects, we have access to thunks. How do you pass values between mapStateToProps and mapDispatchToProps? You don't! You use an inline thunk!!

Below you can see that we're using an inline thunk to look up the value we're after and then dispatch that as the payload. This effectively cuts out the middle man. We no longer have to burden the <Button /> with binding our values for us!

  1. Button only cares about onClick, so that's the only prop we pass.
  2. Using an inline thunk allows us to get access to state within mapDispatchToProps. Now we can look up the value we need right when we need it.
  3. Notice that the React event is now available for us if we want to do something useful like event.preventDefault()
  4. Notice we're using _ instead of redefining dispatch in the function signature for our inline thunk. Using an _ is a standard way to ignore an argument. It doesn't matter what you do here. If you don't like the _, choose something you like more.
  5. Notice that we're passing undefined instead of mapStateToProps
import { connect } from 'redux'

import { selectEntity } from 'modules/foo/selectors'
import { destroyEntity } from 'modules/foo/actions'

import Button from './Button'

const mapDispatchToProps = (dispatch) => ({
  onClick: (event) => {
    // βœ… Prefer using inline thunks to select values from state
    dispatch((_, getState()) => {
      const state = getState()
      // compare to `this.onClick` from above
      const entity = selectEntity(state)
      dispatch(destroyEntity(entity))
    })
  }
})

const ButtonContainer = connect(undefined, mapDispatchToProps)(Button)
export default ButtonContainer

Scenario 2: What if entity is coming from a <Parent />?

In this scenario, we're still using a ButtonContainer but this time the entity is coming from a Parent component. This is a slightly contrived example. The Parent component below is breaking a different rule: never pass a prop to from a grandparent to a grandchild. Most likely this could be refactored to match Scenario 1 above. However...

import React from 'react'

import { ButtonContainer } from 'components/Foo/Button'

const Parent = ({ entity }) => (
  <ButtonContainer entity={entity} />
)

Parent.propTypes = {
  entity: PropTypes.object,
}

export default Parent

<ButtonContainer /> before refactoring

Before refactoring, our container is simply letting the entity pass straight through and relying on the Button to pass it back. After the refactoring of the <Button /> we already did, how do we get the entity into mapDispatchToProps?

import { connect } from 'redux'

import { selectEntity } from 'modules/foo/selectors'
import { destroyEntity } from 'modules/foo/actions'

import Button from './Button'

const mapDispatchToProps = (dispatch) => ({
  // ❌ Avoid using the component to pass values to mapDispatchToProps
  handleClick: (entity) => {
    dispatch(destroyEntity(entity))
  }
})
const ButtonContainer = connect(undefined, mapDispatchToProps)(Button)
export default ButtonContainer

<ButtonContainer /> after refactoring (use ownProps)

We can actually intercept the entity at the container level and avoid passing the burden to the child component. Below you can see that we use ownProps to grab the entity.

  1. Notice that the container can grab the entity from ownProps
  2. Notice that you can provide PropTypes to a container
import { connect } from 'redux'
import PropTypes from 'prop-types'

import { destroyEntity } from 'modules/foo/actions'

import Button from './Button'

const mapDispatchToProps = (dispatch, ownProps) => ({
  onClick: (event) => {
    // βœ… Prefer using ownProps to intercept props passed from parent
    const { entity } = ownProps
    dispatch(destroyEntity(entity))
  }
})

const ButtonContainer = connect(undefined, mapDispatchToProps)(Button)
ButtonContainer.propTypes = {
  entity: PropTypes.object.isRequired,
}
export default ButtonContainer

Scenario 3: What if handleClick is coming from a grandparent and bound in a loop?

This is an extension of Scenario 2, but we're actually skipping the ButtonContainer altogether. Notice that Parent is poorly designed. It should be considered poor practice to pass a prop from a grandparent to a grandchild. If you need to pass a prop deeply within the component tree... use redux.

However, what if we encounter the pattern we see below?

Parent before refactor

Here the <Parent /> is in charge of rendering a button for every entity in the array. Some Grandparent is passing in a handleClick function that will destroy the entity. A keen observer will notice that this is what the ButtonContainer was doing for us in scenario's 1 and 2!

This extremely brittle pattern is passing the burden of binding the entity to it's handler from a grandparent to a grandchild. These types of deep interdependencies should be avoided because they create hard to trace bugs and make refactoring very difficult.

  • <Button /> here is the original version from the top of this document

Can you spot the issues?

  1. The <Parent /> is passing in two values to the <Button /> to be bound together
  2. Notice that Parent has is simply passing handleDestroy to the Button
import React from 'react'

import Button from 'components/Foo/Button'

const Parent = ({ entities, handleDestroy }) => (
  <div>
    {entities && entities.map((entity) => (
      // ❌ Avoid binding two props together in the child component
      // ❌ Avoid passing a prop from grandparent to grandchild
      <Button key={entity.id} entity={entity} handleClick={handleDestroy} />
    ))}
  </div>
)

Parent.propTypes = {
  entities: PropTypes.array,
  handleDestroy: PropTypes.func,
}

export default Parent

Parent first attempt refactor (bind in the loop)

Our first attempt to fix the problem is to bind the click handler in the loop. This is effective and works well with our redesigned button... but we're also re-introducing our initial problem: binding in the render.

  • <Button /> here is the refactored version that only accepts onClick
  • Notice that refactoring Button has pushed the binding work upstream to the Parent

Can you spot the issues?

  1. The <Grandparent /> is passing handleDestroy into the <Parent /> to be bound
  2. The <Parent /> is now forced to bind the values in the render method... for every entity!
import React from 'react'

import Button from 'components/Foo/Button'

// ❌ Avoid binding two props together in the child component
const Parent = ({ entities, handleDestroy}) => (
  <div>
    {entities && entities.map((entity) => (
      // ❌ Avoid binding a function in the render method
      <Button key={entity.id} onClick={() => handleDestroy(entity)} />
    ))}
  </div>
)

Parent.propTypes = {
  entities: PropTypes.array,
  handleDestroy: PropTypes.func,
}

export default Parent

Parent second attempt (use a ButtonContainer)

How do you avoid passing props from a grandparent to a grandchild? Use a redux container!

The trick is to let the ButtonContainer manage destroying the entity itself. There's no need to have the Grandparent manage when entities are destroyed.

  1. Avoid passing a prop from grandparent to grandchild
    • Notice that we no longer receive a handleDestroy prop
    • Presumably we will refactor the <Grandparent /> to remove handleDestroy
  2. Prefer using a container to dispatch actions where they happen
    • <ButtonContainer /> below is the refactored version from scenario 2
    • We're effectively moving the handleDestroy method from the Grandparent to the ButtonContainer
  3. Notice that <Parent /> is now only concerned with one thing (rendering a list of props)

Note: What's missing from this example is refactoring Grandparent to stop passing the handleDestroy function. Most likely the <Grandparent /> looks something like one of the scenarios covered above. Refactor, rinse and repeat.

import React from 'react'

import { ButtonContainer } from 'components/Foo/Button'

const Parent = ({ entities }) => (
  <div>
    {entities && entities.map((entity) => (
      // βœ… Prefer using a container to dispatch actions where they happen
      <ButtonContainer key={entity.id} entity={entity} />
    ))}
  </div>
)

Parent.propTypes = {
  entities: PropTypes.array,
}

export default Parent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment