We are running all js code (client & server) through babel so favor writing with ES6 in mind.
var is mutable, and globally scoped. There shouldn't be any use case for using it.
We should always favor immutability. It makes the code safer, and easier to reason about. If you find yourself re-assigning a variable, it's a good indication the code could be written in a more functional manner. For instance, instead of
// bad
var foo
if (n == 1)
foo = "bar"
else
foo = "baz"It could be re-written
// good
const foo = (n == 1) ? "bar" : "baz"There are some instances where it makes sense to re-assign a variable. In those cases, use let, as it's scoped to the nearest function (opposed to var, which is scoped globally)
With ES6, there are now multiple ways to declare functions in javascript. There are slight differences in behavior with different function declarations, so we'll cover that here and when they should be used.
ES6 introduced the concept of an arrow function. That is, the ability to create a function without the use of the function keyword. An arrow function behaves the same as a non-arrow function, with the exception that the function is always bound the context where it's defined, and not where it's called.
For instance, declaring an arrow function:
const sayHi = () => {
console.log("Hi " + this.name)
}Is the equivalent to:
function sayHi() {
console.log("Hi " + this.name)
}.bind(this)For almost every case, you probably want to use an arrow function
// good
setTimeout(() => {
console.log("foo")
}, 100)// good
export const myFunction = (arg1, arg2) => {
console.log(arg1, arg2)
}// good
const makeSquare = (n) => { return n * n }Parentheses are optional for 0 or 1 arguments. Always use parentheses for consistency.
// good
const sayHi = () => { console.log("Hi") }
// good
list.forEach((item) => { /* do something */ })With arrow functions, you can specificy an implicit return if the function has only one execution statement, and the function is not defined with braces. For instance, the following are equivalent
const makeSquare = (n) => { return n * n }
// preferred
const makeSquare = (n) => n * nThis is common use for performing a single operation over a collection
// Sqaure all items in list
list.map(
(item) => item * item
)The function keyword should only be used for creating local (not exported) functions. They should also only be used at the top-level of the file (not inside another function).
// good
function makeSquare(n) {
return n * n
}
makeSquare(2) // 4Avoid using the function keyword without a name (such as a callback) or storing into a variable.
// bad
setTimeout(function() {
console.log("hi")
}, 1000)// bad
const myFunction = function() {
return "hi"
}// bad
export function myFunction() {
...
}When specifying a default export, make the export line the last line in the file.
const MyComponent = () => (
{ /* contents */ }
)
export default MyComponentImports can be specified in any order, but it might be good to have rules around how they should be declared to offer some sanity.
If anyone has any suggestions, that would be great, but I've sort of been doing the following:
// First, 3rd-party dependencies
import React from "react"
import { fromJS } from "immutable"
...
// Next, local functions & components
import * as CartApi from "highline/api/cart_api"
import { formatUrl } from "highline/utils/url"
import CartComponent from "highline/components/cart"
// Next, Redux actions (if applicable)
import {
productDetailAddToCartClicked,
productDetailOptionSelected,
} from "highilne/redux/actions/product_detail_actions"
// Lastly, styles from css
import styles from "highline/styles/style.css"The imported library/function name should match the name & casing of the default export being imported.
// good
import classnames from "classnames"
// bad
import cn from "classnames"
// bad
import classNames from "classnames"Since we're using babel to transpile both client & server code, we can take advantage of the latest javascript features. This means we can write asynchronous code using the async / await syntax instead of Promises.
For instance, instead of writing:
// bad
function callApi() {
return new Promise((resolve, reject) => {
const response = // make ajax call
if (response.success)
resolve(response)
else
reject(response)
}
}
callApi
.then((response) => { console.log("Success", response) })
.catch((error) => { console.log("Error", error) })We can now write our code in a more procedural manner using async / await
// good
async function callApi() {
const response = await // make ajax call
return response
}
try {
const response = await callApi()
console.log("Success", response)
} catch (error) {
console.log("Error", error)
}There's not yet a standard way for awaiting multiple async calls, so the preferred approach (for now) is to use Promise.all (since async is technically just syntactic sugar around promises)
// good
const responses = await Promise.all([apiCall1, apiCall2, apiCall3])
responses.forEach((response) => {
console.log(response)
})Importing react:
import React from "react"isn't required due to our build process, but we should add it to the top of each file that requires React regardless. This will help if files are ever used outside of our build, and also less confusing for people who might not know why it still works.
If you need an instance of a react component (tapping into state or a lifecycle function), use a class that extends React.PureComponent.
exceptions: Components in layouts or admin can be exempt from this rule.
note: PureComponent will implement componentShouldUpdate with a shallow comparison of props, to determine if the render() function should be called. For the most part, this is fine since we are using immutablejs, but this should be kept in mind if the component is not re-rendering when it should.
class MyComponent extends React.PureComponent {
state = {
name: "bonobos",
}
render() {
const { name } = this.state
return (
<span>Hi { name }</span>
)
}
}If you don't need state, or any of React's built in functions, you can use a function to define the component instead of a class. Some benefits to this approach:
- Less overhead (react doesn't need to maintain an instance of a class in memory)
- Simplicity (easier to read, reason about since there's not instance or state based logic)
To create a stateless functional component, define a function that takes props, and returns a React (or DOM) element
const myComponent = ({
name,
}) => (
<span>{ name }</span>
)
export default myComponentA common pattern is to have an instance function on a component that is bound to that instance. By declaring the function using the arrow () => {} syntax, the function will be bound to the created instance. This avoids the need to bind the function explicitly.
class MyComponent extends React.PureComponent {
constructor(props) {
super(props)
// bad
this.myFunction = this.myFunction.bind(this)
}
// good
myFunction = () => {
this.setState({ ... })
}
render() {
return (
<OtherComponent
onClick={ this.myFunction } // good
onChange={ myFunction.bind(this) } // bad
/>
)
}
}Order functions in the same order that the react-lifeccyle happens. That is:
optional static methods
constructor
getChildContext
componentWillMount
componentDidMount
componentWillReceiveProps
shouldComponentUpdate
componentWillUpdate
componentDidUpdate
componentWillUnmount
... custom methods here ...
render
class MyComponent extends React.Component {
static propTypes { ... }
constructor() { ... }
componentWillMount() { ... }
componentDidMount() { ... }
onButtonClick = () => { ... }
render() { ... }
}When declaring props (either in propTypes or on a Component, always sort alphabetically)
<MyComponent
name="bonobos"
onClick={ onClick }
zIndex={ 20 }
/>Arguments to functions are not declared as const, so destructring the props in the render method is useful for forcing immutability, but also help with checking against typos and using undeclared variables.
// good
render() {
const {
name,
onClick,
zIndex,
} = this.props
return (
<MyComponent
name={ name }
onClick={ onClick }
zIndex={ zIndex }
/>
)
}Referencing this.props or this.state directly in render can lead to calling undefined (or misspelled) props. By pulling into a variable, it's clear which variables are in scope (and used) within the render function.
// bad
render() {
return (
<MyComponent
name={ this.props.name }
onClick={ this.props.onClick }
zIndex={ this.props.zIndex }
/>
)
}// components/shared/my_component.js
export default myComponentIf a component is a basic building block (not specific to any feature), it should live in the top level of the components directory.
/* good */
highline/components/button.js
highline/components/input.js
highline/components/input_with_label.js
highline/components/product_detail/options.js
highline/components/product_detail/product_summary.jsWhen exposing a prop that gets called based on some action (onClick, onChange etc..), use the on prefix when defining the prop in a component. Use handle when defining the prop in a container.
// good
<MyComponent
onClick={ this.componentClicked }
onChange={ this.componentChanged }
/>
const mapDispatchToProps = {
handleInputChange() {
dispatch(somethingChanged())
}
}
// bad
<MyComponent
clicked={ this.componentClicked }
handleChange={ this.componentChanged }
/>All actions that get dispatched should be declared in the past tense. That is, they should represent the result of some action that has already happened. If you find yourself naming an action for something that should happen, think about what is the actual thing that is triggering the need for that action.
// good
PRODUCT_FETCH_STARTED
PRODUCT_FETCH_FAILED
PRODUCT_FETCH_SUCCEEDED
CART_OPEN_CLICKED
// bad
FETCH_NEW_PRODUCTS
OPEN_CART
OPEN_MODALWe have a specific naming convention for actions that correspond to different states of the HTTP request cycle. For each asynchronous action that makes an API call, there are be 4 actions that correspond the each phase of the request
_STARTED // before request is sent
_SUCCEEEDED // on request success
_FAILED // on request error
_COMPLETED // after request done (regardless of success/error) There are two types actions that can be dispatched to redux. Those are action creators (return an Object), and action thunks or, "async" actions (return a function).
Action Creators are just functions that return a plain javascript Object, that has a type field. Convention is to use the action type prefix in the name.
// good
export const productDetailAddToCartClicked = (sku) => ({
type: ActionTypes.PRODUCT_DETAIL_ADD_TO_CART_CLICKED,
sku,
})
// bad
export const addToCartClicked = (sku) => ({
type: ActionTypes.PRODUCT_DETAIL_ADD_TO_CART_CLICKED,
sku,
})Async actions are functions that return a function to be evaulated at a later time. It doesn't necessarily mean there needs to be some type of i/o operation, but just that there is some additional logic based on the application state to be performed. Async actions can also dispatch other actions. The action type prefix is not needed, since there isn't a 1-to-1 correlation of action & action type
(but maybe we should keep the prefix for consistency?)
export const addToCartAsync = (sku) => (
(getState, dispatch) => {
dispatch(productDetailAddToCartStarted())
...
return dispatch(productDetailAddToCartSuccess())
}
)