DevTools/CodingStandards

The DevTools JS code base is quite large, and a lot of different people contribute to it all the time, so it's important that a set of standards be shared when coding so that the code is consistent, written in a predictable style that also hopefully helps avoid common mistakes.

JS linting with ESLint

In order to help people write standard-compliant code from the start and avoid wasting time during code reviews, a set of ESLint configuration files have been added to the DevTools code base so that JavaScript can be analyzed automatically.

This automatic linting can happen either while coding, in a code editor, or when using the command line.

• Learn more about ESLint,
• learn how to integrate ESLint in your code editor,
• or run it on the command line. Note that ESLint has been integrated to our build system and can therefore be run on the command line with ./mach eslint.

Our ESLint rule set is meant to be used with ESLint 1.10.3 at the moment, and is not compatible with ESLint 2.0.0.

Installing ESLint and Recommended plugins

From the root of the project type:

./mach eslint --setup

NOTE: If you are not an administrator you either need to use an administrator account or use sudo depending on your operating system.

ESLint, eslint-plugin-mozilla and eslint-plugin-react will be automatically downloaded and installed.

Running ESLint from the command line

The preferred way of running ESLint from the command line is by using mach like this:

./mach eslint path/to/directory

This makes sure that ESLint runs with the same configuration that on our CI environment (see the next section).

Note that the file .eslintignore at the root of the repository contains a list of paths to ignore. This is because a lot of the code isn't ESLint compliant yet. We're in the process of making code free of ESLint warnings and errors, but this takes time. In the meantime, make sure the file or folder you are running ESLint on isn't ignored.

Running ESLint in CI

Relying only on people to run ESLint isn't enough to guarantee new warnings or errors aren't introduced in the code. Therefore, ESLint also runs automatically in our Continuous Integration environment. This means that every time a commit is pushed to one of the repositories, a job runs ESLint on the whole code.

If you are pushing a patch to the try repository to run the tests, then you can also tell it to run the ESLint job and therefore verify that you did not introduce new errors. If you build on all platforms, then the ESLint job will run by default, but if you selected a few platforms only in your trysyntax, then you need to also add eslint-gecko as a target platform for ESLint to run.

Running ESLint in SublimeText

SublimeText is a popular code editor and it supports ESLint via a couple of plugins. Here are some pointers to get started:

• make sure you have SublimeText 3, the linter plugin doesn't work with ST2,
• install SublimeLinter 3, this is a framework for linters that supports, among others, ESLint, (installing SublimeLinter via Package Control is the easiest way),
• with SublimeLinter installed, you can now install the specific ESLint plugin (the installation instruction provide details about how to install node, npm, eslint which are required).
• make sure to configure SublimeLinter with the --no-ignore option so that errors are also shown for source files that are ignored. To do this, open the SublimeLinter user configuration at: Preferences / Package Settings / SublimeLinter / Settings - User, and add "args": "--no-ignore" to the eslint linter object.

Once done, open the mozilla project in SublimeText and open any JS file in either /browser/devtools or /toolkit/devtools (the 2 main directories for DevTools code). You can then trigger the linter via the contextual menu (right click on the file itself) or with a keyboard shortcut (ctrl+option+L on Mac).

You should see errors and warnings in the gutter as shown in the screenshot below. You can also see all errors listed with ctrl+option+A, and individual errors by clicking in the gutter marker.

Running ESLint in Emacs

• First, install the flycheck package (flymake doesn't support ESLint yet). You can get flycheck from the marmalade or melpa-stable repositories.

• Tell flycheck to disable jslint, and enable flycheck in your javascript mode. This snippet assumes the built-in javascript mode, but with minor changes (just the name of the hook) should work fine with js2-mode as well:

(defun my-js-mode-hacks ()
  ;; Set this locally so that the head.js rule continues to work
  ;; properly.  In particular for a mochitest we want to preserve the
  ;; "browser_" prefix.
  (when (buffer-file-name)
    (let ((base (file-name-nondirectory (buffer-file-name))))
      (when (string-match "^\\([a-z]+_\\)" base)
	(setq-local flycheck-temp-prefix (match-string 1 base)))))
  (flycheck-mode 1))
(require 'flycheck)
(setq-default flycheck-disabled-checkers
	      (append flycheck-disabled-checkers
		      '(javascript-jshint)))
(add-hook 'js-mode-hook #'my-js-mode-hacks)

• flycheck puts its bindings on C-c ! by default, so use C-c ! C-h to see what is available. There are key bindings to list all the errors and to move through the errors, among other things.

Code style

Probably the best piece advice there is when talking about the code style is be consistent with the rest of the code in the file.

The primary source for the style guide is the .eslintrc; but for quick reference here are some of the main code style rules:

• file references to browser globals such as window and document need /* eslint-env browser */ at the top of the file
• lines should be 80 characters maximum (go longer if wrapping hurts readability),
• indent with 2 spaces (no tabs!),
• camelCasePlease,
• don't open braces on the next line,
• don't name function expressions: let o = { doSomething: function doSomething() {} };,
• aim for short functions, 24 lines max (ESLint has a rule that checks for function complexity too),
• aArguments aAre the aDevil (don't use them please),
• "use strict"; globally per module,
• semicolons; // use them,
• no comma-first,
• consider using Task.jsm (Task.async, Task.spawn) for nice-looking asynchronous code instead of formatting endless .then chains,
• use ES6 syntax:
  • function setBreakpoint({url, line, column}) { ... },
  • (...args) => { } rest args are awesome, no need for arguments,
  • for..of loops,
• don't use non-standard SpiderMonkey-only syntax:
  • no for each loops,
  • no function () implicitReturnVal,
  • getters / setters require { },
• only import specific, explicitly-declared symbols into your namespace:
  • const { foo, bar } = require("foo/bar");,
  • const { foo, bar } = Cu.import("…", {});,
• use Maps, Sets, WeakMaps when possible.