DEV Community

Cover image for 10 Things NOT To Do When Building React Applications
jsmanifest
jsmanifest

Posted on • Edited on • Originally published at jsmanifest.com

10 Things NOT To Do When Building React Applications

Follow me on medium if you haven't already :)

React is such a popular tool for developing in the web and i'm sure you react fans out there are feeling blessed to be able to get your hands dirty with such a great library :)

Unfortunately nothing is perfect in life, and react is no different.

React comes with its own set of gotchas--some of it potentially becoming a severe problem to your applications if you don't take care of them now.

Here are 10 Things NOT To Do When Building React Applications:

1. Spending Too Much Time In Your Own Private World

react isolation

If you're spending too much time coding everything in your project and not taking some time to read on what's happening in the community, you might be at risk of coding bad practices that have been reported in the community. And you might be at risk of continuing to code those bad practices until you did it 20 times before you finally got the chance to find out on a medium post that it was bad.

When that happens, now you have to go back and refactor those 20 code implementations because you found out too late while everybody else is ahead of you and moved on with newer news.

When react released hooks, I was so excited and began building a bunch of mini projects to play with these new toys that everybody was all hyped up about. After reading a couple sources that hooks were going to be stable I began implementing these more seriously to my projects. I was using useState and useEffect like a boss everywhere.

And then I came across someone linking to this twitter tweet, which lead me to do some more research about useReducer.

That 30 minutes of research was enough to have me go back and refactor a good amount of code.

2. Using .bind (Not class component constructors)

I think the majority of us react developers are aware that we should .bind our class methods if we want to reference this to access their own class instance inside their methods. (Unless you're using a transpiler to transpile your class properties and methods.

That's great and I agree to prefer declaring them with arrow functions.

But this part I am going to talk about isn't about that. It's about inline functions--or functions that is defined within the render method of a react component and passed down as a prop to a child component.

When inline functions are defined in the render method, react begins to designate a new function instance each time the component re-renders. This is known to cause performance issues due to wasteful re-rendering.

Let's take a look at this example:

const ShowMeTheMoney = () => {
  const [money, setMoney] = useState(0)

  const showThemTheMoney = (money) => {
    setMoney(money)
  }

  const hideTheMoney = () => {
    setMoney(null)
  }

  const sayWhereTheMoneyIs = (msg) => {
    console.log(msg)
  }

  return (
    <div>
      <h4>Where is the money?</h4>
      <hr />
      <div style={{ display: 'flex', alignItems: 'center' }}>
        <SomeCustomButton
          type="button"
          onClick={() => sayWhereTheMoneyIs("I don't know")}
        >
          I'll tell you
        </SomeCustomButton>{' '}
        <SomeCustomButton type="button" onClick={() => showThemTheMoney(0.05)}>
          I'll show you
        </SomeCustomButton>
      </div>
    </div>
  )
}
Enter fullscreen mode Exit fullscreen mode

We know that onClick={() => sayWhereTheMoneyIs("I don't know")} and onClick={() => showThemTheMoney(0.05)} are inline functions.

I've seen a couple of tutorials (including one from Udemy) that encourage doing this:

return (
  <div>
    <h4>Where is the money?</h4>
    <hr />
    <div style={{ display: 'flex', alignItems: 'center' }}>
      <SomeCustomButton
        type="button"
        onClick={sayWhereTheMoneyIs.bind(null, "I don't know")}
      >
        I'll tell you
      </SomeCustomButton>{' '}
      <SomeCustomButton
        type="button"
        onClick={showThemTheMoney.bind(null, 0.05)}
      >
        I'll show you
      </SomeCustomButton>
    </div>
  </div>
)
Enter fullscreen mode Exit fullscreen mode

This seems like it caches the reference thus avoiding unnecessary re-renders because they aren't using arrow inline functions in the render method, but they are actually still creating new functions on each render phase!

Some of us may have already known that if we'd been following the community in the react ecosystem during the times when class components were trending.

However, since react hooks were released the talks about .bind have been swaying away since class components are becoming less popular---and usually, when .bind was the topic to talk about, it'd usually be about binding class methods. And to addon to that, these examples above aren't even binding to class methods at all, so that makes it even harder to notice the consequences here if you aren't careful enough.

The newcomers should especially be aware of this anti-pattern!

3. Passing Dynamic Values As Keys to Children

Have you ever come across a time where you felt forced to provide unique keys to children that were being mapped over?

It's good to provide unique keys:

const Cereal = ({ items, ...otherProps }) => {
  const indexHalf = Math.floor(items.length / 2)
  const items1 = items.slice(0, indexHalf)
  const items2 = items.slice(indexHalf)
  return (
    <>
      <ul>
        {items1.map(({ to, label }) => (
          <li key={to}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
      <ul>
        {items2.map(({ to, label }) => (
          <li key={to}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
    </>
  )
}
Enter fullscreen mode Exit fullscreen mode

Now pretend that some to values in items1 happen to be the same as some in items2.

I've seen that when some people want to refactor a similar component to this, they'd end up doing something like this:

import { generateRandomUniqueKey } from 'utils/generating'

const Cereal = ({ items, ...otherProps }) => {
  const indexHalf = Math.floor(items.length / 2)
  const items1 = items.slice(0, indexHalf)
  const items2 = items.slice(indexHalf)
  return (
    <>
      <ul>
        {items1.map(({ to, label }) => (
          <li key={generateRandomUniqueKey()}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
      <ul>
        {items2.map(({ to, label }) => (
          <li key={generateRandomUniqueKey()}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
    </>
  )
}
Enter fullscreen mode Exit fullscreen mode

This does get the job done with providing unique keys to each children. But there are two things wrong:

  1. Not only are we making react do unnecessary work with generating unique values, but we're also ending up recreating all of our nodes on every render because the key is different each time.

  2. The key concept in react is all about identity. And to identify which component is which, the keys do need to be unique, but not like that.

Something like this would have become a little better:

import { generateRandomUniqueKey } from 'utils/generating'

const Cereal = ({ items, ...otherProps }) => {
  const indexHalf = Math.floor(items.length / 2)
  const items1 = items.slice(0, indexHalf)
  const items2 = items.slice(indexHalf)
  return (
    <>
      <ul>
        {items1.map(({ to, label }) => (
          <li key={`items1_${to}`}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
      <ul>
        {items2.map(({ to, label }) => (
          <li key={`items2_${to}`}>
            <Link to={to}>{label}</Link>
          </li>
        ))}
      </ul>
    </>
  )
}
Enter fullscreen mode Exit fullscreen mode

Now we should feel confident that each item will have their own unique key value while preserving their identity.

4. Declaring Default Parameters Over Null

I was once guilty of spending a good amount of time debugging something similar to this:

const SomeComponent = ({ items = [], todaysDate, tomorrowsDate }) => {
  const [someState, setSomeState] = useState(null)

  return (
    <div>
      <h2>Today is {todaysDate}</h2>
      <small>And tomorrow is {tomorrowsDate}</small>
      <hr />
      {items.map((item, index) => (
        <span key={`item_${index}`}>{item.email}</span>
      ))}
    </div>
  )
}

const App = ({ dates, ...otherProps }) => {
  let items
  if (dates) {
    items = dates ? dates.map((d) => new Date(d).toLocaleDateString()) : null
  }

  return (
    <div>
      <SomeComponent {...otherProps} items={items} />
    </div>
  )
}
Enter fullscreen mode Exit fullscreen mode

Inside our App component, if dates ends up being falsey, it will be initialized with null.

And if we run the code--if you're like me, our instincts tell us that items should be initialized to an empty array by default if it was a falsey value. But our app will crash when dates is falsey because items is null. What?

Default function parameters allow named parameters to become initialized with default values if no value or undefined is passed!

In our case, even though null is falsey, it's still a value!

This mistake caused me a lot of time to debug, especially when the null value was coming from the redux reducers! Ugh.

5. Leaving Repetitive Code Untouched

it can be tempting to copy and paste code when you're being rushed to push out a fix as it can sometimes be the quickest solution.

Here is an example of repetitive code:

const SomeComponent = () => (
  <Body noBottom>
    <Header center>Title</Header>
    <Divider />
    <Background grey>
      <Section height={500}>
        <Grid spacing={16} container>
          <Grid xs={12} sm={6} item>
            <div className={classes.groupsHeader}>
              <Header center>Groups</Header>
            </div>
          </Grid>
          <Grid xs={12} sm={6} item>
            <div>
              <img src={photos.groups} alt="" className={classes.img} />
            </div>
          </Grid>
        </Grid>
      </Section>
    </Background>
    <Background grey>
      <Section height={500}>
        <Grid spacing={16} container>
          <Grid xs={12} sm={6} item>
            <div className={classes.labsHeader}>
              <Header center>Labs</Header>
            </div>
          </Grid>
          <Grid xs={12} sm={6} item>
            <div>
              <img src={photos.labs} alt="" className={classes.img} />
            </div>
          </Grid>
        </Grid>
      </Section>
    </Background>
    <Background grey>
      <Section height={300}>
        <Grid spacing={16} container>
          <Grid xs={12} sm={6} item>
            <div className={classes.partnersHeader}>
              <Header center>Partners</Header>
            </div>
          </Grid>
          <Grid xs={12} sm={6} item>
            <div>
              <img src={photos.partners} alt="" className={classes.img} />
            </div>
          </Grid>
        </Grid>
      </Section>
    </Background>
  </Body>
)
Enter fullscreen mode Exit fullscreen mode

Now's a good time to begin thinking about how to abstract these components in a way where they can be reused multiple times without changing the implementation. If there was a styling issue in one of the Grid components relative to their surrounding *Grid container*s, you'd have to manually change every single one of them.

A better way to have this coded is probably abstracting out the repeated parts, and passing in the props that are slightly different:

const SectionContainer = ({
  bgProps,
  height = 500,
  header,
  headerProps,
  imgProps,
}) => (
  <Background {...bgProps}>
    <Section height={height}>
      <Grid spacing={16} container>
        <Grid xs={12} sm={6} item>
          <div {...headerProps}>
            <Header center>{header}</Header>
          </div>
        </Grid>
        <Grid xs={12} sm={6} item>
          <div>
            <img {...imgProps} />
          </div>
        </Grid>
      </Grid>
    </Section>
  </Background>
)

const SomeComponent = () => (
  <Body noBottom>
    <Header center>Title</Header>
    <Divider />
    <SectionContainer
      header="Groups"
      headerProps={{ className: classes.groupsHeader }}
      imgProps={{ src: photos.groups, className: classes.img }}
    />
    <SectionContainer
      bgProps={{ grey: true }}
      header="Labs"
      headerProps={{ className: classes.labsHeader }}
      imgProps={{ src: photos.labs, className: classes.img }}
    />
    <SectionContainer
      height={300}
      header="Partners"
      headerProps={{ className: classes.partnersHeader }}
      imgProps={{ src: photos.partners, className: classes.img }}
    />
  </Body>
)
Enter fullscreen mode Exit fullscreen mode

So now if your boss ends up changing his mind and wants to make all of these sections about 300px in height, you only have one place to change it.

Now i'm not trying to recommend a solution like this if we were looking to make a component supporting several use cases, this is for specific uses where we know it is going to be re-used only in that environment. A more dynamic re-usable solution for SectionContainer that support multiple use cases would have probably been coded to be more generic like this, still without changing the implementation:

const SectionContainer = ({
  bgProps,
  sectionProps,
  children,
  gridContainerProps,
  gridColumnLeftProps,
  gridColumnRightProps,
  columnLeft,
  columnRight,
}) => (
  <Background {...bgProps}>
    <Section {...sectionProps}>
      {children || (
        <Grid spacing={16} container {...gridContainerProps}>
          <Grid xs={12} sm={6} item {...gridColumnLeftProps}>
            {columnLeft}
          </Grid>
          <Grid xs={12} sm={6} item {...gridColumnRightProps}>
            {columnRight}
          </Grid>
        </Grid>
      )}
    </Section>
  </Background>
)
Enter fullscreen mode Exit fullscreen mode

That way we now allow the developer to optionally extend any part of the components as needed while retaining the underlying implementation.

6. Initializing Props in the constructor

When you initialize state in the constructor:

import React from 'react'

class App extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      items: props.items,
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

You might run into bugs. That's because the constructor is only called once, which is the time the component first gets created.

The next time you try to change props, the state will retain its previous value because the constructor won't be getting called in re-renders.

If you haven't come across this problem yet, I hope this helps you!

And if you were wondering how to make the props synchronize with the state, a better approach would be something like this:

import React from 'react'

class App extends React.Component {
  constructor(props) {
    super(props)
    // Initialize the state on mount
    this.state = {
      items: props.items,
    }
  }

  // Keep the state in sync with props in further updates
  componentDidUpdate = (prevProps) => {
    const items = []
    // after  calculations comparing prevProps with this.props
    if (...) {
      this.setState({ items })
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

7. Conditional Rendering with &&

A common gotcha when conditionally rendering components is using the && operator.

React will attempt to render anything you provide as the alternative output if a condition doesn't meet its requirements. As such, when we look at this:

const App = ({ items = [] }) => (
  <div>
    <h2>Here are your items:</h2>
    <div>
      {items.length &&
        items.map((item) => <div key={item.label}>{item.label}</div>)}
    </div>
  </div>
)
Enter fullscreen mode Exit fullscreen mode

This will actually render a number 0 on the screen when items.length is empty. JavaScript considers the number 0 as a falsey value, so when items is an empty array, the && operator won't be evaluating the expression to the right of it, and will just return the first value.

What I usually do if I want to keep the syntax is using double negation:

const App = ({ items = [] }) => (
  <div>
    <h2>Here are your items:</h2>
    <div>
      {!!items.length &&
        items.map((item) => <div key={item.label}>{item.label}</div>)}
    </div>
  </div>
)
Enter fullscreen mode Exit fullscreen mode

That way, if items is an empty array, react will not render anything on the screen if the evaluated output is a boolean.

8. Not Spreading Previous States

Something that can occasionally creep up to my list of bugs comes from carelessly implementing state update logic.

A recent situation involved react hooks, specifically a useReducer implementation. Here's a basic example of this becoming an issue:

const something = (state) => {
  let newState = { ...state }
  const indexPanda = newState.items.indexOf('panda')
  if (indexPanda !== -1) {
    newState.items.splice(indexPanda, 1)
  }
  return newState
}

const initialState = {
  items: [],
}

const reducer = (state, action) => {
  switch (action.type) {
    case 'add-item':
      return { ...state, items: [...something(state).items, action.item] }
    case 'clear':
      return { ...initialState }
    default:
      return state
  }
}
Enter fullscreen mode Exit fullscreen mode

When the something function invokes and copies the state over, the underlying items property has not changed. When we mutate it using .splice, this mutates state.items and will introduce bugs.

Be especially weary about this in larger code. We would all probably get passed a small example like the above, but when things get messy, this always has to be kept in mind at all times as it is easy to forget, especially when you're being pressured to ship code to production!

9. Not Explicitly Passing Down Props To Child Components

It's a generally recommended practice to be explicit in the props you pass down to child components.

There are a couple good reasons for this:

  1. Easier Debugging Experience
    1. You as a developer know what is being passed to each child.
      1. Other developers will also know that and will have an easier time reading the code
  2. Easier to Understand What a Component Will Do
    1. Another great thing about passing down props explicity is that when you do this, it's also documenting your code in a way where everyone understands without even needing a formal documentation. And that saves time!
  3. There will be fewer props needed in order to determine if the component should re-render or not.

Although there can be some pretty neat use cases for spreading all the props.

For example, if a parent quickly needed one or two things before passing the props down to child components, it can be easy for them (and you) to do so:

const Parent = (props) => {
  if (props.user && props.user.email) {
    // Fire some redux action to update something globally that another
    //    component might need to know about
  }

  // Continue on with the app
  return <Child {...props} />
}
Enter fullscreen mode Exit fullscreen mode

Just make sure you don't find yourself in a situation like this:

<ModalComponent
  open={aFormIsOpened}
  onClose={() => closeModal(formName)}
  arial-labelledby={`${formName}-modal`}
  arial-describedby={`${formName}-modal`}
  classes={{
    root: cx(classes.modal, { [classes.dialog]: shouldUseDialog }),
    ...additionalDialogClasses,
  }}
  disableAutoFocus
>
  <div>
    {!dialog.opened && (
      <ModalFormRoot
        animieId={animieId}
        alreadySubmitted={alreadySubmitted}
        academy={academy}
        user={user}
        clearSignature={clearSignature}
        closeModal={closeModal}
        closeImageViewer={closeImageViewer}
        dialog={dialog}
        fetchAcademyMember={fetchAcademyMember}
        formName={formName}
        formId={formId}
        getCurrentValues={getCurrentValues}
        header={header}
        hideActions={formName === 'signup'}
        hideClear={formName === 'review'}
        movieId={movie}
        tvId={tvId}
        openPdfViewer={openPdfViewer}
        onSubmit={onSubmit}
        onTogglerClick={onToggle}
        seniorMember={seniorMember}
        seniorMemberId={seniorMemberId}
        pdfViewer={pdfViewer}
        screenViewRef={screenViewRef}
        screenRef={screenRef}
        screenInputRef={screenInputRef}
        updateSignupFormValues={updateSignupFormValues}
        updateSigninFormValues={updateSigninFormValues}
        updateCommentFormValues={updateCommentFormValues}
        updateReplyFormValues={updateReplyFormValues}
        validateFormId={validateFormId}
        waitingForPreviousForm={waitingForPreviousForm}
        initialValues={getCurrentValues(formName)}
        uploadStatus={uploadStatus}
        uploadError={uploadError}
        setUploadError={setUploadError}
        filterRolesFalseys={filterRolesFalseys}
      />
    )}
  </div>
</ModalComponent>
Enter fullscreen mode Exit fullscreen mode

And if you do, consider splitting the component parts to separate components so that it's cleaner and more customizable.

10. Prop Drilling

Passing down props to multiple child components is what they call a "code smell".

If you don't know what prop drilling is, it means when a parent is passing down props to multiple levels of components deep down the tree.

Now the problem there isn't the parent nor the child. They should keep their implementation the same. It's the components in the middle that might become an issue in your react apps.

That's because now the components in the middle are tightly coupled and are exposed to too much information that they don't even need. The worst part is that when the parent re-renders, the components in the middle will also re-render, making a domino effect to all the child components down the chain.

A good solution is to use context instead. Or alternatively, redux for props (which consequently are going to be serialized however).

Conclusion

That concludes the end of this post :) I hope you found this article helpful to you, and make sure to follow me for future posts!

Follow me on medium if you haven't already :)

Top comments (11)

Collapse
 
slide109 profile image
Savchenkov Dmitry

Thanks for the article, there are some great tips. Just a small amendment - I suppose it'd be better to avoid using double negation and use Boolean() instead.

const App = ({ items = [] }) => (
  <div>
    <h2>Here are your items:</h2>
    <div>
      {Boolean(items.length) &&
        items.map((item) => <div key={item.label}>{item.label}</div>)}
    </div>
  </div>
)
Collapse
 
ovieokeh profile image
Ovie Okeh

Regarding the inline functions, there's an entry on the React docs that basically says using them is fine if you're using React Hooks. Here's the link for those interested: reactjs.org/docs/hooks-faq.html#ar...

Collapse
 
petyosi profile image
Petyo Ivanov

re: 6 - syncing props with state is somewhat of an antipattern (but sometimes, unavoidable). React has a special static method which makes you feel bad when using it ;):

reactjs.org/docs/react-component.h...

Cheers!

Collapse
 
consciousness_dev profile image
Ario Setiawan • Edited

If You can make this post, i think you can post reverse version of this post, which is solution of this "Not To Do" :D

and for 2nd point, i always use bind like this

<Button onClick={this.myFunction.bind(this)} />

it is not make bad effects, isn't it?
thanks

Collapse
 
ktowen profile image
ktowen • Edited

->3. Keys used within arrays should be unique among their siblings. However they don’t need to be globally unique.
reactjs.org/docs/lists-and-keys.ht...

Collapse
 
aindong profile image
Alleo Indong

11th -> Creating functions in render
Every time your app did re-render those functions are also recreated every time... that is why we have React Hooks now.

Collapse
 
oksanaromaniv profile image
Oksana Romaniv

It would be great if under each of the tips you provide the solution or "how to do". It's a bit confusing as a newcomer to React to understand what I should do instead. Thanks!

Collapse
 
thangchung profile image
Thang Chung

The point number 9 is quite cool and happens all the times when we coded

Collapse
 
amcsi profile image
Attila Szeremi⚡

For #6, aren't we supposed to use getDerivedStateFromProps()? That way the logic would only be needed there, and wouldn't need to be repeated in the constructor.

Collapse
 
jalaj profile image
Jalaj

Some very good tips here. Thanks!