Common Mistakes and Anti-Patterns

---

Table of contents

• Don’t use short circuiting against functions that could have side effects
• Name prop event callbacks onSomething, not handleSomething
• Name event handlers handleSomething, not somethingHandler
• Avoid using lodash for truthy checks
• Use JSX instead of string interpolation
• Use ternary operators and parenthesis to make JSX easier to read
• Avoid calling public methods on child components
• Use useRef for cancel tokens in functional components

---

Don’t use short circuiting against functions that could have side effects

Using short-circuiting makes code difficult to read. An if statement clearly communicates that the code should only be run if a condition is fulfilled. But using a logical operator for the same purpose is just confusing. You can read more about this topic here.

// Bad
cond1 && cond2 && submitForm();

// Bad
foo && foo();

// Good
if (cond1 && cond2)
  submitForm();

// Good
foo?.();

// Good
const bar = cond1 && cond2 && submitForm();

// Good
<span>{cond1 && <Something />}</span>

Name prop event callbacks onSomething, not handleSomething

Callback props should be named like events, not handlers.

export interface MyProps {
  handleRemoveItem: (item: Foo) => void; // Bad
  onRemoveItem: (item: Foo) => void; // Good
  onRemoveItem?: (item: Foo) => void; // Even better, if possible
}

Name event handlers handleSomething, not somethingHandler

Handlers should be prefixed with “handle”, unless a better name is available.

private clickHandler = () => { ... } // Bad
private handleClick = () => { ... } // Good
private doSomething = () => { ... } // Also good

Avoid using lodash for truthy checks

Typescript has many inference features, but many features in lodash do not infer information. Thus, you should avoid using certain lodash methods if there is a more simplistic approach available to you.

const bar: number | undefined = ...;

// Bad, typescript needs to be told bar will not be undefined
const foo = !_.isEmpty(bar) ? bar! : defaultValue;

// Good, a simple truthy check will suffice here
const foo = bar ? bar : defaultValue;

Use JSX instead of string interpolation

String interpolation can potentially render things like “undefined”, whereas JSX will not. You should rely on JSX for rendering whenever possible.

// Bad
<span>{`My name is ${person.name}`}</span>

// Good
<span>My name is {person.name}</span>

Use ternary operators and parenthesis to make JSX easier to read

JSX can get long-winded. Try to format your code to maintain readability.

// Bad
{condition1 && this.hasValidData(someData) ? <Something data={this.getFormattedData(someData)} what={true} when={false} /> : <NoDataView />}

// Bad (hard to find last curly brace)
{condition1 && this.hasValidData(someData)
  ? <Something data={this.getFormattedData(someData)}/>
  : <NoDataView />}

// Good
{condition1 && this.hasValidData(someData) ? (
  <Something
    data={this.getFormattedData(someData)}
    what={true}
    when={false}
  />
) : (
  <NoDataView />
)}

// Also fine
{
  condition1 && this.hasValidData(someData)
  ? <Something data={this.getFormattedData(someData)} what={true} when={false}/>
  : <NoDataView />
}

Avoid calling public methods on child components

Take the following code for example:

export class Parent extends React.Component {
  private readonly child = React.createRef<Child>();

  public render() {
    return (
      <Child ref={this.child} />
      <Button onClick={this.increment} />
    );
  }

  private increment = () => {
    this.child.current?.increment();
  }
}

interface ChildState {
  value: number;
}

export class Child extends React.Component<{}, ChildState> {
  constructor() {
    super({});
    this.state = { value: 1 };
  }

  public render() {
    return this.state.value;
  }

  public increment() {
    this.setState(prevState => ({ value: prevState.value + 1 }));
  }
}

The with this implementation is that the parent is trying to do something that would cause a change in the state of the child component. In other words, the parent is trying to control the child.

A better way of achieving this is to make value a controlled value, by making it a prop in the child instead of state:

interface ParentState {
  value: number;
}

export class Parent extends React.Component<{}, ParentState> {
  constructor() {
    super({});
    this.state = { value: 1 };
  }

  public render() {
    return (
      <Child value={this.state.value} />
      <Button onClick={this.increment} />
    );
  }

  private increment = () => {
    // Now parent owns the value state
    this.setState(prevState => ({ value: prevState.value + 1 }));
  }
}

export interface ChildProps {
  // Now "value" is a controlled value
  value: number;
}

export class Child extends React.Component<ChildProps> {
  public render() {
    return this.props.value;
  }
}

Use useRef for cancel tokens in functional components

const MyComponent = () => {
  const cancel = useRef(CancelToken.source());

  // Cancel on unmount
  useEffect(() => {
    const currentToken = cancel.current;
    return () => currentToken.cancel();
  }, []);

  ...
  const data = await apollo.appointments.query({
    ...
    httpOptions: {
      cancel: cancel.current.token
    }
  });
  ...
};