React clean code, coding style guide, and best practice

react-clean-code.md

General

Git

• Use ~/.gitignore to ignore files related to your environment / editor.
• Use present tense in commits "upgrade dependencies" or "introduce custom error object"
• Use [Conventional Commits][conventional-commits-url]:

feat: add hat wobble #227
^--^  ^------------^ ^--^
|     |              |
|     |              +-> Issue number
|     |
|     +-> Summary in present tense.
|
+-------> Type: chore, docs, feat, fix, refactor, style, or test.

Git naming reference

• Changes to projects are achieved through issues and pull requests (PRs).
• Issues are created first to describe the change that is needed (add a new endpoint to an API, fix a bug, refactor a module, cleanup, etc...).
• Next, a PR is opened to address the issue. Prefix the git branch name with the issue number (e.g. 197-add-get-publisher).
• Pull request title should state the goal of pull request clearly.
• Pull request description should describe how the PR will achieve that goal.
• If PR contains UI changes or additions, please include screenshots or animated gifs showing the change. (e.g. use https://getkap.co/ on OSX)
• In the description, use the phrase "Closes #XX" (where XX is the issue number).
• Each PR should be as small and simple as possible to prevent bugs and make code review as quick as possible. Do not include unrelated changes. Open a separate issue/PR for those.
• When you want specific persons to review your pull request, mention them using @ syntax in the end of the description (e.g. "@iqbalnovramadani, please review.")
• You may use as many commits as you would like while working on a pull request. However, before it can be merged it must be squashed to a single commit. This commit should have either the issue or PR number added to the end (e.g. "feat: add get publisher endpoint (#197)"). This way it is easy to find the context for a change in the git log.

Front-end

Library

• Utility: JavaScript ES6 + Lodash
• Styling: Styled Components + CSS Modules
• Asynchronous Requests: axios
• Formatting: StandardJS
• Type Checking: PropTypes
• State Management: React's local state and Redux (with Redux-Thunk and Redux-Saga)
• Routing: React Router
• UI Components: React-bootstrap
• Internationalization: React-Intl
• Time: moment.js or date-fns
• Testing: Jest with Enzyme, Cypress

Project structure

Below is the guideline for project structure.

.babelrc
.env
cypress.json
doczrc.js
package.json
package-lock.json
README.md
/node_modules
/public
/cypress
  utils.js (all testing helper functions here)
  /examples (all example generated code from cypress put here)
  /fixtures
  /integration (test suites here)
  /plugins
  /support
/src
  /api
    request.js (axios wrapper for easy api call)
    user.js
    page.js
    ...
  /assets
    /images
  /components (all stateless components goes here)
    /Button
      Button.mdx (documentation)
      StyledComponents (component styling)
      index.js
      Button.test (jest/enzyme test)
    /Input
    /...
  /constants
    /messages (all paragraph, dialog, popup, or toast message for users (separate per language))
      ar.js
      en.js
      id.js
    /labels (all text in button, dropdown, etc (separate per language))
      ar.js
      en.js
      id.js
  /containers (all stateful components goes here)
    /Homepage
      index.js
      reducer.js
      selector.js (redux selector e.g. using reselect)
      actions.js
      constants.js (constants for redux actions)
      /components
        /Sidenav (follow above pattern for components)
          Sidenav.mdx
          StyledComponents
          index.js
          Sidenav.test (jest/enzyme test)
          /components
            /...
      /...
  /utils
    history.js (createBrowserHistory)
    index.js (all helper functions goes here)
  /styles
    index.css (don't style without specific classname here!)
  globalReducer.js
  index.js (entry point)
  store.js (Redux store)



// next.js project
project
  components
  containers
  pages
  ... follow above pattern

Code style

This repository uses standard to maintain code style and consistency and avoid style arguments. npm start should also run standard so that it will fail to start when style does not meet the requirements.

Other style points:

• Code should be simple and explicit.
• Callbacks are preferred over Promises.
• Functional style is preferred to OOP.
• Prefer small modules that do one thing well and avoid large frameworks.
• If a module exports a single function use module.exports = ... not module.exports.thing = ....
• Line length should be limited to 80 characters.
• When writing a module use this order:
  • External dependencies (e.g. import qs from 'query-string')
  • Internal dependencies (e.g. import api from './api')
  • Any declarations or actions absolutely needed before module.exports
  • module.exports
  • Everything else should be ordered "high-level" to "low-level" and "first-use" to "last-use" -- low-level helper functions should be at the bottom.
• Use descriptive variable names.
• Code and comments should stay within 80 character line length.
• Prefer .forEach() over for loops.
• Default name for a callback is cb.
• Functions should generally not accept more than 3-4 arguments, consider using an "options" object instead.

function foo (a, b, c, d, cb) { cb(a, b, c, d) } // wrong
function foo (opts, cb) { cb(opts.a, opts.b, opts.c, opts.d) } // right

• Use single line conditionals when possible.
• Use early returns when possible.

function getAsyncThing (cb) {
    someAsyncWork(function (err, result) {
      if (err) return cb(err)
      if (!result) return cb(new Error('No result found.'))
      cb(null, result)
    })
}

• Destructure used properties at the top most, with this kind of pattern:

// wrong
handleClickApply = () => {
  this.submitResponse(this.state.name, this.state.email, this.state.phone)
  this.props.onClick()
}

// right
handleClickApply = () => {
  const { onClick } = this.props
  const {
    name,
    email,
    phone
  } = this.state
  
  this.submitResponse(name, email, phone)
  onClick()
}

• Give upper most declarations (usually object destructuring and local vars) separate new line
• Give if expression / conditional expression separate new line

handleShowToast = () => {
  let amount = 0
  const { users, onShowToast } = this.props
                                              // empty line here
  const ids = users.map(user => user.uuid)
  amount = ids.length  
                                              // empty line here
  if (amount > 0) {
    onShowToast()
  }
}

• Give empty line between this kind of destructuring

  import React from 'react'
  import { pure } from 'recompose'
                                      // empty line here
  import {
    Button,
    Input,
    TextArea
  } from 'components'
                                      // empty line here
  import {
    SCREEN_ADD,
    SCREEN_DELETE,
    SCREEN_MOVE
  } from 'constants'

  ....

  const { 
    isLoading, 
    messages, 
    receiving,
    receivingChannelId
  } = this.state
                                      // empty line here
  const {
    talking,
    toast, 
    topic 
  } = this.props

• Put react render method on top most after state declaration

class HomePage extends PureComponents {
  state = {
    user: null,
    page: 1
  }
  
  render () {
    <div>
      ...
    </div>
  }
  
  // life cycle methods here
  componentDidMount () {

  }
  
  // other methods here
  handleClick = () => {
    ...
  }
}

• Use fat arrow function assigned to class property instead of common function for UI Component methods
• Don't bind function on render method and use inline style, bad for performance

class Navbar extends PureComponent {
  state = {
    menuId: null
  }

  render () {
    <div>
      <button onClick={this.handleClick}>
        Credits
      </button>
      <button
        onClick={this.handleClick2.bind(this)}  // very wrong!!! 
        style={{ borderColor: 'red' }}          // performance suffer because of these!
      >
        Credits2
      </button> 
    </div>
  }

  // wrong
  handleClick2 (e) {
    const { onClick } = this.props
    const { menuId } = this.state

    onClick(e, menuId)
  }

  // right
  handleClick = (e) => {
    const { onClick } = this.props
    const { menuId } = this.state

    onClick(e, menuId)
  }
}

• Function declaration should have a space before parentheses, but function call should not

// wrong
sortArray(array) {
  array.sort ()  
}

// right
sortArray (array) {
  array.sort()
}

• Sort constants, props alphabetically, wherever possible
• Sort value props then handler props
• Embed "on" in front of handler props name to distinguish it with value props
• Embed "is"/"has"/"should" for boolean variable/props
• When stateless component has a lot of props, destructure props inside the function

// wrong
import {
  SCREEN_DELETE,
  USER_ADD,
  SCREEN_MOVE,
  SCREEN_ADD
} from './constants'

const Screen = (props) => {
  const {
    addScreenText,
    screenAdd,
    addUserText,
    userAdd,
    deleteScreenText,
    screenDelete,
    moveScreenText,
    screenMove,
    show
  } = props

  return (
    show &&
    <div>
      <button onClick={userAdd}>
        {addUserText}
      </button>
      <button onClick={screenAdd}>
        {addScreenText}
      </button>
      <button onClick={screenDelete}>
        {deleteScreenText}
      </button>
      <button onClick={screenMove}>
        {moveScreenText}
      </button>
    </div>
  )
}

// right
import {
  SCREEN_ADD,
  SCREEN_DELETE,
  SCREEN_MOVE,
  USER_ADD
} from './constants'

const Screen = (props) => {
  const {
    addScreenText,      // value props first
    addUserText,
    deleteScreenText,
    isShow,
    moveScreenText,
    onScreenAdd,        // then handler props
    onScreenDelete,
    onScreenMove,
    onUserAdd
  } = props

  return (
    isShow &&
    <div>
      <button onClick={onUserAdd}>
        {addUserText}
      </button>
      <button onClick={onScreenAdd}>
        {addScreenText}
      </button>
      <button onClick={onScreenDelete}>
        {deleteScreenText}
      </button>
      <button onClick={onScreenMove}>
        {moveScreenText}
      </button>
    </div>
  )
}

• When stateless component has few props, destructure props on the function declaration

const buttonStyle = {
  color: 'blue'
}

// wrong
const BlueButton = (props) => (
  <button onClick={props.onClick} style={buttonStyle}>
    {props.children}
  </button>
)

// right
const BlueButton = ({ children, onClick }) => (
  <button 
    onClick={onClick}     // make it horizontally concise and spread vertically,
    style={buttonStyle}   // easier to read
  >
    {children}
  </button>
)

• Always use PureComponent for stateful component (container), for stateless component wrap with recompose pure function to ensure best performance

// WRONG
// Button.js
import React, { Component } from 'react'

class Button extends Component {
  ...
}

export default Button

// Input.js
import React from 'react'

const Input = (props) => (
  <input ... />
)

export default Input



// RIGHT
import React, { PureComponent } from 'react'

class Button extends PureComponent {
  ...
}

export default Button

// Input.js
import React from 'react'
import { pure } from 'recompose'

const Input = (props) => (
  <input ... />
)

export default pure(Input)

• Make code horizontally concise and spread vertically

// wrong
handleClickScreenLink = (link) => {
  // spread it vertically!
  const { hasUserUpdatePrevilege, isAutomaticallyUpdate, isEditable, isShown } = link
  if (hasUserUpdatePrevilege && isAutomaticallyUpdate && isEditable && isShown) {
    screenLinks = this.changeScreenLinkColor( link, SCREEN_LINK_ACTIVE_COLOR, true, screenLinks)
    this.setState({ screenLinks: screenLinks }) // redundant, use ES6 same property name assignment feature!
  }

}

// right
handleClickScreenLink = (link) => {
  const {
    hasUserUpdatePrevilege,
    isAutomaticallyUpdate,
    isEditable,
    isShown
  } = link

  if ( hasUserUpdatePrevilege &&
       isAutomaticallyUpdate &&
       isEditable &&
       isShown )
  {
    screenLinks = this.changeScreenLinkColor(
      link,
      SCREEN_LINK_ACTIVE_COLOR,
      true,
      screenLinks
    )

    this.setState({ screenLinks })
  }
}

• Gather all related functions close, with called functions below the function which is calling them
• Called functions should be declared in correct order as called

// wrong
// ... other methods here

updateContextMenuScreenHandlers = () => {
  ...
}

// ... other methods here

updateMenuAddTranslation = () => {
  ...
}

// ... other methods here

updateMenuConnectToScreen = () => {
  ...
}

// ... other methods here

updateMenuConnectScreen = () => {
  ...
}

// ... other methods here

generateContextMenuScreen = () => {
  this.updateMenuAddTranslation()
  this.updateMenuConnectToScreen(screenList)
  this.updateMenuConnectScreen(selectedScreen, screen, screenList)
  const handlers = this.updateContextMenuScreenHandlers(screenList, screen, selectedScreen)

  return handlers
}

// right
// ... other methods here

generateContextMenuScreen = () => {
  this.updateMenuAddTranslation()
  this.updateMenuConnectToScreen(screenList)
  this.updateMenuConnectScreen(selectedScreen, screen, screenList)
  const handlers = this.updateContextMenuScreenHandlers(screenList, screen, selectedScreen)

  return handlers
}

updateMenuAddTranslation = () => {
  ...
}

updateMenuConnectToScreen = () => {
  ...
}

updateMenuConnectScreen = () => {
  ...
}

updateContextMenuScreenHandlers = () => {
  ...
}

// ... other methods here