DevTools/CodingStandards

From MozillaWiki
< DevTools
Revision as of 14:14, 9 June 2015 by Tromey (talk | contribs) (add emacs instructions)
Jump to navigation Jump to search

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 coding 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 the JS code can be analyzed automatically.

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

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).

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.

ESLint in SublimeText 3

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 ()
  (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:

  • 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.