A few do’s and don’ts in React

I have been lucky to to work with the React ecosystem for a couple of years now. What is really cool about React is that their creators really challenged all the conventional wisdom and “best practices” that had previously been treated like some kind of dogma within the JavaScript developers community. As a result, mixing JavaScript with HTML with a little bit of inline CSS on top of that is no longer considered a cardinal sin and other concepts such as Virtual DOM, single-direction data flow or encapsulating pieces of UI in a form of re-usable components are being embraced by competing libraries and frameworks pretty much as we speak.

Still, in this world of new, big programming ideas and best practices being rethought, there are certain patterns and techniques that are really timeless and I can see no reason why they shouldn’t be followed in React applications. You know, things like good naming conventions, code clarity, readability etc.

But whenever I tried enforcing some of these practices in my conversations with peer developers, I was often confronted with one of the following responses: no one else is doing it this way or why would you do it like that?.

I blame the Internet as it’s full of code samples and projects that seem to ignore some of these well-established, good programming practices. These things tend to become ubiquitous very quickly as we all love copy-and-paste. But just because something’s been published on GitHub and it has several hundred stars, doesn’t always mean it should be followed blindly.

So let’s discuss some of the most common pitfalls React developers seem to fall into and how they could be improved.

Referencing globals

Let’s start from things like window or document. I thought this one would be quite obvious since the emergence of the immediately invoked function expression (IIFE) pattern that gained popularity a few years ago:

(function(dom) {
// "document" has been mapped to a local
// variable called "doc" = better performance!
console.log(dom)
})(document)

The idea is simple: rather than referencing things like the document scope directly within the function, you inject it as an argument that the IIFE maps to a local variable. Referencing a local variable is always going to perform better than depending on globals as there’s no need for scope look-ups.

And yet if you search for some React code samples where you need to rely on DOM APIs, chances are you’ll bump into direct references to document or window right in the middle of render() or some event handler method body. But wasn’t abstarcting out the DOM one of the fundamental concepts behind React?.. These things can get even more problematic if your application is isomorphic as there is no such thing as document on the server-side.

So is there a better way then? Well, I tend to steer clear of things like React context as much as I can but this is the approach we decided to resort to in one of my recent projects. We simply made window available via a React context variable but I suppose you might as well just pass it down to child components as a property or, at the very least, destructure it as an instance variable in your React class constructor.

And by the way, we’ve only done it for the window scope as document is available within it anyway (window.document that is).

Confusing variable names

How many times have you seen pieces of React code where they would call the event object passed to an onClick handler something other than e? I see this stuff everywhere.

The thing is “e” is a letter of the alphabet and it doesn’t mean anything. Confusing variable names are evil — they make the code harder to understand and this particular one is a serious offender as e sometimes stands for event and at other times — error.

Just don’t use single letter names and everything will be fine. It’s only four extra characters that will make your code more readable and you will probably get your cryptic variable name back anyway, in the resulting compiled bundle, provided you are using an uglifier.

And last but not least: Facebook themselves use the full event term in their documentation so why not follow their lead?

Complex JSX

My approach to JSX code basically boils down to these two rules:

Don’t put too much JSX code in one place

This is really nothing more than adhering to the olde good KISS principle: if you end up with a very long, often deeply nested block of JSX code, that usually means it’s time for some re-factoring. I don’t know, it could mean pulling some nodes out and wrapping them up in separate methods or modules, it could mean rethinking the components hierarchy altogether, whatever.

This is important not just for better code readability but also from the maintainability and testability perspective. The simpler a unit of code, the easier it is to reuse it in different contexts and the easier it is to write tests for it. Oh, and let’s not forget any node you plonk into JSX becomes a React component, native HTML elements included. So there’s an important performance consideration here as well (think about it before you slap another <span> or <div> onto your code).

Don’t mix logic with JSX to manipulate what actually gets rendered

This one is probably going to prove more controversial because, well, everyone does it! I’m sure you’ve come across ternary operators sprinkled around JSX, more often than not, often encapsulating multiple lines of code as well:

The bulkier and heavier on logic the JSX is, the more complex and convoluted the resulting transpiled block of JavaScript becomes. Not convinced? Pick a few lines of your JSX code and run it in Babel REPL to see the final product you are going to ship to your user’s browser.

Even without those virtual “dozens of lines of code” within the ternary operator in the example above, you can see we’re dropping a completely useless null in the code.

There’s really no need for that and the example below demonstrates how this mess can easily be tidied up:

Voilà! We now have code that’s more expressive, easier to read, and its individual units are clean and simple. And another good thing about this approach is it forces you to consider reusability as the getVisibleContent is now literally waiting there to be factored out to a separate module.

Messy order of functions (or the return of GOTO)

I don’t know when I first came across this advice but it seems like there is a general consensus around always placing the render method right at the bottom of your class-based React components. As a result, reading the code, often means basically jumping up and down the file to follow the flow of it. It’s like turning back in time and finding yourself in the era of GOTO.

The solution to this is obvious. Don’t define your functions up-front — instead, only do it after they’ve been called. If you keep your code well structured and simple, you should then be able to read it from top to bottom, like you do when reading a book.

So if e.g. there is an event handler that you call from within the render method, declare the handler below it. And yes, this means no more render methods glued to the bottom of the class body.

In fact, this advice also applies to lifecycle methods available in React as there’s no reason why you’d want to declare something like componentDidUpdate before render considering the fact the former is always invoked after the latter.

The following template reflects the order in which React lifecycle methods are invoked and thus is my starting point for class-based components written in ES6:

class MyClass extends React.Component {
  constructor (props, context) {}
  componentWillReceiveProps (nextProps) { }
  shouldComponentUpdate (nextProps, nextState) { return true }
  componentWillUpdate (nextProps, nextState) { }
  render () {}
  componentDidMount () { }
  componentDidUpdate (prevProps, prevState) { }
  componentWillUnmount () { }
}

Notice I’ve not included componentWillMount in the template as in most cases the ES6 constructor method can be used instead.

Function calling another function calling another function calling another function…

This one is exactly what it reads on the tin and I believe it might have been popularised with the emergence of the Redux ecosystem as well as Higher Order Components (HOCs). There’s nothing wrong with Redux or HOCs but how on earth something like this can possibly be considered an example worth following is beyond me:

connectToStore(addMiddleware(wrap(enhance(MyComponent)), secondArgument))

There’s literally nothing that is good about this piece of code. Is it easy to read? No. Is the code simple? No. Is it easy to work out which function accepts the secondArgument? Not at all.

Some people consider decorators a good alternative to it that allows you to avoid wrapping stuff in function calls. But I personally find them even worse as they are just too “magical” and they are yet to be standardised in EcmaScript.

Instead, let’s just break things down, make some stylistic tweaks and apply some menaningful variable names that describe the results of individual function calls.

const enhancedComponent = enhance(MyComponent)
const wrappedComponent = wrap(
enhancedComponent, secondArgument
)
const withMiddleware = addMiddleware(wrappedComponent)
connectToStore(withMiddleware)

We have now converted a crammed and confusing one-liner into several lines of code. This in itself conveys and important message that we are going to apply a set of operations to our component. And the fact each of these operations is assigned to a variable (that is supposed to be well named) gives even more context and meaning to the individual steps that are taking place. Yes, there is more code to write but it is definitely easier to read and follow.


And that is it for now. The issues I’ve listed down keep coming back in my code reviews but the list is certainly not exhaustive. There a few more things I keep pointing at almost everyday so, hopefully, we cover them in the next rant.