Code Of Conduct Frontend Development

Overview

If you are working on the frontend web-app project, please read the following carefully. It's intended to offer guidance on high-level architecture principles that should be followed when developing features.

Table of Contents

• React
  • Components
  • State
  • Effects
  • Hooks
  • Thinking in React
• Browser APIs
• HTTP API Integration
• Styling
• Typings
• Pull Requests

React

At the core of React is a component based model used to encapsulate component rendering logic and user interactions.

It's important to remember the React is just the view part of the application.

Components

Don’t treat components as just another function, they are functions but they’re actually much more than that. They have a unique lifecycle and a powerful API for managing user behaviour (interactions) and presentation via state transitions.

// BAD
function Component() {
  // How many times will this function be called, 
  // Answer, on every single render, is that desirable
  function onetimeThing() {
    ...
  }

  return ( ... );
}

// GOOD
function Component() {
  // Inside useEffect, it only execute once on component mount
  useEffect(() => {
    function onetimeThing() { ... }
  }, []);

  return ( ... );
}

It's important to understand how these events will affect code placed within the component's render function.

Don’t treat component as buckets to place functions or other supporting code. Components should only hold functions and variables directly related to how they should render, or handling user interactions. In practice this means accepting input (props), managing its own state and event handlers. Anything else is probably business logic and should be external in either Redux, or utils functions.

// BAD
function Component() {
  const { values } = props;

  function transformSomething() { ... }

  function handleClick(...) {
    transformSomething(...);
  }

  return <div onClick={handleClick}>{values}</div>;
}

Do place functions that do not directly require a component's state or props outside the component function, possibly in a different file. This will ensure components stay clean and focused on presentation concerns.

// GOOD
import { transformSomething } from "somewhere";

function Component(props: Props) {
  const { values } = props;

  function handleClick(...) {
    transformSomething(...);
  }

  return <div onClick={handleClick}>{values}</div>;
}

Don't use the component lifecycle (i.e render cycles) to manage business logic e.g. interacting with external APIs or manipulating application state that's reliant on these interactions.

Triggering an additional render of the component and potentially it's entire sub-tree can be computationally expensive and degrade the user experience.

State

It's ok for components to hold state that's why React provides the hook useState but think carefully about whether it's needed and importantly where else it might be needed.

Generally speaking state can be divided into two categories, application state and UI state.

• Application state is the core data used by the application, generally persists between app screens etc.
• UI state's sole purpose is to manage the DOM e.g. active, focus elements etc.

Do use component state to manage localised UI logic.

Don't use component state when the state is needed by other components, except immediate children.

Don't use component state when the state is used to drive the state of the application, or effects the wider application in some way e.g. navigation.

Effects

The Effect Hook lets you perform side effects in function components. You can think of useEffect Hook as componentDidMount, componentDidUpdate, and componentWillUnmount combined.

Don't implement side effects in components outside of useEffect

// BAD
function Component() {
  // This will add a new listener every time the component renders, 
  // this is potentially hundreds of times
  document.addEventListener(...);

  return ( ... );
}

Do use useEffect to wrap any code with potential side effects

// GOOD
function Component() {
  useEffect(() => {
    // This will run only once on component mount, 
    // because use use `[]` otherwise as specified by props passed
    document.addEventListener(...);
  }, []);

  return ( ... );
}

Don't leave resources or subscriptions in useEffect, this will cause memory leaks

// BAD
function Component() {
  useEffect(() => {
    // This event listener is added on component mount, 
    // but it will also hang around when the component unmounted
    document.addEventListener(...);
  }, []);

  return ( ... );
}

Do use return function to clean up resources.

// GOOD
function Component() {
  useEffect(() => {
    document.addEventListener(...);

    return () => {
      // This functions is called when the component is unmounted.
      // Use it to clean up any resource, unsubscribe etc.
      document.removeEventListener(...);
    }
  }, []);

  return ( ... );
}

Do use the second argument to provide a list of value (e.g. [document, someHandler]) which will trigger the useEffect hook.

// GOOD
function Component() {
  useEffect(() => {
    document.addEventListener(someHandler);
    // This will only execute on mount, 
    // or when either document or someHandler change
  }, [document, someHandler]);

  return ( ... );
}

We have an eslint rule to enforce an exhaustive list, you should not disable this rule.

Custom Hooks

Hooks are a powerful new tools in the React ecosystem which allow us to develop functional components are first class citizens. They are intended to provide an alternative to class based lifecycle methods.

Do write custom hooks to declutter components for readability, or when it's desirable to reuse the same logic between components.

Reusability is one of the benefits hooks have over class based lifecycle hooks.

Don't write custom hooks when you don't need to observe component lifecycle events or state.

Use your common sense when deciding whether a hook is appropriate.

Thinking in React

READ THIS PAGE OF DOCUMENTATION.

The article explains how to decide which parts of a design can be broken down into components.

Browser APIs

Don't use browser APIs directly in the component unless they are directly required for rendering. As opposed to storage APIs like localStorage which should be managed via application state i.e. Redux.

// BAD
function Component() {
  function handleClick(value: string) {
    localStorage.setItem(value);
  }

  return <div onClick={handleClick}>...</div>;
}

// GOOD
function Component() {  
  function handleClick(value: string) {
    dispatch(setSomething(value));
  }

  return <div onClick={handleClick}>...</div>;
}

function setSomething(value: string): AppThunk {
  try {
    dispatch({ type: SET_SOMETHING, payload: value });
    localStorage.setItem(value);
  } catch (e) { ... }
}

Don't use access browser APIs directly without appropriate error handling, in most case the try catch is most suitable.

// BAD
const value = localStorage.getItem("someValue");

// GOOD
try {
  const value = localStorage.getItem("someValue"); 
} catch (e) {
  // log to sentry
}

Don't use document.location.href to navigate the application programmatically.

// BAD
document.location.href = '/login';

This will tear down the entire component tree, throwing away state and then rebuild it all again. This is particularly problematic on lower spec devices, or other slow internet connections.

Do use history.push to navigate the application

// GOOD - IN COMPONENTS
import { useHistory } from "react-router-dom";

function Component() {
  const history = useHistory();
  history.push('/login');
  ...
}

Do use Redux middleware to interact with browser APIs, for example localStorage.

HTTP API Integration

Use either browser's fetch function or axios package to make API calls. However, once decided, stick to it for the whole project.

Don't make API calls outside effects.

function Component() {
  const [data, setData] = useState();

  fetch('/endpoint')
    .then(res => res.json())
    .then(json => setData(json))
    .catch(e => handleError(e));

  return (
    <>
      ...
    </>
  );
}

This would result in unexpected behaviour like browser sending hundreds of requests per second

Do use effects for API calls.

function Component() {
  const [data, setData] = useState();

  useEffect(() => {
    fetch('/resource/')
      .then(res => res.json())
      .then(json => setData(json))
      .catch(e => handleError(e));
  }, []);

  return (
    <>
      ...
    </>
  );
}

The empty [] indicates the effect is independant of state. Hence React won't re-run the effect hundreds of times.

Do use specialised clients for the task.

const client = axios.create({
  baseURL: 'http/s base url here',
  ... // Any extra axios client configurations, same as request config.
});

For fetch, create a class with all state related to fetch.

Do use request interceptors for adding token from localStorage.

client.interceptors.request.use((config) => {
  const token = localStorage.getItem("token");

  if (!token) {
    return config;
  }

  const newConfig = config;
  newConfig.headers.Authorization = `Bearer ${token}`;

  return newConfig;
});

Don't write fetch calls in each component.

This makes it difficult to track where the API calls are being made. Also, in case the endpoint has changed, it's difficult to refractor. Furthermore, adding data like authentication token or custom headers will require more work if the API calls are distributed across the codebase.

Do make one file called sdk.js or api.js which contains all API calls.

// sdk.js

// Name the exported object as per the resource
// For example, the following is for the `/resource` endpoint
export const resource = {
  /**
   * @param {AxiosRequestConfig} params
   */
  list: (params) => client.get(resource, {
    params,
  }),

  /**
   * @param {string} id
   * @param {AxiosRequestConfig} params
   */
  get: (id, params) => client.get(`/resource/${id}`, {
    params,
  }),

  /**
   * @param {object} data
   * @param {AxiosRequestConfig} params
   */
  create: (data, params) => client.post('/resource/', data, {
    params,
  }),

  /**
   * @param {string} id
   * @param {object} data
   * @param {AxiosRequestConfig} params
   */
  update: (id, data, params) => client.put(`/resource/${id}`, data, {
    params,
  }),

  /**
   * @param {string} id
   * @param {AxiosRequestConfig} params
   */
  delete: (id, params) => client.delete(`/resource/${id}`, {
    params,
  }),
}

// Using in Component.jsx

import { resource } from './sdk';

function Component() {
  const [data, setData] = useState();

  useEffect(() => {
    resource
      .list()
      .then(res => res.json())
      .then(json => setData(json))
      .catch(e => handleError(e));
  }, []);

  return (
    <>
      ...
    </>
  );
}

Note that it is tricky to write such API calls. However, the 20-30 minutes spent on writing such a file will save dozens of hours of debugging, refractoring, and testing. This also ensures that your API calls are unit testable, as you can write jest tests for this file.

Some tips for writing an SDK file:

1. You can use the above code and create a "Mixin" which is a function which takes a base URL like '/resource' and returns the above object with basic CRUD operations.
2. Write unit tests for the file.
3. Use axios client with interceptors to automatically add headers like authentication tokens.
4. The above example uses jsdoc, which uses typescript types to provide intelligent suggestions in VScode for vanilla javascript functions. Hence you will get Axios's type suggestions in plain javascript.

Styling

We use SASS and BEM for defining component based styles that are insulated from css styles leaking across application.

Do define a companion file to hold a component's style.

feature/
  Feature.tsx
  Feature.scss

The file name should match the component's file name so they are

Don't use inline styles except where JavaScript variables are to be interpolated.

// BAD
<div style={{ backgroundColor: "#1d71b8" }} />

// GOOD

// component.tsx
<div style="style" />

// component.scss
.style {
  background-color: "#1d71b8"
}

Do use global variables to share common colour, dimensions etc. between component styles.

// GOOD

// _variables.scss
$blue_100: #1d71b8;

// component.scss
.style {
  color: $blue_100;
}

We currently store them in src/assets/_variables.scss

Do use inheritable style blocks to share common traits across components. An example here is %Clickable which applied the same styling to elements which are intended to be clickable.

// _blocks.scss
%Clickable {
  box-sizing: border-box;
  text-decoration: none;
  outline: none !important;
  cursor: pointer;
}

// component.scss
.style {
  @extend %Clickable;
  ...
}

We currently store them in src/assets/_blocks.scss

Do use meaningful class names based on BEM strategy inline with a component's structure and behaviour.

// BAD
<div className="container">
  <div className="text bold active">...</div>
<div/>

// GOOD
<div className="block">
  <div className="block__element block__element--modifier">...</div>
<div/>

Do use classnames when manipulating class names based on state

import cx from "classnames";

<div className={cx("block__element", {
  "block__element--active": isActive
})}>

Typings

We use TypeScript for type safety, to define and enforce contracts between producers and consumers usage. It's also a useful tool for documenting and communicate data structure and usage and serves as live documentation.

Don't use any type in the code, in 99% of cases this can be avoid with define proper types.

// BAD
function doSomething(someValue: any) {

// GOOD
function doSomething(someValue: string) {

There are a small number of exceptions to this rule to support dynamic data structures, payloads etc. If you use any you should be ready to justify why in your PR.

Don't define inline type definitions. Named types can be reusable and help reduce duplication. The type names also help to convey intent, so try to use meaningful names.

// BAD
async connect({ protooUrl, deviceId }: { deviceId: string; protooUrl: string }) {

// GOOD
interface ConnectArgs { deviceId: string; protooUrl: string };
async connect({ protooUrl, deviceId }: ConnectArgs) {

Pull Requests

Pull requests are an important part of the development process and help to ensure we are committing quality code. It's an opportunity to ask others for feedback and to provide an additional pair of eyes to help highlight any potential issues you may missed, or may not be aware of.

The following PR template and it will be added automatically when you create your PR. You are expected to complete this information as best you can to help the reviewer.

#### Changes proposed in this Pull Request

<!-- Describe in ONE sentence what this pull request does -->

<!--
  Examples:
- This PR fixes a bug where the user could not log in
- Refactor ui components for registration flow
- Update copy on splash landing page
-->

#### Details

<!-- Provide a more detailed overview of the proposed changes. This should communicate the changes in the PR without having to look at the code -->
<!-- If applicable, explain why you did certain choices (used library X because.., disabled flow in Y because..) -->
<!-- Link to relevant tasks, issues, other PRs if they can provide more context-->

#### Demo

<!-- If applicable, add screen shots, screen recordings (aimated gif) or link to video  -->

#### How I tested the changed

<!-- Describe how you tested these changes and why you're confident this is correct -->

#### Checklist

- [ ] I have reviewed the code myself
- [ ] I have updated the readme (if appropriate)
- [ ] The build is green
- [ ] The current state of the PR description matches the changes

<!-- If you push new commits to the PR, ensure you also update the description to match -->

#### Caveats, Yet to be done

<!-- If something is not complete or maybe even introduces bugs, explain what they are -->

#### Notes to reviewer

<!-- Give any information that could be useful for reviewing the code -->
<!-- For example, if there has been a lot of changes due to refactoring, point the reviewer to specific commits or files -->

Use your common sense, not all section always have to be complete and you can always add additional info.