Contributor Guide

This document collects various tips, conventions, and procedures that may be useful to Popcode contributors. In particular, any code review feedback that invokes a general principle should be accompanied by a section in this document (which will be expanded as demanded by that rule).

Code style

Most code style conventions are enforced by an extensive ESLint configuration, but there are a handful that can't be expressed as ESLint rules.

Multiline lists of things

If a list of things (array elements, object entries, function arguments/parameters, etc.) does not fit on a single line, project style is to put each element on its own line, and also put the start and end delimiters on their own line:

const toppings = [
  'cheese',
  'mushrooms',
  'olives',
  'green peppers',
  'roasted red peppers',
  'sun-dried tomatoes'
];

const meals = {
  breakfast: 'Dunkin donuts breakfast sandwich',
  lunch: 'Falafel sandwich',
  dinner: 'Burrito',
};

function findSnack(
  hungerLevel,
  dietaryRestrictions,
  crunchiness,
  saltiness,
  sweetness
) {
  // implementation left to the reader
}

Destructuring should reduce repetition without reducing clarity

Destructuring can make code much more readable, but there are plenty of situations in which it has the opposite effect. Generally, use destructuring when it reduces repetition in code, and avoid it when it makes the code less clear.

For example, this code:

const {current: editor} = editorRef;

can be written much more clearly, and with no added repetition, as:

const editor = editorRef.current;

On the other hand, this code:

const type = action.type;
const payload = action.payload;

can use destructuring to remove repetition of the tokens type, action, and payload:

const {type, payload} = action;

Destructuring function parameters is nearly always a good idea.

Managing dependencies

Avoiding unnecessary duplicate versions

yarn’s dependency resolution algorithm often results in multiple versions of the same package being installed, even when there is one version that satisfies all constraints. The yarn-deduplicate tool is designed to mitigate this by operating on an existing yarn.lock file, merging together duplicate dependencies wherever possible. To run it in Popcode, run this:

$ yarn deduplicate

Popcode’s CI includes a check for duplicate dependencies, and will fail if yarn deduplicate needs to be run.

Avoiding unnecessary version changes in yarn.lock

If your pull request adds, updates, or removes package dependencies, it’s important that the changes to the yarn.lock file reflect the differences in package.json, and nothing more. Particularly in the case of a merge conflict with master, yarn will sometimes inadvertently upgrade unrelated package dependencies when auto-resolving the conflict.

The easiest way to ensure that the changes to yarn.lock are kept to the necessary minimum is to do this:

$ git checkout master -- yarn.lock
$ yarn install
$ yarn deduplicate

If you’re dealing with a merge conflict in the middle of a rebase, do this instead:

$ git checkout HEAD -- yarn.lock
$ yarn install
$ yarn deduplicate
$ git add yarn.lock
$ git rebase --continue