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',
};

const dietRegime = {
  breakfast: [
    'Fruit Smoothie',
    'Egg',
  ],
  lunch: [
    'Whole Wheat Turkey Sandwich',
    'Low-fat Greek Yogurt',
    'Apple',
  ],
  dinner: 'Salad',
};

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;

A more complicated destructuring example:

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

can be rewritten as:

const {type, payload: {credentials}} = action;

Special note, in this destructuring scenario, typeof payload === "undefined"

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

Deprecations

Certain patterns in our code are deprecated, meaning that they should not be used in new modules, and they will eventually be refactored to use the currently preferred pattern in all existing cases. Specifically, look out for:

Reducers with switch

Writing reducers using a switch statement is deprecated, although most reducers currently are written this way. For new reducers, prefer handleActions as demonstrated in the resizableFlex reducer.

Sagas

Our use of redux-saga has been deprecated in favor of redux-logic, which provides analogous functionality to sagas but allows us to write asynchronous code using standard async/await structures. New asynchronous/side-effect/business logic code should be written in a logic, not a saga.

Logics live in src/logic and each logic should be its own module.

Tape/Karma tests

Our tape/karma test suite is deprecated in favor of Jest unit tests. Jest tests can be found in __test__ directories throughout the codebase.

Both the deprecated tape/karma suite and the Jest suite are run as part of the CI build. It’s fine to modify existing tape tests to reflect changes in existing functionality, but new tests should be written in Jest.