Code Style

This document aims to give general guidelines of how code is written and structured in the Cloud Manager project.

Writing a React Component

When it comes to writing a React component, there are some clear optimizations you should be making to your code.

As a rule, if you're writing a class component and do not intend on writing any shouldComponentUpdate logic yourself, write a PureComponent. PureComponents implement a shallow comparison of props and state by default so this should encourage you to pass down props and use state that is flat and doesn't need any deep checking.

Good

class MyComponent extends React.PureComponent<MyProps> {}

Also good

import { equals } from 'ramda'

class MyComponent extends React.Component<MyProps> {
  shouldComponentUpdate(prevProps: MyProps) {
    if(equals(prevProps, this.props)) {
      return false;
    }
    return true;
  }
}

Worse

class MyComponent extends React.Component<MyProps> {}

Function components have their place, but please keep in mind function components are treated as Classes under the hood in React, so really, you're not getting any performance boost from writing a function component versus a class.

That being, said with the introduction of hooks, function components have become a lot more valuable, so you may find yourself writing them more often than you would a PureComponent. With that in mind, nearly all function components should be memoized with the invocation of React.memo() in order to gain the same benefit that PureComponents do.

Like PureComponents, function components wrapped in React.memo() have a shallow prop and state comparison implemented by default. You can also create your own update conditions as the second argument passed to React.memo()

Okay

const MyComponent: React.FC<MyProps> = (props) => ()

Much better

const MyComponent: React.FC<MyProps> = (props) => ()

/** this is what we need to export */
const EnhancedComponent = React.memo(MyComponent);

With custom update conditions

import { equals } from 'ramda'

const MyComponent: React.FC<MyProps> = (props) => ()

/*
 return true if passing nextProps to render would return
 the same result as passing prevProps to render,
 otherwise return false
*/
const areEqual = (prevProps: MyProps, nextProps: MyProps) => {
  return equals(prevProps, nexProps)
}

/** this is what we need to export */
const EnhancedComponent = React.memo(MyComponent, areEqual);

Testing Memoized components

Testing memoized components is little tricky, since they dont return JSX when shallow rendered, but instead return an object. This means when you try

const MyChildComponent = React.memo(props => {
  return <div />
});

const MyComponent = React.memo(props => {
  return <MyChildComponent />
});

const Component = shallow(<MyComponent />); // fails here

expect(Component.find('MyChildComponent'))

You end up with an error like this in your test

Invariant Violation: ReactShallowRenderer render(): Shallow rendering works only with
custom components, but the provided element type was `object`.

Instead, try something like this

const MyChildComponent = React.memo(props => {
  return <div />
});

const MyComponent = props => {
  return <MyChildComponent />
};

export default React.memo(MyComponent);

const Component = shallow(<MyComponent />); // all systems go

expect(Component.find('MyChildComponent')); // wait - it's still failing here

Now you'll be able to correctly shallow render the component and still be able to leverage memoization; however, the smoke test still fails? Why?

Well, if you console.log(Component.debug()), you'll see something like this

<[object Object] />

Now we're running into the issue where our component doesn't have a display name. Our current solution is to implement solution that mimics our end-to-end tests

const MyChildComponent = React.memo(props => {
  return <div />
});

const MyComponent = props => {
  return <MyChildComponent data-qa-child-component />
};

export default React.memo(MyComponent);

const Component = shallow(<MyComponent />); // all systems go

expect(Component.find('[data-qa-child-component]')); // our test is passing!!! woooo

Avoiding instance methods that could be made into components.

Everytime an instance of a component is created so are all the instance methods, just like renderContent in the following code. So this takes more CPU to create, more memory to store, and more CPU to tear down. This code smells because theres a function invocation that has no arguments. That screams side-effects. To correct this we simply extract the functionality into a new component passing the props as necessary.

Before

class MyComponent extends Component {
  renderContent = () => {
    const { error, loading, data } = this.props;

    if (error) {
      return this.renderError();
    }

    if (loading) {
      return this.renderLoading()
    }

    if (!data || data.length === 0) {
      return this.renderEmptyState()
    }

    return this.renderData();
  }

  render(){
    <div>
      { this.renderContent() }
    </div>
  }
}

After

const MyComponent = (props) => {
  const { loading, error, data } = props;

  return (
    <div>
      <ComponentContent loading={loading} error={error} data={data}/>
    </div>
  );
}

const ComponentContent = (props) => {
  const { loading, error, data } = props;

  if (error) {
    return <ErrorComponent error={error}>
  }

  if (loading) {
    return <LoadingComponent />
  }

  if (!data || data.length === 0) {
    return <EmptyStateComponent >
  }

  return <DataComponent data={data} />
}

const ErrorContent = (props) => {
  const { error } = props;
  return (...);
}

const LoadingComponent = (props) => {
  return (...);
}

const EmptyStateComponent = (props) => {
  return (...);
}

const DataComponent = (props) => {
  const { data } = props;
  return (...);
}

So now we have two separate and testable components. We've also done the type checking so inside the DataComponent, we dont have to worry about if there's no data or an empty array, we can just work with what we're provided!

Abstraction

Abstracting code is a great way to not repeat yourself and keep the code DRY. It's not a huge deal if you need to repeat some code twice, but more than that, whatever logic you write should be abstracted into either it's own file or outside of the Class/function. Here are some hard, fast rules when it comes to abstracting:

Any functions that are inside a React Class/function that don't rely on state or props should be abstracted out

Bad

class MyComponent extends React.PureComponent<MyProps> {
  /** no reason for this to be attached to the Class */
  filterOutNumbers = (arrayOfNumbers: number[]) => {
    return arrayOfNumbers.filter(eachNumber => eachNumber > 10)
  }

  return <div />
}

Good

class MyComponent extends React.PureComponent<MyProps> {
  return <div />
}

/** this can now be unit tested seperately */
const filterOutNumbers = (arrayOfNumbers: number[]) => {
  return arrayOfNumbers.filter(eachNumber => eachNumber > 10)
}

Any logic that is being duplicated or even used more than twice should live in it's own file (ideally in the /utilities dir)

Bad

class MyComponent extends React.PureComponent<MyProps> {
  return (
    <h1>
    {
      this.props.title
        .split(' ')
        .map(capitalize)
        .join(' ')
    }
    </h1>
    <p>
    {
      this.props.subtitle
        .split(' ')
        .map(capitalize)
        .join(' ')
    }
    </p>
  )
}

Good

import { capitalizeAllWords } from 'src/utilities/word-formatting-utils'

class MyComponent extends React.PureComponent<MyProps> {
  return (
    <h1>{capitalizeAllWords(this.props.title)}</h1>
    <p>{capitalizeAllWords(this.props.subtitle)}</p>
  )
}

Try your best to abstract even components imported from external libraries

We're creating abstractions of all external components, even if that's just an immediate exporting of the component export { default } from '@material-ui/core'. We're doing this for the following reasons;

Provides a common entry point where can make site-wide changes to the components structure or functionality.
Allows us control of the API we consume, regardless of where the component comes from.
The wrapper component allows us to respond to naming/file structure changes made by the component authors.

Importing and Structuring Dependencies

As a rule, always use absolute paths for module imports. Something that looks similar to

import MyComponent from 'src/components/MyComponent';

is much better than

import MyComponent from '../../../MyComponent';

This project relies on a number of third-party dependencies. It us important that when importing those dependencies you import only the necessary files. For example, if I needed to create an Observable using RxJS I would import only Observable and the type of Observable I want to create. This keeps bundle size down substantially.

Bad

import { Observable } from 'rxjs/Rx';

Good

import 'rxjs/add/observable/of';
import { Observable } from 'rxjs/Observable';

Other Things

Toasts

Showing messaging to users is a complex task that varies depending on whether an action is immidiate or scheduled to happen some n time in the future. For all actions that we cannot predict their completion time, we use toasts or snackbar messages.

We're leveraging notistack for all toasts, which is an abstracted HOC built upon material-ui's Snackbar. All MUI's props can be applied to the Snackbar as well

An example of how to use a Toast is as follows:

import React from 'react';

import { InjectedNotistackProps, withSnackbar } from 'notistack';

interface Props extends InjectedNotistackProps {
  /**
   * props here
   */
}

export const Example: React.SFC<Props> = props => {
  const handleClick = () => {
    props.enqueueSnackbar('this is a toast notification', {
      onClick: () => alert('you clicked the toast!')
    })
  }

  return (
    <div onClick={handleClick}>
      Click Me
    </div>
  );
};

export default withSnackbar(Example);

Testing Redux Functionality

Most Redux artifacts are basic functions, so actions and reducers can be exported and tested functionally. Many reducers also include asynchronous actions with Thunk, which generally call API methods from the services library and dispatch multiple actions. Testing these requires more setup:

Mock the services library module that is called by the Thunk in question:

jest.mock('../../../services/instances', () => ({
  getInstances: () => Promise.resolve('return value');
}))

To verify that the Thunk called the correct method, you will have to mock that specific method (in the example above, the getInstances method can't be accessed later in your tests).

  // Using requireMock makes it semantically clear that this import is not used, and avoids TypeScript issues.
  const requests = require.requireMock('../../../services/instances');

  // (In your tests somewhere)

  requests.getInstances = jest.fn(() => Promise.resolve('return something here'));

  it("calls the right method", () => {
    expect(requests.getInstances).toHaveBeenCalled();
  });

To actually test the Thunk, you will need to dispatch it, which requires the creation of a mock store:

import configureStore from 'redux-mock-store';
import ReduxThunk from 'redux-thunk';

const middlewares = [ReduxThunk];
const createMockStore = configureStore(middlewares);
const store = createMockStore({});

You can then dispatch your async actions normally:

await store.dispatch(instances.getInstances() as any);

The mock store allows you to check the actions dispatched by Thunks:

const actions = store.getActions();
// [{ type: ACTION_1 }, { type: ACTION_2, payload: some_payload }]
expect(actions).toEqual([instances.load(), instances.handleError(error)]);

It is also helpful to reset the mock store before each request, to keep the actions history clean:

const store = createMockStore({});
beforeEach(() => {
  jest.resetAllMocks();
  store.clearActions();
});

FilesExpand file tree

CODE_STYLE.md

Latest commit

History

CODE_STYLE.md

File metadata and controls

Code Style

Writing a React Component

Testing Memoized components

Avoiding instance methods that could be made into components.

Abstraction

Importing and Structuring Dependencies

Other Things

Toasts

Testing Redux Functionality