Skip to content

Instantly share code, notes, and snippets.

@TedSean

TedSean/react-best-practices.md

Forked from wasinsandiego/react-best-practices.md
Created January 6, 2021 13:08
Show Gist options
  • Save TedSean/546891d5db45d15d9dbc8f68a121df8c to your computer and use it in GitHub Desktop.
Save TedSean/546891d5db45d15d9dbc8f68a121df8c to your computer and use it in GitHub Desktop.
Download ZIP
React
Raw
react-best-practices.md

🔻

NOTE: This page is currently a work in progress. There is a lot to cover in detail. The plan is to breakout details on conventions like 'How To Write Your Selectors' and 'How To Normalize Your Data' a bit later. For now just look for existing examples in the code or ask a friend.

To get the quick and dirty bullet list, scroll down to the TL;DR; below.

React Best Practices & Patterns

There are a few tricks we've picked up since we started that will help us with debugging, reuse, scalability, performance, and common pitfalls of complex singles page app development. We are not code Nazis but getting familiar with these things will help the whole team increase output and reduce code debt. These things should be pointed out in code reviews and corrected.

The goal is to write consistent code with less opportunities for bugs. Complex code is buggy code. It's really that simple.

Core Concepts

The main hurdle of working with react and redux is not the syntax or JSX, it's the cycle. The how, when, and why of react and redux working together. I know I felt like I was just going through the motions for a long time before it clicked. It may also be the first time you have worked with these things or even the first time working on a single page app. Hopefully a short overview will help.

What does React do?

The first misconception is that it's just the 'V' in MVC, MVVC or any other pattern acronym with a V. It's more than that. It manages how/when/if things are added and removed from the DOM. It manages memory and the cycle of rendering and running code. It also creates a universe where functional patterns thrive.

A React component should be pure. Each time it's given the same input the output will always be the same. It takes props and returns markup. The goal is to keep it that simple.

React is unidirectional. The changes in data travel down the tree - never up. The trickiest part is passing this data down to deeply nested components and telling the ancestors about events in those nested components. This is one reason we use redux.

What does Redux do?

Redux manages a single store for our app state. Instead of managing state internally in components, all that state is in one place in the redux store.

The flow is:

dispatch actions -> change app state -> components render -> components display whats in app state -> repeat

This keeps things clean. As long as a component can handle whatever's in app state, then everything is okay.

What does React-Redux do?

Redux has nothing to do with react. To make it easier to bring the two together we use react-redux. It's job is simply to make make a dispatch function available on any component we want it on.

The TL;DR; for best practices.

  • Reducers should...
    • ALWAYS return Immutable data.
    • NEVER mutate.
    • Compose functions for complex reducers.
  • Sagas should...
  • Actions should...
    • ALWAYS be created with action creators.
    • Use namespaced action type constants with this convention @@<channel>.<VERB>_<NOUN>. Ex: @@snapchat/OPEN_MENU or @@shell/OPEN_BRAND_SWITCHER.
    • Have this shape... 
      {
  type: '@@snapchat/OPEN_MENU',
  payload: {
    ...
  }
}
    • Async related actions should be created with redux-saga-routines.
  • Stateless components should...
    • Have flat props whenever possible.
    • Not extend React.Component unless using life cycle methods is required.
    • Extend React.PureComponent only if it's found to improve performance AND has flat props.
    • When extending React.PureComponent do not implement shouldComponentUpdate.
  • Connected Components should...
    • Export the class for testing without needing to mock the redux store.
    • Use pure functions for most things, defined at the module level, not as class methods. These should be exported individually so they can be tested.
    • Be used primarily for "page" components.
    • Compose stateless components.
    • Use connect to wire up the store and action creators.
    • Pass props down to the statless components.
    • Does not usually need to handle shouldComponentUpdate.
    • Does NOT use internal/local state.
  • mapStateToProps
    • Uses (reselect) selectors to get values from the store.
  • mapDispatchToProps
    • Should be an object - not a function - almost always, always.
  • Always define propTypes.
  • Always define defaultProps for non-required props.
  • Selectors should...
    • Be made with reselect.
    • Be composed from memoizable selector functions.
    • Be generic and shared when not specialized.
    • Return a useful default.
    • ALWAYS copy data from the store.
    • NEVER mutate data.
  • The store should...
    • NEVER be mutated or modified directly.
    • Not have deeply nested state tree slices.
    • ALWAYS use Immutable state tree slices (* if it's value is more than a primitive). We are using seamless-immutable for this.
    • Have state tree slice keys that are name spaced by channel if necessary.
  • Remote data fetched from API should...
    • Be normalized is there is any nesting beyond one level. We are using normalizr for this.
    • Be considered read only.
    • Have camelized keys before being put into the store. This will happen automatically if using fetch-routines to fetch the data. We are using humps for this.
  • Tests for stateless components should...
    • Only use shallow render.
    • Test for specifics in addition to using a snapshot.
    • Assert different states in individual tests.
  • Tests for connected components should...
    • Import the "non-connected" class and test that.
    • Import all pure module functions for testing.
    • Only use shallow render. Using the 2 points mentioned above, a deep render is unnecessary.
  • Snapshots should...
    • Use snapshots only if verified to be correct with your real, life, actual, human eyes.
    • Not be used in isolation, i.e. writ assertions that things like class names or content are actually there.
    • Smell if updated in a PR. Always look these over in when reviewing PRs. Blindly updating snapshots can lead to this.

The Good, the Bad, and the Ugly

Don't duplicate source of truth

This leads to the duplication of the source of truth. Is it state or is it props? If you pass in new props.mrData it wont be used because its only invoked when the component is first created. DON’T do this. If you do need to set state from props clearly mark the prop name so its clear that it's only used as seed data.

// Bad
constructor(props) {
  super(props)
  this.state = {
    mrData: props.mrData
  }
}

// Good
constructor(props) {
  super(props)
  this.state = {
    mrData: props.initialMrData
  }
}

Don’t bypass the React event system unnecessarily

Avoid bypassing things React handles unless there's no other reasonable way. There are lots of ways to do this and lots of reasons why we shouldn't. Here are just a couple.

Events React normalizes events so that they have consistent properties across different browsers and reused for performance reasons.

// Bad
class Hello extends React.Component {
  componentDidMount() {
    document.addEventListener(‘click’, this.onClick)
  }
  
  onClick = () => alert(‘Clicked!’)
  
  render() {
    return (
      <div>
        <h1>Hello {this.props.name}</h1>
      </div>
    )
  }
}

// Good
const ClickAnything = ({ onClick }) => (
  <div onClick={onClick}>
    {children}
  </div>
)

class App extends React.Component {
  onClick = () => alert(‘Anything Was Clicked!’)
  
  render() {
    return (
      <ClickAnything onClick={this.onClick}>
        <Hello name='yurmom' />
      </ClickAnything>
    )
  }
}

DOM Manipulation

// Bad
const onClick = () => {
  if ($('.menu').hasClass('isOpen')) { 
    $('.menu').removeClass('isOpen')
  } else {
    $('.menu').addClass('isOpen')
  }
}
const Menu = () => (
  <div className='menu' onClick={onClick} >
    <item className='menuItem'>Thing 1</item>
    <item className='menuItem'>Thing 2</item>
  </button>
)

// Good
const Menu = ({ isOpen, onClick  }) => (
  <div className={`menu ${isOpen ? 'isOpen' : ''}`} onClick={onClick} >
    <item className='menuItem'>Thing 1</item>
    <item className='menuItem'>Thing 2</item>
  </button>
)

MapDispatchToProps

Unless you need to dispatch an action based on ownProps or getting into advance scenarios using per-instance memoization, this should be an object – not a function. The connect HoC will do the work of wrapping your action creators and call them with dispatch for you. Using an object will keep our component code clean, reduce boilerplate, and eliminate the occasional misstep that results in an action creator silently failing to dispatch. Robots are better at writing boilerplate code than we are.

// Bad(ish)
const mapDispatchToProps = (dispatch) => {
  openMenu: () => {
    dispatch(openMenu())
  },
  getCampaign: (campaignId) => {
    dispatch(getCampaign({ campaignId }))
  }
}

// Good
const mapDispatchToProps = {
  openMenu,
  getCampaign
}

Write Action Creators

Event if it's a simple action without a payload. It's best to keep this pattern consistent. Write your action creators in a separate actions file. It will help us separate concerns and keep the use of action type constants to the reducers and sagas.

// Bad (not using mapDispatchToState at all)
const onCampaignDetailClick = () => {
  this.props.dispatch({ type: GET_CAMPAIGN, payload: { campaignId: this.props.campaignId } })
}

// Bad ()
const mapDispatchToProps = {
  openMenu: () => ({ type: OPEN_MENU }),
  getCampaign: (campaignId) => ({ type: GET_CAMPAIGN, payload: { campaignId } })
}

// Good
const onCampaignDetailClick = () => {
  this.props.getCampaign(this.props.campaignId)
}

const mapDispatchToProps = {
  openMenu,
  getCampaign
}

PropTypes and DefaultProps

It's a good habit to always define the propTypes for your components. And if any props are not required then also define defaultProps. This should be true even for stateless components.

// Bad
const Button = ({ onClick, label = 'Click Me' }) => (
  <button onClick={onClick}>{label}</button>
)

// Bad
const Button = ({ onClick, label }) => (
  <button onClick={onClick}>{label}</button>
)

Button.propTypes = {
  onClick: PropTypes.func,
  label: PropTypes.string
}

Button.defaultProps = {
  onClick: null, // it's okay to use null or undefined
  label: 'Click Me'
}

Keep Component Props Flat

It's best to keep all the props on component as flat as possible. This will make equality checks in shouldComponentUpdate much easier. If extending PureComponent the strict equality check that is done automatically will only work with flat props. Do not use Pure shouldComponentUpdate when extending PureComponent.

Of course flat props is not always possible, but look for every opportunity to keep props to just primitive values. Functions of course being the universal exception.

// Bad
const BrandMenuItem = ({ brand, href }) => (
  <Link id={brand.id} href={href}>{brand.name}</Link>
)

// Good
const BrandMenuItem = ({ brandId, brandName, href }) => (
  <Link id={brandId} href={href}>{brandName}</Link>
)

Avoid Expensive shouldComponentUpdate Checks

Checking to see if your component should update seem like a great performance booster on the surface. However, this can actually be expensive and hurt performance. Don't get into premature optimizations. Keeping props flat can help in this area as well.

If you do have nested data props on your component, then pass these checks off to the stateless components down the tree. If the larger parent component that composes the smaller pieces does the a recursive check on a nested data structure it will be much slower than a simple !(nextProps === this.props) check on the flat props of child components.

NOTE: BTW, This example is not great. For illustration purposes only. There are much better ways to derive data from deep structures. Checkout the info on reselect and normalizr!

// Bad
import { isEqual  } from 'lodash'

class BrandsMenuList extends React.Component {
  shouldComponentUpdate(nextprops) {
    !isEqual(nextProps.deepNestedData, this.props.deepNestedData)
  }
  
  render() {
    const { companies } = this.props.orgs
    const brands = this.deriveBrandsFromAllCompanies(companies)
    return (
      <div className={brandMenu}>
        {brands.map(brand => (
          <BrandMenuItem key={`${brand.name}-${brand.id}`} brandId={brand.id} brandName={brand.name} href={createHref(brand)} />
        ))}
      </div>
    )
  }
}

// Good
class BrandsMenuList extends React.Component {
  // No `componentShouldUpdate` here...
  
  render() {
    /* ...same as above... */
  }
}

// Let the children do the grunt work with flat props and `PureComponent`...
class BrandMenuItem extends PureComponent {
  render() {
    const { brandId, brandName, href } = this.props
    return (
      <div id={brandId} href={href}>{brandName}</div>
    )
  }
}

Use Selectors!

Selectors are functions that grab, compose, format, and/or transform the data you need in your components. Use them when getting state from the store in your mapStateToProps function.

Please use the naming convention get<Noun> for your selectors.

Assume the example below if for the BrandsMenuList in the example above. The orgs slice is deep and all we want is a consolidated list of brands from across all orgs and companies. This code could live in the component as in the example above. But other components may need the same thing. It's much cleaner and reusable for this stuff to live in selectors.

// Bad
const mapStateToProps = state => (
  {
    orgs: state.orgs
  }
)

// Good
import { getBrands } from 'platform-selectors'

...

const mapStateToProps = state => (
  {
    brands: getBrands(state)
  }
)

Normalize Deep Data Structures

Deeply nested anything could cause performance issues, especially when we need to dip into the dark depths repeatedly. They are also a nightmare to work with when mutation is a no-no. Normalizing deep data structures that come from a remote source is a nice consistent way to format things, keep them flat, and free of duplication.

// Bad
{
  response: [
    {
      id: 1,
      type: "group",
      name: "Mullen",
      companies: [
        {
          id: 1,
          type: "company",
          name: "Nike",
          brands: [
            { "id": 1, "name": "Nike\/Football", ... }, { "id": 2, "name": "Nike\/Baseball", ... }
          ]
        },
        {
          id: 2,
          type: "company",
          name: "Tommee Tippee",
          brands: [
            { "id": 3, "name": "Explora EasiFlow", ... }, { "id": 4, "name": "Sippee Cups", ... }
          ]
        }
      ]
    }
  ],
  count: 1
}

// Good
{
  response: {
    entities: {
      brands: {
        '1': { id: 1, name: 'Nike/Football', ... },
        '2': { id: 2, name: 'Nike/Baseball', ... },
        '3': { id: 3, name: 'Explora EasiFlow', ... },
        '4': { id: 4, name: 'Sippee Cups', ... }
      },
      companies: {
        '1': { id: 1, type: 'company', name: 'Nike', brands: [ 1, 2 ] },
        '2': { id: 2, type: 'company', name: 'Tommee Tippee', brands: [ 3, 4 ] }
      },
      groups: {
        '1': { id: 1, type: 'group', name: 'Mullen', companies: [ 1, 2 ] }
      }
    },
    result: [ 1 ]
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment