12 KiB

Raw Permalink Blame History

Best practices

This document explains our best practices. Follow these best practices when you're working on our code.

Git branch names

Branch names should start with a Conventional Commits scope like feat/ or fix/. If you're closing an issue with your PR then put the issue number after the scope. Finally, describe the changes in the branch in a few words.

Some good branch names:

feat/13732-cacache-cleanup
fix/15431-gitea-automerge-strategy
refactor/jest-reset-mocks
docs/rewrite-packageRules-section

Avoid branch names like patch-1.

If you don't know the correct Conventional Commit scope: give your branch a descriptive name like issue-1-feature-foo.

If you forgot to pick a good branch name when you started work, then rename the branch before creating the pull request. Read the GitHub Docs, renaming a branch to learn how to rename your branch on GitHub.

General

Prefer full function declarations for readability and better stack traces, so avoid const func = ():void => {}
Prefer interface over type for TypeScript type declarations
Avoid Enums, use unions or immutable objects instead
Always add unit tests for full code coverage
- Only use istanbul comments for unreachable code coverage that is needed for codecov completion
- Use descriptive istanbul comments
Avoid cast or prefer x as T instead of <T>x cast
Prefer satisfies operator over as, read the TypeScript release notes for satisfies operator to learn more
Avoid Boolean instead use is functions from @sindresorhus/is package, for example: is.string

// istanbul ignore next: can never happen

Functions

Use function foo(){...} to declare named functions
Use function declaration instead of assigning function expression into local variables (const f = function(){...}) (TypeScript already prevents rebinding functions)
- Exception: if the function accesses the outer scope's this then use arrow functions assigned to variables instead of function declarations
Regular functions should not access this, but arrow functions and methods may access this
Only use nested functions when the lexical scope is used

Use arrow functions in expressions

Avoid:

bar(function(){...})

Use:

bar(() => {
  this.doSomething();
});

Generally the this pointer should not be rebound. Only use function expressions if you need to dynamically rebind this.

Source: Google TypeScript Style Guide, function declarations.

Code style

Write understandable code

Write code that is easier to read, review and maintain. Avoid "clever" code that's hard to understand.

Prefer verbose code which is easier for others to read and maintain than concise code which may be hard or slower for others to understand. For example, Array reduce() functions are often hard to understand first time, and can be replaced with simpler for loops. A for loop is just as fast but is simpler to understand and maintain.

Write single purpose functions

Single purpose functions are easier to understand, test and debug.

function caller() {
  // ..code..
  calculateUpdateAndPrint(data)
  // ..code..
}

function calculateUpdateAndPrint(...) { /* code */ }

Simplified code:

function caller() {
    // code..
    const res = calculate(data);
    update(res);
    print(res);
    // code..
}

function calculate(...) { /* code */ }
function update(...)    { /* code */ }
function print(...)     { /* code */ }

Keep indentation level low

Fail quickly. Nested code logic is hard to read and prone to logic mistakes.

function foo(str: string): boolean {
  let result = false;
  if (condition(str)) {
    const x = extractData(str);
    if (x) {
      // do something
      result = true;
    }
  }
  return result;
}

Simplified code:

function foo(str: string): boolean {
  if (!condition(str)) {
    return false;
  }

  const x = extractData(str);
  if (!x) {
    return false;
  }

  // do something
  return true;
}

Refactor one thing at a time

Refactor the code or refactor the tests.

Avoid refactoring the code and tests at the same time, this can mask regression errors.

Logging

For WARN, ERROR and FATAL log messages use logger metadata. Also use logger metadata the result is a complex metadata object needing a multiple-line pretty stringification.

For INFO and DEBUG log messages inline the metadata into the log message where feasible. It is OK to not inline metadata if it's complex, but in that case first think whether that much information really needs to be logged.

WARN, ERROR and FATAL messages are often used in metrics or error catching services. These log messages should have a static msg component, so they can be automatically grouped or associated.

Good:

logger.debug({ config }, 'Full config');
logger.debug(`Generated branchName: ${branchName}`);
logger.warn({ presetName }, 'Failed to look up preset');

Avoid:

logger.debug({ branchName }, 'Generated branchName');
logger.warn(`Failed to look up preset ${presetName}`);

Array constructor

Avoid the Array() constructor, with or without new, in your TypeScript code. It has confusing and contradictory usage. So you should avoid:

const a = new Array(2); // [undefined, undefined]
const b = new Array(2, 3); // [2, 3];

Instead, always use bracket notation to initialize arrays, or from to initialize an Array with a certain size. For example:

// [0, 0, 0, 0, 0]
Array.from<number>({ length: 5 }).fill(0);

Source

Iterating objects & containers

Use for ( ... of ...) loops instead of [Array|Set|Map].prototype.forEach and for ( ... in ...).

Using for ( ... in ...) for objects is error-prone. It will include enumerable properties from the prototype chain
Using for ( ... in ...) to iterate over arrays, will counterintuitively return the array's indices
Avoid [Array|Set|Map].prototype.forEach. It makes code harder to debug and defeats some useful compiler checks like reachability

Only use Array.prototype.map() when the return value is used, otherwise use for ( ... of ...).

Source: Google TypeScript Style Guide, iterating objects

Exports

Use named exports in all code. Avoid default exports. This way all imports follow the same pattern.

Source: Google TypeScript Style Guide, exports

Imports

Use ES6 module syntax. For example:

import { square, diag } from 'lib';

// You may also use:

import * as lib from 'lib';

And avoid require:

import x = require('...');

HTTP & RESTful API request handling

Prefer using Http from util/http to simplify HTTP request handling and to enable authentication and caching, as our Http class will transparently handle host rules. For example:

import { Http } from '../../../util/http';

const http = new Http('some-host-type');

try {
    const body = (await http.getJson<Response>(url)).body;
} catch (err) {
  ...
}

Async functions

Never use Promise.resolve in async functions. Never use Promise.reject in async functions, instead throw an Error class type.

Dates and times

Use the Luxon package to handle dates and times. Use UTC to be time zone independent.

Example from our code :

if (end) {
  const now = DateTime.now().toUTC();
  const eol = DateTime.fromISO(end, { zone: 'utc' });
  return eol < now;
}

Unit testing

Separate the Arrange, Act and Assert phases with newlines
Use it.each rather than test.each
Prefer Tagged Template Literal style for it.each, Prettier will help with formatting
- See Example
Mock Date/Time when testing a Date/Time dependent module
- For Luxon mocking see Example
Prefer jest.spyOn for mocking single functions, or mock entire modules
- Avoid overwriting functions, for example: (func = jest.fn();)
Prefer toEqual
Use toMatchObject for huge objects when only parts need to be tested
Avoid toMatchSnapshot, only use it for:
- huge strings like the Renovate PR body text
- huge complex objects where you only need to test parts
Avoid exporting functions purely for the purpose of testing unless you really need to
Avoid cast or prefer x as T instead of <T>x cast
- Use partial<T>() from test/util if only a partial object is required

Fixtures

Use the Fixture class to load fixtures. For example:

Fixture.get('./file.json'); // for loading string data
Fixture.getJson('./file.json'); // for loading and parsing objects
Fixture.getBinary('./file.json'); // for retrieving a buffer

Working with vanilla JS files (renovate/tools only)

Declare types and function prototypes with JSDoc.

Example

Classes

Use Typescript getter setters (Accessors) when needed. The getter must be a pure function i.e.
- The function return values are identical for identical arguments
- The function has no side effects