Linters

Flipper Desktop comes with a variety of ESLint checks pre-enabled, which enable us to enforce sustainable coding practices and skip over discussions in code reviews.

Specific Linters

This section contains an incomplete list of unusual linters we deploy, why we use them, and how to fix them (where relevant).

promise/no-nesting

• Summary - avoid nested then() or catch() statements. For more details, see no-nesting.md on GitHub.
• Why - nested promise chains can be difficult to read and understand. Often, you can achieve the same result by either returning the promise and handling them on a higher level or converting them to an async function.

Example

Before

private pushFileToiOSDevice(
  udid: string,
  bundleId: string,
  destination: string,
  filename: string,
  contents: string,
): Promise<void> {
  return tmpDir({unsafeCleanup: true}).then((dir) => {
    const filePath = path.resolve(dir, filename);
    promisify(fs.writeFile)(filePath, contents).then(() =>
      iosUtil.push(
        udid,
        filePath,
        bundleId,
        destination,
        this.config.idbPath,
      ),
    );
  });
}

After

async pushFileToiOSDevice(
  udid: string,
  bundleId: string,
  destination: string,
  filename: string,
  contents: string,
): Promise<void> {
  const dir = await tmpDir({unsafeCleanup: true});
  const filePath = path.resolve(dir, filename);
  await fs.writeFile(filePath, contents);
  return iosUtil.push(
    udid,
    filePath,
    bundleId,
    destination,
    this.config.idbPath,
  );
}

In addition to less indentation, you also maintain the promise chain here, meaning that you can handle potential errors on the call-side.

flipper/no-console-error-without-context

• Summary - avoid 'naked' console.error calls. Prefer console.error("Failed to connect open iOS connection socket", e) to console.error(e).
• Why - we create error tasks internally for every console.error call. It can be hard to find the origin of the error without context.

Example

Before

try {
    // ...
} catch (e) {
    console.error(e);
}

After

try {
    // ...
} catch (e) {
    console.error(`Failed to connect to paste host ${hostname}`, e);
}

promise/catch-or-return

• Summary - ensure that each time a then() is applied to a promise, a catch() is applied as well. Exceptions are made if you are returning that promise. For more details, see catch-or-return.md on GitHub.
• Why: Unhandled exceptions have no stack trace and will just show up as "Unhandled promise rejection", making them very hard to triage and reproduce. By always ensuring that promises are returned (ensuring they are a chain) or explicitly catching errors, we can improve the user experience by acting more quickly on errors.

Example

Before

function request() {
    // If fetch() fails, the exception will bubble to the top.
    fetch("https://example.com").then(res => {
        doSomethingWith(res);
    });
}

After

// Option 1
function request() {
    fetch("https://example.com").then(res => {
        doSomethingWith(res);
    }).catch((e) => {
        console.error("Failed to fetch from example.com", e);
    });
}

// Option 2
function request() {
    // Allow the call-site to handle the error.
    return fetch("https://example.com").then(res => {
        doSomethingWith(res);
    });
}

communist-spelling/communist-spelling

• Summary - we try to avoid using British spellings for identifiers.
• Why - this is clearly controversial, but it's very inconvenient when you have to bridge American and British APIs. const greyColour = COLORS.GRAY; is something nobody should have to read or write.

Example

Before

const GreyedOutOverlay = initialiseComponent();

After

const GrayedOutOverlay = initializeComponent();

no-restricted-properties

• Summary - try to avoid using electron.remote directly.
• Why - using electron.remote is considered harmful. Not only is accessing it slow but it also makes it harder to abstract away from Electron, which we're planning to do.
• How to fix it - for now, the best way is to use utilities, wherever possible, which cache to access. This is not always possible. Adding ignores for legitimate accesses is fine.

Example

Before

import electron from 'electron';
const portforwardingClient =
  path.join(
    electron.remote.app.getAppPath()
    'PortForwardingMacApp.app',
    'Contents',
    'MacOS',
    'PortForwardingMacApp'
  );

After

import {getStaticPath} from './utils/pathUtils';
const portforwardingClient = getStaticPath(
  path.join(
    'PortForwardingMacApp.app',
    'Contents',
    'MacOS',
    'PortForwardingMacApp',
  ),
);

node/no-sync

• Summary: Use asynchronous methods wherever possible. More details.
• Why: Synchronous method calls block the event loop. Even innocuous calls like fs.existsSync() can cause frame drops for users or even long stalls.
• How to fix it: We have fs-extra as a dependency, which provides Promise-based alternatives for all fs functions. Most often, replacing a sync call with an async call and adding an await is all that's needed.

Before

import fs from 'fs';
function ensureCertsExist() {
    if (
        !(
        fs.existsSync(serverKey) &&
        fs.existsSync(serverCert) &&
        fs.existsSync(caCert)
        )
    ) {
        return generateServerCertificate();
    }
}

After

import fsExtra from 'fs-extra';
async function ensureCertsExist() {
    const allExist = Promise.all([
        fsExtra.exists(serverKey),
        fsExtra.exists(serverCert),
        fsExtra.exists(caCert),
        ]).then((exist) => exist.every(Boolean));
    if (!allExist) {
      return this.generateServerCertificate();
    }
}