best-practices.md

Best practices

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

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 union 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
• 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 (as opposed to arrow functions and methods) should not 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 this pointer should not be rebound. Function expressions may only be used if dynamically rebinding this is needed.

Source: Google TypeScript Style Guide, function declarations.

Code simplicity

Write simple code

Simple code is easy to read, review and maintain. Choose to write verbose and understandable code instead of "clever" code which might take someone a few attempts to understand what it does.

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 difficult 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 (!condetion(str)) {
    return false;
  }

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

  // do something
  return true;
}

Logging

Use logger metadata if logging for WARN, ERROR, FATAL, or if the result is a complex metadata object needing a multiple-line pretty stringification. Otherwise, inline metadata into the log message if logging at INFO or below, or if the metadata object is complex.

WARN and above messages are often used in metrics or error catching services, and should have a consistent msg component so that they will be automatically grouped/associated together. Metadata which is separate from its message is harder for human readability, so try to combine it in the message unless it's too complex to do so.

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 i.e.

// [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

Exports

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

Source, reasoning and examples.

Imports

Use ES6 module syntax, i.e.

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. 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 Luxon to handle dates and times. Use UTC to be time zone independent.

Example

Unit testing

• Separate Arrange, Act and Assert phases with empty line
• 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 Fixture class for loading fixtures

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)

Use JSDoc to declare types and function prototypes.

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

Source

• Omit constructors when defining Static classes
• No #private fields. instead, use TypeScript's visibility annotations
• Avoid underscore suffixes or prefixes, for example: _prop, use whole words as suffix/prefix i.e. internalProp

regex

Use Named Capturing Groups when capturing multiple groups, for example: (?<groupName>CapturedGroup).

Windows

We recommend you set core.autocrlf = input in your Git config. You can do this by running this Git command:

git config --global core.autocrlf input

This prevents the carriage return \r\n which may confuse Renovate bot. You can also set the line endings in your repository by adding * text=auto eol=lf to your .gitattributes file.