DEV Community

loading...

5 tips to improve your React and JS code

adancarrasco profile image Adán Carrasco ・4 min read

Two weeks ago I started to work in a new project where some code was already written. However, there wasn't any best practices to follow. Something that matter when you start a new project is to get together to define the foundations and best practices/guidelines the team will follow to do the best code: Maintainable, readable, easy to understand.

I'm going to describe 5 scenarios that I saw in the project and how those can be improved.

Keyword for this is: Consistency

1. Order of import modules

Arranging your ES6 modules in an organized way will save you some time while trying to find any missing/not needed modules.

Before

import { DatePicker } from '../../components'
import axios from 'axios'
import { IUser } from '../../models/User'
import React from 'react'
import { toCamelCase } from '../utils'
import { Button } from '@material-ui/core'

After

// node_modules
import React from 'react'
import { Button } from '@material-ui/core'
import axios from 'axios'

// Local modules
import { DatePicker } from '../../components'
import { toCamelCase } from '../utils'

// Types + Interfaces
import { IUser } from '../../models/User'

In the Before we can see that the packages are unordered, probably for one file wouldn't make too much noise, but when you open a ton of files and try to look for a specific package is really hard to do it. Something that the team has agreed on is to group the imports in the After way, separating each module by an empty line. The comments can be removed since the files will be consistent.

2. Using destructuring whenever is possible

Another important thing is to prevent unnecessary nesting and repetition. In most of the cases this will improve readability a lot.

Before

const UserProfile = props => (<div>
    <span>{props.firstName}</span>
    <span>{props.lastName}</span>
    <img src={props.profilePhoto}/>
  </div>)

After

const UserProfile = ({ firstName, lastName, profilePhoto }) =>
  (<div>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
  </div>)

3. Naming convention for variables and methods

Something important about code is to know what a method will return or also easily read what a variable represents just by its name, for example:

Before

let User = {}
User.car = true
User.admin = true

function NewUser() {
  return User
}

function add_photo(photo) {
  user.photo = photo
}

After

let user = {}
user.hasCar = true
user.isAdmin = true

function getUser() {
  return user
}

function setUserPhoto(photoUrl) {
  user.photoUrl = photoUrl
}

In After we are keeping consistency on how to name variables and methods, being consistent in:

  • For booleans use: is, has, should prefixes
  • For methods use get/set prefix if are for props
  • Overall use camelCase for methods and variables

4. Make your components prepared for common props

Before

const UserProfile = props => {
  const { firstName, lastName, profilePhoto } = props
  return (<div>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
  </div>)
}

After

const UserProfile = props => {
  const { firstName, lastName, profilePhoto, ...rest} = props
  return (<div {...rest}>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
  </div>)
}

In the After, the component is prepared to inject common React properties such as: style, className, key, etc. Using the spread operator you are grouping all common props and passing them to the container.

5. Really dumb components will make your life easier

Creating dumb components and following the Single Responsibility Principle allows you to create and contribute in an easily way and keep a clean codebase.

Before:

import axios from 'axios'

const UserProfile = props => {
  const [user, setUser] = React.useState(null);
  React.useEffect(() => {
    getUser();
  }, []);

  async function getUser() {
    try {
      const user = await axios.get('/user/25')
    } catch(error) {
      console.error(error)
    }

    if(user.country === "DE") {
      user.flag = "/de-flag.png"
    } else if(user.country === "MX") {
      user.flag = "/mx-flag.png"
    }
    setUser(user);
  }

  const { firstName, lastName, profilePhoto, userFlag} = user

  return (<div>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
    <img src={userFlag}>
  </div>)
}

After:

What can cause issues?
Adding Business Logic (BL) inside a component can make it hard to maintain, debug and test. My recommendation is to keep your component as presentational component. In this way you isolate the BL and you can focus on testing that part independent. Previously everything was mixed. Now we have separated each responsibility, which makes it easy to test and debug.

// UserProfilePage.jsx
// Does everything related to the UserProfilePage, adding any additional props or BL
import { fetchUser } from '../api'

const UserProfilePage = props => {
  const [user, setUser] = React.useState(null);
  React.useEffect(() => {
    getUser();
  }, []);

  async function getUser() {
    const user = fetchUser(error => console.error(error))
    if(user.country === "DE") {
      user.flag = "/de-flag.png"
    } else if(user.country === "MX") {
      user.flag = "/mx-flag.png"
    }
    setUser(user);
  }
  return <UserProfile {...user}/>
}

// API.js
// Fetches the data and handles errors on that. That's it
export const fetchUser = async (errorHandler) => {
  try {
    const user = await axios.get('/user/25')
  } catch(error) {
    errorHandler(error)
  }
}

// UserProfile.jsx
// Displays the UserProfile and that's it

const UserProfile = props => {
  const { firstName, lastName, profilePhoto, ...rest} = props
  return (<div {...rest}>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
  </div>)
}

Bonus: If you are using a type-checker, make it count.

In case your team chooses to use a type-checker it's really important that you become strict and use it to ensure it's covering and serving for the purpose it was decided to use it.

Before:

const UserProfile = (props: any) => {
  const { firstName, lastName, profilePhoto, shouldShowPhoto } = props
  return (<div>
    <span>{firstName}</span>
    <span>{lastName}</span>
    <img src={profilePhoto}/>
  </div>)
}

After:

interface IUserProfile {
  firstName: string
  lastName: string
  profilePhoto: string
  shouldShowPhoto?: boolean
}

const UserProfile = (props: IUserProfile) => {
  const { firstName, lastName, profilePhoto, shouldShowPhoto } = props
  return (<div>
    <span>{firstName}</span>
    <span>{lastName}</span>
    {shouldShowPhoto && <img src={profilePhoto}/>}
  </div>)
}

I'm not saying these rules apply for all the projects but your team should be able to define them and agree on that.

Which best practices/guidelines do you use?

Discussion

pic
Editor guide
Collapse
marklai1998 profile image
Mark Lai

For the first Tip
manually order the imports is tedious
I'm using import-sort with husky pre-commit hook
That makes my life much easier, I never need to care about the ordering of it
Just use auto import and commit + push my code, really simple
github.com/renke/import-sort

Collapse
s0xzwasd profile image
Daniil Maslov

Thanks for sharing!

Collapse
adancarrasco profile image
Adán Carrasco Author

This is so cool Mark. What a nice way to do it; automating it!
Thanks for sharing!

Collapse
jkhaui profile image
Jordan Lee

These are great. Another simple tip is to always format your code (and ideally, to one of the widely accept standards, e.g. 2 spaces for indents). Nothing annoys me more than seeing unformatted code - it only takes 2-3 keypresses to fix!

Collapse
adancarrasco profile image
Adán Carrasco Author

This is a really good tip! Totally agree with you.

Collapse
seanmclem profile image
Seanmclem

In #5, you're manually setting user.flag before passing it to setUser. Does that cause any issues with immutablity? Normally I would do something like setUser({...user, flag}), but if I could save myself that step I would like to.

Collapse
mjnlonzame profile image
mjnlonzame

Hello, user.flag is not a state

Collapse
seanmclem profile image
Seanmclem

User is. So isn't user.flag? It's on the user object created with usestate

Collapse
sunnie profile image
sunnie

Hello,In #5 API.js
export const fetchUser = async (errorHandler) => {
try {
const user = await axios.get('/user/25')
} catch(error) {
errorHandler(error)
}
}

Is there need to return ‘user ’?

Collapse
idanen profile image
Idan Entin

A better refactor of #5 would be to extract the logic to a custom hook.
That way it won't be coupled to the UserProfile component

Collapse
adancarrasco profile image
Adán Carrasco Author

Hi Idan, in this case the UserProfilePage component has the logic decoupled from the User component. The User component is a presentational component while your UserProfilePage holds the Business Logic, this follows Atomic Design. bradfrost.com/blog/post/atomic-web...
Please let me know if you think otherwise. :)

Collapse
idanen profile image
Idan Entin

The logic is decoupled, however not reusable.
So if you need the same logic in, say, Comments component, you'd need to write a CommentsPage.
You can avoid that by changing UserProfilePage to useUserProfile and have it return the user instead of JSX.
Small change, with large benefits

Collapse
iammgk profile image
Collapse
s0xzwasd profile image
Daniil Maslov

Thanks for the 4 point, very valuable!