Refactoring a React app with ESLint and Airbnb's style guide


   



On the previous post we built a chat application with Django Channels and a React frontend.

Just because something works it doesn't mean it can't be vastly improved, and some developers were quick to point out the frontend app's flaws.

On this post, I will configure ESLint to follow Airbnb's style guide. After that, I will use the React app of the last post to walk you through a refactoring session where I split the app into components, add modern React features and improve the code quality.


A quick note for Emacs users


If you do most of your development with Emacs, setting it up for Javascript is a must. I recommend this guide.

By default, Emacs doesn't indent Javascript properly. If you like using tabs, you should replace tabs with 2 spaces.

~/.emacs

(setq js-indent-level 2)
(setq rjsx-indent-level 2)

After doing this you can select a whole file with C-x h and indent it with M-x indent-region. I fixed around 50% of the errors shown by ESLint just by doing this.

Installing ESLint


$ cd frontend/src
$ npm install eslint --save
$ eslint --init

You will be asked a series of questions to configure the linter. Here are my answers:

? How would you like to use ESLint? To check syntax, find problems, and enforce code style
? What type of modules does your project use? JavaScript modules (import/export)
? Which framework does your project use? React
? Does your project use TypeScript? No
? Where does your code run? Browser
? How would you like to define a style for your project? Use a popular style guide
? Which style guide do you want to follow? Airbnb: https://github.com/airbnb/javascript
? What format do you want your config file to be in? JavaScript
Checking peerDependencies of [email protected]
The config that you've selected requires the following dependencies:

[email protected]^7.19.0 [email protected] [email protected]^5.16.0 || ^6.8.0 [email protected]^2.20.1 [email protected]^6.2.3 [email protected]^2.5.0 || ^1.7.0
? Would you like to install them now with npm? (Y/n) Y


Getting started with ESLint


The eslint CLI tool will output rows with four columns:

line:column
error|warning
message
type

I find it useful to order the output by type.

$ eslint components/App.js | sort -k 4


This is how the React code looked at the end of the previous post:

frontend/src/components/App.jsx

import React, { Component } from "react";
import { render } from "react-dom";


class App extends Component {
constructor(props) {
super(props);
this.state = {
messages: [],
};
}


componentDidMount(){
const roomName = location.pathname.substr(1);

var socketPath = 'ws://'
+ window.location.host
+ '/ws/'
+ roomName;

const chatSocket = new WebSocket(
socketPath
);

chatSocket.onmessage = (e) => {
var data = JSON.parse(e.data);
var message = {text: data.message, date: data.utc_time};
message.date = moment(message.date).local().format('YYYY-MM-DD HH:mm:ss');

let updated_messages = [...this.state.messages];
updated_messages.push(message);
this.setState({messages: updated_messages});
};

chatSocket.onclose = (e) => {
console.error('Chat socket closed unexpectedly');
};

document.querySelector('#chat-message-input').focus();
document.querySelector('#chat-message-input').onkeyup = (e) => {
this.clickSubmitMessage
};

document.querySelector('#chat-message-submit').onclick = (e) => {
var messageInputDom = document.querySelector('#chat-message-input');
var message = messageInputDom.value;

chatSocket.send(JSON.stringify({
'message': message
}));
messageInputDom.value = '';
};
}

render() {
return (
<div>

{this.state.messages.map(function(item, i){
return <div key={i} id="message" className="card">

<div className="cell large-4">{item.text}</div>
<div className="cell large-2 text-right"><small>{item.date}</small></div>
</div>
;}
)}


<textarea id="chat-message-input" type="text" cols="100" /><br />
<input id="chat-message-submit" type="button" className="button" value="Send" />

</div>
);
}
}


export default App;

const container = document.getElementById("app");
render(<App />, container);


And here's what the linter shows for me after fixing most indentation issues:

/home/.../frontend/src/components/App.js  

39 errors and 0 warnings potentially fixable with the `--fix` option.
✖ 55 problems (53 errors, 2 warnings)
42:7 error Expected an assignment or function call and instead saw an expression no-unused-expressions
65:23 error Requires a space before '}' block-spacing
31:34 error Use callback in setState when referencing the previous state react/no-access-state-in-setstate
37:7 warning Unexpected console statement no-console
65:23 error Closing curly brace should be on the same line as opening curly brace or on the line after the previous block brace-style
59:34 error Unexpected function expression prefer-arrow-callback
61:18 error Expected indentation of 10 space characters but found 17 react/jsx-indent
64:22 error Expected indentation of 10 space characters but found 21 react/jsx-indent
60:1 error Expected indentation of 10 spaces but found 8 indent
65:16 error Expected indentation of 8 space characters but found 15 react/jsx-indent
66:1 error Expected indentation of 8 spaces but found 32 indent
36:27 error 'e' is defined but never used no-unused-vars
41:62 error 'e' is defined but never used no-unused-vars
45:63 error 'e' is defined but never used no-unused-vars
31:11 error 'updated_messages' is never reassigned. Use 'const' instead prefer-const
29:22 error 'moment' is not defined no-undef
67:68 error `br` must be placed on a new line react/jsx-one-expression-per-line
1:34 error Strings must use singlequote quotes
2:24 error Strings must use singlequote quotes
78:43 error Strings must use singlequote quotes
66:33 error Unexpected newline before ')' function-paren-newline
58:7 error JSX not allowed in files with extension '.js' react/jsx-filename-extension
60:26 error Do not use Array index in keys react/no-array-index-key
60:16 error Missing parentheses around multilines JSX react/jsx-wrap-multilines
50:9 error Unnecessarily quoted property 'message' found quote-props
1:1 error 'react' should be listed in the project's dependencies, not devDependencies import/no-extraneous-dependencies
2:1 error 'react-dom' should be listed in the project's dependencies, not devDependencies import/no-extraneous-dependencies
59:42 error Missing space before function parentheses space-before-function-paren
14:22 error Missing space before opening brace space-before-blocks
59:51 error Missing space before opening brace space-before-blocks
28:21 error A space is required after '{' object-curly-spacing
33:21 error A space is required after '{' object-curly-spacing
67:71 error A space is required before closing bracket react/jsx-tag-spacing
28:61 error A space is required before '}' object-curly-spacing
33:48 error A space is required before '}' object-curly-spacing
13:1 error Trailing spaces not allowed no-trailing-spaces
21:1 error Trailing spaces not allowed no-trailing-spaces
25:1 error Trailing spaces not allowed no-trailing-spaces
30:1 error Trailing spaces not allowed no-trailing-spaces
66:35 error Trailing spaces not allowed no-trailing-spaces
69:1 error Trailing spaces not allowed no-trailing-spaces
17:22 error Unexpected string concatenation prefer-template
23:17 error Missing trailing comma comma-dangle
50:27 error Missing trailing comma comma-dangle
65:24 error Missing trailing comma comma-dangle
59:34 warning Unexpected unnamed function func-names
31:11 error Identifier 'updated_messages' is not in camel case camelcase
31:34 error Must use destructuring state assignment react/destructuring-assignment
59:10 error Must use destructuring state assignment react/destructuring-assignment
15:22 error Unexpected use of 'location' no-restricted-globals
17:5 error Unexpected var, use let or const instead no-var
27:7 error Unexpected var, use let or const instead no-var
28:7 error Unexpected var, use let or const instead no-var
46:7 error Unexpected var, use let or const instead no-var
47:7 error Unexpected var, use let or const instead no-var


Amazing! Don't be surprised if working Javascript code results in dozens of linting errors, the syntax of this language is much more forgiving than the syntax of most languages.

Another useful way to order eslint's output is by line number:

$ eslint components/App.js | sort -k 1


Updating ESLint's output (almost) in real time


This command will run eslint every 5 seconds and sort the results by type:

$ watch -n 5 "eslint src/components/App.jsx | sort -k 4"




Refactoring the React app


You can find the fully refactored code in the repository, but I will solve some of the errors here, so you can learn the workflow.

The first step is to rename App.js to App.jsx.

$ mv src/components/App.js src/components/App.jsx

webpack.config.js

module.exports = {
module: {
rules: [
{
test: /\.(js|jsx)$/,
exclude: /node_modules/,
use: {
loader: 'babel-loader'
}
}
]
}
};


Solving ESLint errors methodically


59:34  error    Unexpected function expression                                                                                 prefer-arrow-callback


Whenever you find a message you don't understand, you can refer to the style guide for a more detailed explanation, or to the eslint plugin docs.

If we go to https://github.com/airbnb/javascript#arrow-functions and type prefer-arrow-callback in the browser's search, it takes us to this subsection:

8.1 When you must use an anonymous function (as when passing an inline callback), use arrow function notation. eslint: prefer-arrow-callback, arrow-spacing

Why? It creates a version of the function that executes in the context of this, which is usually what you want, and is a more concise syntax.

Why not? If you have a fairly complicated function, you might move that logic out into its own named function expression.


An anonymous function that starts this way:

          this.state.messages.map(function(item, i){

should actually start this way:

         this.state.messages.map((item, i) => (


You can use the same workflow for every error:

  1. Go to the line:number
  2. Optionally search the message's type in the style guide
  3. Fix it
  4. Read the linter's output


Letting ESLint do the heavy lifting


It's easy to see how this workflow can become tedious after a while. If you are working with a large codebase you could be dealing with thousands of errors and warnings.

Fortunately ESLint can fix a lot of errors for you. All you need to do is run it with the --fix option.

$ eslint src/components/App.jsx --fix


When I run the linter again, the errors are almost gone!

   1:1   error    'react' should be listed in the project's dependencies, not devDependencies      import/no-extraneous-dependencies
2:1 error 'react-dom' should be listed in the project's dependencies, not devDependencies import/no-extraneous-dependencies
14:22 error Unexpected use of 'location' no-restricted-globals
28:22 error 'moment' is not defined no-undef
30:13 error Identifier 'updated_messages' is not in camel case camelcase
30:36 error Must use destructuring state assignment react/destructuring-assignment
30:36 error Use callback in setState when referencing the previous state react/no-access-state-in-setstate
35:27 error 'e' is defined but never used no-unused-vars
36:7 warning Unexpected console statement no-console
41:63 error 'e' is defined but never used no-unused-vars
55:10 error Must use destructuring state assignment react/destructuring-assignment
56:21 error Do not use Array index in keys react/no-array-index-key

✖ 12 problems (11 errors, 1 warning)


By fixing indentation, running --fix and doing a couple of manual fixes, I have gone from 126 to 12 errors.


Fixing some more code


The code is almost compliant with real-estate mogul standards, but there still are a few errors that eslint won't dare to fix.

  1:1   error    'react' should be listed in the project's dependencies, not devDependencies      import/no-extraneous-dependencies
2:1 error 'react-dom' should be listed in the project's dependencies, not devDependencies import/no-extraneous-dependencies

package.json

  "devDependencies": {
"@babel/core": "^7.9.6",
"@babel/preset-env": "^7.9.6",
"@babel/preset-react": "^7.9.4",
"babel-loader": "^8.1.0",
"eslint": "^6.8.0",
"eslint-config-airbnb": "^18.1.0",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-react": "^7.20.0",
"eslint-plugin-react-hooks": "^2.5.1",
"webpack": "^4.43.0",
"webpack-cli": "^3.3.11"
},
"dependencies": {
"react": "^16.13.1",
"react-dom": "^16.13.1"
}


 30:36  error    Use callback in setState when referencing the previous state  react/no-access-state-in-setstate
      const updated_messages = [...this.state.messages];
updated_messages.push(message);
this.setState({ messages: updated_messages });

Taking a copy of the state, modifying it and setting it again is a bad practice. Instead, state should be updated with prevState, something we can achieve by replacing the code above with this line:

      this.setState((prevState) => ({ messages: prevState.messages.concat(message) }));

You can read more about updater functions and setState here.


 54:21  error    Do not use Array index in keys           react/no-array-index-key
        {this.state.messages.map((item, i) => (
<div key={i} id="message" className="card">

Keys help React identify which items have changed, are added, or are removed.

At a quick glance, it looks like the code above is really nice. List items have an index, and when we use map every element must have a key, so let's make that key the index of each item.

However, this is just wrong.

A list can have different items at the same index at different moments in time. By using the index as the element's key, React will think different elements are the same when the list changes, leading to a massive brain fart.

You should use a unique property such as the element's id instead.

        {this.state.messages.map((item) => (
<div key={item.id} id="message" className="card">


34:7   warning  Unexpected console statement             no-console
    chatSocket.onclose = () => {
console.error('Chat socket closed unexpectedly');
};

Console logs are mostly useful for troubleshooting purposes and may as well be removed.


Destructuring assignment


53:10  error    Must use destructuring state assignment  react/destructuring-assignment


Quoting MDN:

The destructuring assignment syntax is a JavaScript expression that makes it possible to unpack values from arrays, or properties from objects, into distinct variables.

With this error, the linter is trying to encourage cleaner syntax.

  render() {
const { messages } = this.state;

This code is equivalent to

  render() {
const messages = this.state.messages;

And we can now map items like this:

        {messages.map((item) => (


Adding message and message input components


So far, the application has a single component (App). However, we are rendering a list of messages inside the App component, which is calling for a Message component. We will also add a message input component that doesn't take props but will make the render loop cleaner.

components/Message.jsx

import React from 'react';


function Message(props) {
const { text, date } = props;
return (
<div id="message" className="card">
<div className="cell large-4">
{text}
</div>
<div className="cell large-2 text-right"><small>{date}</small></div>
</div>
);
}

export default Message;


components/MessageInput.jsx

import React, { useRef } from 'react';


function MessageInput() {
const messageInput = useRef(null);
messageInput.current.focus();

return (
<div>
<textarea id="chat-message-input" type="text" cols="100" />
<br />
<input id="chat-message-submit" type="button" className="button" value="Send" />
</div>
);
}


export default MessageInput;


components/App.jsx

  render() {
const { messages } = this.state;
const messageInput = <MessageInput />;

const messageList = (
<div>
{messages.map((item) => (
<div key={item.id}>
<Message text={item.text} date={item.date} />
</div>
))}
</div>
);

return (
<div>
{messageList}
{messageInput}
</div>
);
}


Typechecking components with PropTypes


The Message component should always receive text and date props of type string. One way to validate this is with PropTypes.

$ npm i -S prop-types

components/Message.jsx

import PropTypes from 'prop-types';

function Message(props) {...}

Message.propTypes = {
text: PropTypes.string.isRequired,
date: PropTypes.string.isRequired,
};


Replacing Javascript with React hooks


The React site says that Refs provide a way to access DOM nodes or React elements created in the render method. They even use a text input as an example!

React 16.8 introduced hooks in early 2019. Hooks are a revolutionary feature in React, because they let you do things in function components that were previously limited to class components, including using refs.

Let's start by getting rid of this line:

App.jsx

    document.querySelector('#chat-message-input').focus();

MessageInput.jsx

import React, { useRef, useEffect } from 'react';


function MessageInput() {
const messageInput = useRef(null);

function focusInput() {
messageInput.current.focus();
}

useEffect(() => {
focusInput();
});

return (
<div>
<textarea id="chat-message-input" type="text" cols="100" ref={messageInput} />
...
useEffect is a React hook. It is called after the component is rendered, and in this case it will call take care of focusing the input on the textarea.


App.jsx

    document.querySelector('#chat-message-submit').onclick = () => {
const messageInputDom = document.querySelector('#chat-message-input');
const message = messageInputDom.value;

chatSocket.send(JSON.stringify({
message,
}));
messageInputDom.value = '';
};


Remove this line:

messageInputDom.value = '';

and add this code:

MessageInput.jsx

  function resetInput() {
    messageInput.current.value = '';
  }

...

<input id="chat-message-submit" type="button" className="button" value="Send" onClick={resetInput} />


Moving the websocket logic to its own function


The code that creates a socket path and a WebSocket instance has nothing to do with a React app. By moving it outside of the App component we can make the code more organized and reusable.

App.jsx

function getSocket() {
const roomName = window.location.pathname.substr(1);
const socketPath = `ws://${
window.location.host
}/ws/${
roomName}`;

const chatSocket = new WebSocket(
socketPath,
);

return chatSocket;
}


class App extends Component {
constructor(props) {
super(props);

this.state = {
messages: [],
};

this.chatSocket = getSocket(); // Remember to replace all chatSocket occurrences with this.chatSocket!
}


Removing the final querySelector


The app component still contains some smelly code. Go ahead and remove this:

App.jsx

    document.querySelector('#chat-message-submit').onclick = () => {
const messageInputDom = document.querySelector('#chat-message-input');
const message = messageInputDom.value;

this.chatSocket.send(JSON.stringify({
message,
}));
};


We will pass the websocket connection to the MessageInput component, and it will take care of sending the message by listening to its own click events. When the button is clicked a function will call the function that sends the message and the one that clears the input.

Other than that, a PropType will make sure the component actually receives a websocket.

These are the final versions of the three components:

App.jsx

import React, { Component } from 'react';
import { render } from 'react-dom';
import MessageInput from './MessageInput';
import Message from './Message';


function getSocket() {
const roomName = window.location.pathname.substr(1);
const socketPath = `ws://${
window.location.host
}/ws/${
roomName}`;

const chatSocket = new WebSocket(
socketPath,
);

return chatSocket;
}


class App extends Component {
constructor(props) {
super(props);

this.state = {
messages: [],
};

this.chatSocket = getSocket();
}


componentDidMount() {
this.chatSocket.onmessage = (e) => {
const data = JSON.parse(e.data);
const message = { text: data.message, date: data.utc_time };
message.date = moment(message.date).local().format('YYYY-MM-DD HH:mm:ss');

this.setState((prevState) => ({ messages: prevState.messages.concat(message) }));
};
}

render() {
const { messages } = this.state;
const messageInput = <MessageInput socket={this.chatSocket} />;

const messageList = (
<div>
{messages.map((item) => (
<div key={item.id}>
<Message text={item.text} date={item.date} />
</div>
))}
</div>
);

return (
<div>
{messageList}
{messageInput}
</div>
);
}
}


export default App;

const container = document.getElementById('app');
render(<App />, container);


MessageInput.jsx

import React, { useRef, useEffect } from 'react';
import PropTypes from 'prop-types';


function MessageInput(props) {
const messageInput = useRef(null);
const { socket } = props;

function focusInput() {
messageInput.current.focus();
}

function resetInput() {
messageInput.current.value = '';
}

function sendMessage() {
const message = messageInput.current.value;
socket.send(JSON.stringify({
message,
}));
}

function handleSubmission() {
sendMessage();
resetInput();
}

useEffect(() => {
focusInput();
});

return (
<div>
<textarea id="chat-message-input" type="text" cols="100" ref={messageInput} />
<br />
<input id="chat-message-submit" type="button" className="button" value="Send" onClick={handleSubmission} />
</div>
);
}

MessageInput.propTypes = {
socket: PropTypes.instanceOf(WebSocket).isRequired,
};


export default MessageInput;


Message.jsx

import React from 'react';
import PropTypes from 'prop-types';


function Message(props) {
const { text, date } = props;
return (
<div id="message" className="card">
<div className="cell large-4">
{text}
</div>
<div className="cell large-2 text-right"><small>{date}</small></div>
</div>
);
}

Message.propTypes = {
text: PropTypes.string.isRequired,
date: PropTypes.string.isRequired,
};

export default Message;


Conclusion


We have learned how ESLint can help us massively improve the quality of a Javascript / React application, and how to use it on a Linux terminal.

The App component has been split into App, MessageInput and Message components, and vanilla Javascript code has been replaced with React code.

We have learned about React hooks and PropTypes, and how to pass a websocket to a React component.

Is the code perfect yet? It's not even close! I challenge you to keep refactoring it and to add more cool React features.

You can find the code repository here.