Name	Name	Last commit message	Last commit date
parent directory ..
README.md	README.md

Eventbrite React & JSX Coding Style Guide

ESLint rules, guidelines and best practices used by Eventbrite to provide consistency and prevent errors in React and JSX code.

What is React?
What is JSX?
General rules
Component files
Component class
Component organization
Component reference naming
Component propTypes
Helper components
Component methods
JSX wrapping
JSX alignment
JSX attribute values
React key prop
Event handlers
Refs
Dangerous props
render()
State

What is React?

React is a JavaScript-centric UI library. It abstracts away the DOM, giving a simpler programming model and better performance. It can also render on the server using Node, and can even power native apps using React Native. React implements one-way reactive data flow which reduces boilerplate and is easier to reason about than traditional data binding.

For more info, see Getting started with React.

⬆ back to top

What is JSX?

JSX is a JavaScript syntax extension that looks similar to XML. We use it with React because it is a concise and familiar syntax for defining tree structures with attributes.

For more info, see JSX in Depth.

⬆ back to top

General rules

Always use JSX syntax (don't use React.createElement):

// good
render() {
    return (
        <Component value="foo">
            <ChildComponent />
        </Component>
    );
}

// bad (why torture yourself with React.createElement?)
render() {
    return React.createElement(
        Component,
        { value: "foo" },
        React.createElement(ChildComponent, null)
    );
}

⬆ back to top

Component files

Use PascalCase for React component names, e.g. TextInput
Use .js extension for React components
Use the component name for filenames. E.g., TextInput.js

⬆ back to top

Component class

Class style

Prefer ES6 classes over React.createClass (eslint: react/prefer-es6-class):

// good
export default class MainComponent extends React.Component {

}

// bad (uses React.createClass)
const MainComponent = React.createClass({

});
export default MainComponent

NOTE: There is a common practice to use stateless/pure functions over ES6 classes for components without any internal state (eslint: react/prefer-stateless-function), but we are choosing to stick with ES6 classes for a few reasons:

When the component does more than just render props, such as attaching callback handlers, the stateless function can become unnecessarily complex with nested functions
propTypes are defined within the ES6 class, but have to be defined as additional properties outside of a stateless function
Using ES6 classes for the main/default component help differentiate it from helper components

⬆ back to top

`displayName`

Do not use displayName for naming components. Instead, name the class expression. React will infer the displayName from the reference name:

// good
export default class TextInput extends React.Component {
}

// ok (uses class expression assigned to a named const reference)
const TextInput = class extends React.Component {
};

// bad (missing name of `class` expression)
export default class extends React.Component {
}

// bad (uses `displayName` instead of `class` name)
export default class extends React.Component {
    static displayName = 'TextInput';
}

Component organization

Export only one component per file as the default (eslint: react/no-multi-comp)

// MainComponent.js

// good
export default class MainComponent extends React.Component {

}


// bad (exports multiple components)
export class MainComponent extends React.Component {

}
export class OtherComponent extends React.Component {

}

⬆ back to top

Component reference naming

Use PascalCase for React components and camelCase for their instances (eslint: react/jsx-pascal-case).

Components:

// good
import TextInput from './atoms/TextInput';

// bad (uses camelCase for component reference)
import textInput from './atoms/TextInput';

Component instances:

// good
let emailField = (<TextInput name="email" />);

// bad (uses PascalCase for component instances)
let EmailField = (<TextInput name="email" />);

⬆ back to top

Component `propTypes`

Defining `propTypes`

Use static class property syntax to define propTypes and defaultProps:

// good
export default class TextInput extends React.Component {
    static propTypes = {
        type: React.PropTypes.string,
        defaultValue: React.PropTypes.string
    }

    static defaultProps = {
        type: 'text',
        defaultValue: ''
    }
}

// bad (adds `propTypes` & `defaultProps` after class definition)
const TextInput = class extends React.Component {
    static propTypes = {
        type: React.PropTypes.string,
        defaultValue: React.PropTypes.string
    }
};

TextInput.propTypes = {
    type: React.PropTypes.string,
    defaultValue: React.PropTypes.string
};
TextInput.defaultProps = {
    type: 'text',
    defaultValue: ''
};

export default TextInput;

NOTE: Static class properties are not a part of the ES2015 specification and are a in the midst of the ECMAScript proposal approval process. Currently they are sitting in Stage 1. For all proposed features, check out ECMAScript proposals.

⬆ back to top

Props naming

Use camelCase for propTypes (eslint: camelcase):

// good
export default class TextInput extends React.Component {
    static propTypes = {
        type: React.PropTypes.string,
        defaultValue: React.PropTypes.string,
        onChange: React.PropTypes.func,
        onFocus: React.PropTypes.func,
        onBlur: React.PropTypes.func
    }
}

// bad (uses non-camelCase)
export default class TextInput extends React.Component {
    static propTypes = {
        type: React.PropTypes.string,
        default_value: React.PropTypes.string,
        OnChange: React.PropTypes.func,
        On_Focus: React.PropTypes.func,
        on_Blur: React.PropTypes.func
    }
}

⬆ back to top

Required props

Don't mark any of the propTypes as required if they are included in defaultProps or are boolean values that (implicitly) default to false:

// good
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func.isRequired,
        type: React.PropTypes.string,
        required: React.PropTypes.bool,
        defaultValue: React.PropTypes.string,
        role: React.PropTypes.string
    }

    static defaultProps = {
        type: 'text',
        defaultValue: ''
    }
}

// bad (`type` is marked as required even though it's defaulted &
// `required` is marked as required even though it's boolean w/ `false` default)
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func.isRequired,
        type: React.PropTypes.string.isRequired,
        required: React.PropTypes.bool.isRequired,
        defaultValue: React.PropTypes.string,
        role: React.PropTypes.string
    }

    static defaultProps = {
        type: 'text',
        defaultValue: ''
    }
}

⬆ back to top

`propTypes` Ordering

Define required propTypes first to make it clear what the set of minimum props are needed to use the component:

// good
export default class TextInput extends React.Component {
    static propTypes = {
        role: React.PropTypes.string.isRequired,
        onChange: React.PropTypes.func.isRequired,

        type: React.PropTypes.string,
        required: React.PropTypes.bool,
        defaultValue: React.PropTypes.string
    }

    static defaultProps = {
        type: 'text',
        defaultValue: '',
    }
}

// bad (required props are not first)
export default class TextInput extends React.Component {
    static propTypes = {
        type: React.PropTypes.string,
        required: React.PropTypes.bool,
        defaultValue: React.PropTypes.string,
        role: React.PropTypes.string.isRequired,
        onChange: React.PropTypes.func.isRequired
    }

    static defaultProps = {
        type: 'text',
        defaultValue: '',
    }
}

⬆ back to top

Vague `propTypes`

Don't use the vague prop types, React.PropTypes.any, React.PropTypes.array, and React.PropTypes.object, and instead be more explicit using, React.PropTypes.oneOfType, React.PropTypes.arrayOf, and React.PropTypes.shape (eslint react/forbid-prop-types):

// good
export default class Candidate extends React.Component {
    static propTypes = {
        id: React.PropTypes.oneOfType([
            React.PropTypes.number,
            React.PropTypes.string
        ]),
        names: React.PropTypes.arrayOf(
            React.PropTypes.string
        ),
        person: React.PropTypes.shape({
            name: React.PropTypes.string,
            age: React.PropTypes.number
        })
    }
}

// bad (uses vague prop types)
export default class Candidate extends React.Component {
    static propTypes = {
        id: React.PropTypes.any,
        names: React.PropTypes.array,
        person: React.PropTypes.object
    }
}

⬆ back to top

Helper components

When a component contains a lot of markup or it contains significant logic that determines how its markup should appear, use helper components to keep render() as small as possible. Instead of using class declarations for these helper components, use stateless functions. Because these components are only useful to the main component and only exist to keep render() lean, these helper components shouldn't be placed in their own files, nor should they be exported within the main component.

Let's look at a simple example. Let's say you had a global footer that contained a section with links to important top-level pages, another section with a bunch of links for SEO (😐), a section of links to all of your top-level domains (.com, .co.uk, etc), and a final section of links to all of your social media accounts.

Properly using helper components, this would look like:

// good (clean render w/ help of helper components)

// using arrow functions for stateless functions
const SiteLinks = () => (
    <ul className="global-footer__site-links">
        <li><a href="/about">About</a></li>
        <li><a href="/blog">Blog</a></li>
        <li><a href="/help">Help</a></li>
        <li><a href="/careers">Careers</a></li>
    </ul>
);

// arrow function takes in props object
const SeoLinks = (props) => {
    let linkItems = props.links.map((link) => (
        <li><a key={link.url} href={link.url}>{link.label}</a></li>
    ));

    return (
        <ul className="global-footer__seo-links">
            {linkItems}
        </ul>
    );
};

// arrow function immediately destructures props object into
// `allTlds` and `currentTld`
const DomainLinks = ({allTlds, currentTld}) => {
    // use destructuring to immediately pull out `allTlds` & `currentTld`

    // filter out the current tld and then map to <li>
    let domainItems = allTlds.filter((tldInfo) => tldInfo.tld !== currentTld)
        .map(({tld, url, label}) => (
            <li><a key={tld} href={url}>{label}</a></li>
        ));

    return (
        <ul className="global-footer__domain-links">
            {domainItems}
        </ul>
    );
};

// arrow function immediately destructures props object into `links`
const SocialLinks = ({links}) => {
    // Return null to indicate you want nothing rendered
    // Returning `undefined` will cause a render error
    if (!links) {
        return null;
    }

    let socialItems = links.map(({url, label}) => (
        <li><a key={url} href={url}>{label}</a></li>
    ));

    return (
        <ul className="global-footer__social-links">
            {socialItems}
        </ul>
    );
};

export default class GlobalFooter extends React.Component {
    static propType = {
        allTlds: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                tld: React.PropTypes.string.isRequired,
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        ).isRequired,
        currentTld: React.PropTypes.string.isRequired,
        seoLinks: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        ).isRequired,

        // socialLinks are optional
        socialLinks: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        )
    }

    render() {
        let {allTlds, currentTld, seoLinks, socialLinks} = this.props;

        return (
            <div className="global-footer">
                <SiteLinks />
                <SeoLinks links={seoLinks} />
                <DomainLinks allTlds={allTlds} currentTld={currentTld} />
                <SocialLinks links={socialLinks} />
            </div>
        );
    }
}

As you can see, with this best practice, the render() of GlobalFooter is really clean. It's immediately obvious that the global footer consists of site, SEO, domain and social links. The GlobalFooter is composed of these helper components in true React style. Furthermore, if more content is needed for a given section, it's easy for the developer to know where to add code, and render() of GlobalFooter doesn't need to grow.

Let's take a look at the "bad" approach:

// bad (longer, less maintainable render)
export default class GlobalFooter extends React.Component {
    static propType = {
        allTlds: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                tld: React.PropTypes.string.isRequired,
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        ).isRequired,
        currentTld: React.PropTypes.string.isRequired,
        seoLinks: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        ).isRequired,

        // socialLinks are optional
        socialLinks: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                url: React.PropTypes.string.isRequired,
                label: React.PropTypes.string.isRequired
            })
        )
    }

    render() {
        let {allTlds, currentTld, seoLinks, socialLinks} = this.props;
        let seoItems = seoLinks.map((link) => (
            <li><a key={link.url} href={link.url}>{link.label}</a></li>
        ));
        let domainItems = allTlds.filter((tldInfo) => tldInfo.tld !== currentTld)
            .map(({tld, url, label}) => (
                <li><a key={tld} href={url}>{label}</a></li>
            ));
        let socialItems;

        if (socialLinks) {
            socialItems = socialLinks.map(({url, label}) => (
                <li><a key={url} href={url}>{label}</a></li>
            ));
        }

        return (
            <div className="global-footer">
                <ul className="global-footer__site-links">
                    <li><a href="/about">About</a></li>
                    <li><a href="/blog">Blog</a></li>
                    <li><a href="/help">Help</a></li>
                    <li><a href="/careers">Careers</a></li>
                </ul>
                <ul className="global-footer__seo-links">
                    {seoItems}
                </ul>
                <ul className="global-footer__domain-links">
                    {domainItems}
                </ul>
                <ul className="global-footer__social-links">
                    {socialItems}
                </ul>
            );
            </div>
        );
    }
}

So why is this code "bad" when it looks like less code? In it's current state, there's actually nothing inherently wrong with the code. If the GlobalFooter was going to remain in this state forever, then this code would be just fine. The use of BEM-style CSS classes plus separating logic from of the JSX make render() pretty readable in its current state.

However, as we all know code has entropy; over time it'll change as new functionality is added and other is removed. If the markup becomes more than a simple unordered list or the logic determining what should be rendered becomes more complicated, having everything mixed together in render() will slowly become unwieldy. A code refactor would be needed, but with everything intertwined that could prove to be a big challenge.

Better to start the code on the right foot.

⬆ back to top

Component methods

Private helper methods

JavaScript doesn't (yet) have a mechanism for declaring a method as private, which would only allow the class to call the method. As a quick signal that a React component method helper is private prefix it with underscore (_):

// good
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(value);
        }
    }

    render() {
        return (
            <input type="text" onChange={this._handleChange.bind(this)} />
        );
    }
}

// bad (private method does not start with `_`)
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func
    }

    handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(value);
        }
    }

    render() {
        return (
            <input type="text" onChange={this._handleChange.bind(this)} />
        );
    }
}

⬆ back to top

Method ordering

For consistency and ease of finding methods within a React component, the order of methods should be as follows (eslint react/sort-comp):

propTypes
contextTypes
childContextTypes
defaultProps
state
getChildContext
componentWillMount
componentDidMount
componentWillReceiveProps
shouldComponentUpdate
componentWillUpdate
componentDidUpdate
componentWillUnmount
event handlers (like _handleChange)
getters called from render (like _getType())
helper render methods (like _renderSubHeading())
render

⬆ back to top

JSX Wrapping

Always wrap JSX tags in parentheses as a signal that we're transitioning from vanilla JavaScript to JSX (eslint: react/wrap-multilines):

// good (multi-line JSX wrapped in parentheses)
render() {
    return (
        <Component value="foo">
            <ChildComponent />
        </Component>
    );
}

// good (single-line JSX is also wrapped in parentheses)
let content = (<div>Content</div>);

// bad (missing wrapping parentheses for multi-line)
render() {
    return <Component value="foo">
        <ChildComponent />
    </Component>;
}

// bad (missing wrapping parentheses for single-line)
let content = <div>Content</div>

⬆ back to top

JSX alignment

Single-line

When a component has three props or less with no content, the tag can be on a single line (eslint: react/jsx-max-props-per-line):

// good
<TextInput type="email" name="email" />

// not-so-good (attributes are on the long side)
<TextInput type="email" name="really-long-email-name" placeholder="Enter in your email here please" />

// bad (more than 3 attributes)
<TextInput type="email" name="email" id="email" tabIndex="0" />

⬆ back to top

Multi-line

However, if the props are too long or there are more than three props, the JSX attributes should each be on their own line (eslint: react/jsx-first-prop-new-line):

// good
<TextInput
    type="email"
    name="email"
    placeholder="Enter in your email"
/>

// bad (first attribute is on the same line)
<TextInput type="email"
    name="email"
    placeholder="Enter in your email"
/>

// bad (two attributes on the same line when others are multi-line)
<TextInput
    type="email" name="email"
    placeholder="Enter in your email"
/>

⬆ back to top

Indentation

JSX attributes must be indented four spaces (eslint: react/jsx-indent):

// good
<TextInput
    type="email"
    name="email"
    id="email"
    placeholder="Enter in your email"
/>

// bad (attributes aren't indented 4 spaces)
<TextInput
type="email"
name="email"
id="email"
placeholder="Enter in your email"
/>

⬆ back to top

Closing bracket

The closing bracket should be aligned with the opening tag (eslint: react/jsx-closing-bracket-location):

// good
<TextInput
    type="email"
    name="email"
    id="email"
    placeholder="Enter in your email"
/>
<Button type="submit">
    Go!
</Button>

// bad (self-closing isn't aligned with opening tag)
<TextInput
    type="email"
    name="email"
    id="email"
    placeholder="Enter in your email" />

// bad (closing tag isn't aligned with opening tag)
<Button type="submit">
    Go!</Button>

⬆ back to top

Self-closing

If the component has no content, the JSX tag should be self-closing (eslint: react/self-closing-comp) with a space before the self-closing tag (eslint: react/jsx-space-before-closing):

// good
<TextInput type="email" name="email" htmlFor="email" />

// bad (missing space before self-closing tag)
<TextInput type="email" name="email" htmlFor="email"/>

// bad (too much spacing before self-closing tag)
<TextInput type="email" name="email" htmlFor="email"      />

// bad (not self-closing w/ no content)
<TextInput type="email" name="email" htmlFor="email"></TextInput>

⬆ back to top

JSX attribute values

Quoting

Always use double quotes (") for JSX attribute values (eslint: jsx-quotes):

// good
<TextInput type="email" name="email" htmlFor="email" />

// bad (uses single quotes instead of double quotes)
<TextInput type='email' name='email' htmlFor='email' />

⬆ back to top

Boolean values

A true prop value must be explicitly specified as the attribute value (eslint react/jsx-boolean-value):

// good
<TextInput type="email" required={true} />

// bad (missing explicit `true` value)
<TextInput type="email" required />

⬆ back to top

Curly brace padding

When passing a variable to a prop, the curly braces should not be padded by spaces (eslint: react/jsx-curly-spacing) and neither should the equals (eslint: react/jsx-equals-spacing):

// good
<TextInput defaultValue={value} />

// bad (padding within curly braces)
<TextInput defaultValue={ value } />

// bad (padding around equals)
<TextInput defaultValue = {value} />

⬆ back to top

React `key` prop

Mandatory `key` prop

When rendering an array of React components, specify the key prop for each component in the array to aid React's Virtual DOM diffing algorithm (eslint: react/jsx-key):

// good
export default class NamesList extends React.Component {
    static propTypes = {
        names: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                id: React.PropTypes.string,
                name: React.PropTypes.string
            })
        ).isRequired
    }

    render() {
        let {names} = this.props;
        let nameItems = names.map(({id, name}) => (
            <NameItem key={id} name={name} />
        ));

        return (
            <div>{nameItems}</div>
        );
    }
}

// bad (fails to specify `key` prop in loop)
export default class NamesList extends React.Component {
    static propTypes = {
        names: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                id: React.PropTypes.string,
                name: React.PropTypes.string
            })
        ).isRequired
    }

    render() {
        let {names} = this.props;
        let nameItems = names.map(({id, name}) => (
            <NameItem name={name} />
        ));

        return (
            <div>{nameItems}</div>
        );
    }
}

Not specifying key will not only be an ESLint error, but React will also display a warning messaging in the console complaining that it's missing. It really is critical for performance. For more info, see: Multiple Components | Dynamic Children.

⬆ back to top

Index as `key`

Avoid passing the loop iteration index as the key prop, as this ends up confusing React's Virtual DOM diffing algorithm. Instead, always use an id or something else that uniquely identifies each item in the array:

// good
export default class NamesList extends React.Component {
    static propTypes = {
        names: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                id: React.PropTypes.string,
                name: React.PropTypes.string
            })
        ).isRequired
    }

    render() {
        let {names} = this.props;
        let nameItems = names.map(({id, name}) => (
            <NameItem key={id} name={name} />
        ));

        return (
            <div>{nameItems}</div>
        );
    }
}

// bad (uses array index as a key)
export default class NamesList extends React.Component {
    static propTypes = {
        names: React.PropTypes.arrayOf(
            React.PropTypes.shape({
                id: React.PropTypes.string,
                name: React.PropTypes.string
            })
        ).isRequired
    }

    render() {
        let {names} = this.props;
        let nameItems = names.map(({name}, nameIndex) => (
            <NameItem key={nameIndex} name={name} />
        ));

        return (
            <div>{nameItems}</div>
        );
    }
}

Keep in mind that the data uniquely identifies each item in the array could be the item itself in the case of an array of strings or numbers. For more info, see: Index as key is an anti-pattern.

⬆ back to top

Event handlers

Event handler naming

React doesn't support two-way binding. Parent components can update their children by passing props. Child components can communicate with their parent by calling callback functions (also passed by parents to children via props). Prefix the name of the props with on and the internal method handler with _handle (eslint: react/jsx-handler-names):

// good
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(value);
        }
    }

    render() {
        return (
            <input type="text" onChange={this._handleChange.bind(this)} />
        );
    }
}

// bad (callback prop is not prefixed with `on`)
export default class TextInput extends React.Component {
    static propTypes = {
        // this should be named `onChange`
        change: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.change) {
            this.props.change(value);
        }
    }

    render() {
        return (
            <input type="text" onChange={this._handleChange.bind(this)} />
        );
    }
}

// bad (event handler is not prefixed with `_handle`)
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func
    }

    _onChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(value);
        }
    }

    render() {
        return (
            <input type="text" onChange={this._onChange.bind(this)} />
        );
    }
}

⬆ back to top

Semantic event handler names

If you are defining an event handler prop that is wrapping a DOM event, you should name your prop semantically without tying it to the underlying DOM event that triggered it.

For example, let's say you have a pagination component (Pagination) that contains a bunch child Button components for each page (1, 2, 3, etc). When clicking a Button, in the _handleClick handler within the Pagination, it updates the components state to reflect the current page.

The component wants to also notify the parent component in _handleClick that the page has changed. That prop should be called something semantic like onPageChange instead of onPageClick. This way if the DOM interaction that triggers the page changes (such as a hover), the event handler name doesn't have to change as well.

// good (uses semantic onPageChange event handler name)
export default class Pagination React.Component {
    static propTypes = {
        onPageChange: React.PropTypes.func
    }
    state = {
        page: 1
    }

    _handlePageClick(e, page) {
        this.setState({page});

        let {onPageChange} = this.props;

        if (onPageChange) {
            onPageChange(page);
        }
    }

    render() {
        let buttons = [1, 2, 3, 4, 5].map((page) => (
            <Button key={page} onClick={this._handlePageClick.bind(this, page)} />
        ));

        return (
            <div>{buttons}</div>
        );
    }
}

// bad (event handler name is DOM-specific)
export default class Pagination React.Component {
    static propTypes = {
        onPageClick: React.PropTypes.func
    }
    state = {
        page: 1
    }

    _handlePageClick(e, page) {
        this.setState({page});

        let {onPageClick} = this.props;

        if (onPageClick) {
            onPageClick(page);
        }
    }

    render() {
        let buttons = [1, 2, 3, 4, 5].map((page) => (
            <Button key={page} onClick={this._handlePageClick.bind(this, page)} />
        ));

        return (
            <div>{buttons}</div>
        );
    }
}

⬆ back to top

DOM event handling

When handling a DOM event that will be passed to the parent via a callback, avoid passing the entire DOM event object. Instead, narrow the component's API by only passing the minimal data that's needed.

If you pass the entire DOM event object:

It's a leaky interface. The parent now has access to event.taget (among other properties) that gives it access to DOM nodes that it shouldn't have access to. At worst, it could manipulate or even remove those DOM nodes.
It's a poor interface. Instead of being passed the data it will need directly, it now has to navigate within the event object to get the data it wants.
It's a fragile interface. If you later want to change how the event is triggered, maybe another type of DOM event can also trigger it, the parents may now have to check the type of event object it

As a result, this means that you must always handle DOM events it within the component even if it's just a wrapper. Otherwise the event object will still be implicitly returned:

// good
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func,
        onBlur: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            // only the value is passed back
            this.props.onChange(value);
        }
    }

    // blur is explicitly handled even though it's a basic wrapper
    _handleBlur() {
        if (this.props.onBlur) {
            this.props.onBlur();
        }
    }

    render() {
        return (
            <input
                type="text"
                onChange={this._handleChange.bind(this)}
                onBlur={this._handleBlur.bind(this)}
            />
        );
    }
}

// bad (_handleChange passes entire event object back)
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func,
        onBlur: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(e);
        }
    }

    _handleBlur() {
        if (this.props.onBlur) {
            this.props.onBlur();
        }
    }

    render() {
        return (
            <input
                type="text"
                onChange={this._handleChange.bind(this)}
                onBlur={this._handleBlur.bind(this)}
            />
        );
    }
}

// bad (blur event isn't wrapped, which implicitly passed back event object)
export default class TextInput extends React.Component {
    static propTypes = {
        onChange: React.PropTypes.func,
        onBlur: React.PropTypes.func
    }

    _handleChange(e) {
        let value = e.target.value;

        if (this.props.onChange) {
            this.props.onChange(e);
        }
    }

    _handleBlur() {
        if (this.props.onBlur) {
            this.props.onBlur();
        }
    }

    render() {
        return (
            <input
                type="text"
                onChange={this._handleChange.bind(this)}
                onBlur={this.props.onBlur}
            />
        );
    }
}

⬆ back to top

Refs

Avoid using React refs. Both callback-style (eslint: react/jsx-no-bind) and string (eslint: react/no-string-refs) refs are prohibited by default.

React refs are a way to get references to underlying DOM nodes after the component has been mounted. This is needed when:

You need to pass a reference to a DOM node to a non-React library (such as jQuery)
You need to manually control a DOM node (such as to call .focus() on an input)

Generally when refs are used within React, it can be rewritten to use state that will cause a re-render. This is a preferred approach because it keeps the code within the optimizations of React's Virtual DOM. Causing ESLint errors when React refs are used, strongly encourages the developer to rethink the approach.

However, if refs are needed, use callback-style refs and temporarily disable the ESLint rules:

export default class RawContainer extends React.Component {
    render() {
        let innerHTML = {__html: '<span>Safe HTML</span>'};

        return (
            {/* eslint-disable react/jsx-no-bind */}
            <input type="text" ref={(input) => this._input = input} />
            {/* eslint-enable react/jsx-no-bind */}
        );
    }
}

This will be a clear signal in code reviews that a special exception is happening. If refs were universally accepted, it would be harder to distinguish between valid and incorrect uses of React refs.

For more info on React refs, see Refs to Components.

⬆ back to top

Dangerous props

Avoid using dangerouslySetInnerHTML (eslint: react/no-danger). This will through an ESLint warning instead of an error.

By default all text passed as content to HTML nodes are sanitized by React, preventing exploits. React is safe by default. However, if your text contains HTML you would like rendered, the HTML will be encoded instead of rendered.

Just like with refs, using dangerouslySetInnerHTML is something that should be used sparingly. When it is truly needed, you can temporarily disable rule:

export default class RawContainer extends React.Component {
    render() {
        let innerHTML = {__html: '<span>Safe HTML</span>'};

        return (
            {/* eslint-disable react/no-danger */}
            <div dangerouslySetInnerHTML={innerHTML} />
            {/* eslint-enable react/no-danger */}
        );
    }
}

Once again, this will be a clear signal in code reviews that a special exception is happening.

For more info on dangerouslySetInnerHTML, see: Dangerously Set innerHTML.

⬆ back to top

`render()`

In addition to render() being the last method in a component (see Method ordering), we have additional best practices...

Logic and JSX

React and JSX supporting logic and markup in the same file allows for substantial complexity in markup generation over other traditional templating languages (like handlebars). But with that increased complexity can come a decrease in readability.

In order to maximize both complexity and readability, we suggest keeping all logic out of JSX, except variable references and method calls. Expressions, particularly ternary expressions, should be stored in variables outside of JSX.

// good
render() {
    let {includeHeader} = this.props;
    let buttons = [1, 2, 3, 4, 5].map((page) => (
        <Button key={page} onClick={this._handlePageClick.bind(this, page)} />
    ));
    let header;

    if (includeHeader) {
        header = (<h2>Pagination</h2>);
    }

    return (
        <div>
            {header}
            {buttons}
        </div>
    );
}

// bad (expressions in JSX)
render() {
    let {includeHeader} = this.props;

    return (
        <div>
            {includeHeader ? (<h2>Pagination</h2>) : null}
            {[1, 2, 3, 4, 5].map((page) => (
                <Button key={page} onClick={this._handlePageClick.bind(this, page)} />
            ))}
        </div>
    );
}

The above "bad" example doesn't seem so bad right? But as we know, code tends to grow over time. If we add more expressions, add more markup to the header, or the map gets more more logic, the code will become unwieldy. Setting up this guideline, even in the most simple examples, helps set the code along the right path.

See Helper components for another way to help keep render() lean.

⬆ back to top

Hiding elements

With React's optimized re-rendering via its Virtual DOM abstraction, you should never need to hide elements with CSS (except maybe with some sophisticated CSS animations). Instead, don't render the element when it shouldn't be visible, and render it when it should:

// good
export default class Togglr extends React.Component {
    state = {visible: false}

    _handleToggle() {
        this.setState({
            visible: !this.state.visible
        })
    }

    render() {
        let {visible} = this.state;
        let message;

        if (visible) {
            message = (
                <p>This message is toggled on/off with React not CSS!</p>
            );
        }

        return (
            <div>
                <Button click={this._handleToggle.bind(this)}>Toggle!</Button>
                {message}
            </div>
        );
    }
}

// bad (uses CSS to hide element instead of not rendering)
export default class Togglr extends React.Component {
    state = {visible: false}

    _handleToggle() {
        this.setState({
            visible: !this.state.visible
        })
    }

    render() {
        let {visible} = this.state;
        let messageClassName;

        if (!visible) {
            messageClassName = 'hidden';
        }

        return (
            <div>
                <Button click={this._handleToggle.bind(this)}>Toggle!</Button>
                <p className={messageClassName}>
                    This message is toggled on/off with CSS 🙁!
                </p>
            </div>
        );
    }
}

Friendly reminder: If you want an entire component to be conditionally rendered, the component must return null. Returning undefined will be an error.

⬆ back to top

State

There are two ways of maintaining data in a React component: props and state.

Props are used by a component's parent to configure the component and are immutable within the component.

State is internal to the component so that it can maintain data that will change over time. Whenever the state changes (via setState), the component will re-render. A component's state should not be manipulated outside of it.

Initializing

We rely on the Class fields proposal for initialize state over assigning this.state in the constructor so that it's more declarative. We don't use getInitialState which was the only option in ES5.

// good
export default class Togglr extends React.Component {
    state = {visible: false}

    // rest of the component
}

// bad (assigns `this.state` unnecessarily in constructor)
export default class Togglr extends React.Component {
    constructor(props, context) {
        super(props, context);
        this.state = {visible: false};
    }

    // rest of the component
}

// bad (uses ES5 `getInitialState`)
export default class Togglr extends React.Component {
    getInitialState() {
        return {visible: false};
    }

    // rest of the component
}

⬆ back to top

Defaulting from props

In general, using props to generate state is an anti-pattern because it results in duplicate "sources of truth" (see Props in getInitialState as an anti-pattern). But if your props is properly named to indicate that it's only used as seed data for the component's internally-controlled state, it's no longer an anti-pattern.

We tend to prefix these types of props with default* to match the defaultValue prop React uses for input elements. initial* is also a good prefix.

When defaulting from props, using the declarative class field no longer works as the props are not available in static scope. The only option is to initialize within the constructor:

// good
export default class Togglr extends React.Component {
    constructor(props, context) {
        super(prop, context);
        this.state = {visible: props.defaultVisible};
    }

    // rest of the component
}

// bad (confusingly-named prop)
export default class Togglr extends React.Component {
    constructor(props, context) {
        super(prop, context);
        this.state = {visible: props.visible};
    }

    // rest of the component
}

In the "bad" example, both props and state have a property called visible, which is very confusing. Should you use this.props.visible or this.state.visible. The one in props cannot change, while the one in state can. Naming the prop defaultVisible (as shown in the "good" example) makes things clearer.

As a reminder, setting the state in the constructor should only be used when defaulting from props.

⬆ back to top

Resetting

The most common use case for resetting state is a form that when submitted should return to its original default values. Resetting the state is as simple as setting it to the same object used to initialize it. To keep your code DRY, store the initial state in a constant so it can be used both for initialization and reset:

// good
const INITIAL_STATE = {
    name: '',
    message: ''
}

export default ContactForm extends React.Component {
    state = {
        name: '',
        message: ''
    }

    _handleFormSubmit() {
        // code for submitting form

        // reset form (from const)
        this.setState(INITIAL_STATE);
    }

    render() {
        // render form w/ inputs
    }
}

// bad (duplicate initial state)
export default ContactForm extends React.Component {
    state = {name: '', message: ''}

    _handleFormSubmit() {
        // code for submitting form

        // reset form
        this.setState({
            name: '',
            message: ''
        });
    }

    render() {
        // render form w/ inputs
    }
}

In ES5, we could've just called getInitialState to reset the form state, but since we're using the declarative approach, we need to store the initial state object in a const.

⬆ back to top

DOM state

Sometimes you will need to render different markup based on the presence of certain DOM APIs such as SVG support, touch support, CSS animation support, etc.

You may be tempted to use state to maintain the presence of the combination of DOM APIs, but since the value will not be changing over time, state is not the appropriate place for this data. Further, each component instance has its own unique state, and the presence of the DOM APIs will not change between instances. Therefore storing this data in state would also be redundant.

Instead use a private static variable to maintain this data:

// good
import React from 'react';

// maintain a private that stores the combination of the
// DOM APIs
let SUPPORTS_FANCINESS;

const SvgLoading = () => {
    // code for an SVG loading display
};

const FallbackLoading = () => {
    // code for a fallback/image loading display
};

export default class Loading extends React.Component {
    _supportsFanciness() {
        if (SUPPORTS_FANCINESS === undefined && /* determine presence of DOM APIs */) {
            SUPPORTS_FANCINESS = true;
        }

        return SUPPORTS_FANCINESS;
    }

    render() {
        let loadingComponent = this._supportsFanciness
            ? (<SvgLoading />)
            : (<FallbackLoading />);

        return (
            <div>
                {loadingComponent}
            </div>
        );
    }
}


// bad (stores API state in `state`)
import React from 'react';

const SvgLoading = () => {
    // code for an SVG loading display
};

const FallbackLoading = () => {
    // code for a fallback/image loading display
};

export default class Loading extends React.Component {
    state = {
        supportsFanciness: false
    }

    componentDidMount() {
        if (/* determine presence of DOM APIs */) {
            this.setState({
                supportsFanciness: true
            })
        }
    }

    render() {
        let {supportsFanciness} = this.state;
        let loadingComponent = supportsFanciness
            ? (<SvgLoading />)
            : (<FallbackLoading />);

        return (
            <div>
                {loadingComponent}
            </div>
        );
    }
}

Setting state in componentDidMount is a poor practice in general because it can result in unnecessary double calls to render(): the initial render and then the subsequent render as a result of setState. In fact we use the react/no-did-mount-set-state ESLint rule to prevent this.

However, in the "good" example, by storing SUPPORTS_FANCINESS in a private static variable, once the first component tries to render, the value will be calculated and subsequently cached. And we still only have one render.

FilesExpand file tree

react

Directory actions

More options

Directory actions

More options

Latest commit

History

react

Folders and files

parent directory

README.md

Eventbrite React & JSX Coding Style Guide

Table of Contents

What is React?

What is JSX?

General rules

Component files

Component class

Class style

displayName

Component organization

Component reference naming

Component propTypes

Defining propTypes

Props naming

Required props

propTypes Ordering

Vague propTypes

Helper components

Component methods

Private helper methods

Method ordering

JSX Wrapping

JSX alignment

Single-line

Multi-line

Indentation

Closing bracket

Self-closing

JSX attribute values

Quoting

Boolean values

Curly brace padding

React key prop

Mandatory key prop

Index as key

Event handlers

Event handler naming

Semantic event handler names

DOM event handling

Refs

Dangerous props

render()

Logic and JSX

Hiding elements

State

Initializing

Defaulting from props

Resetting

DOM state

`displayName`

Component `propTypes`

Defining `propTypes`

`propTypes` Ordering

Vague `propTypes`

React `key` prop

Mandatory `key` prop

Index as `key`

`render()`