DevTools/CodingStandards: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
(Fixed screenshot)
(Re-formatted the code style rules section)
Line 25: Line 25:
[[File:Eslint-sublimetext3.png|600px|ESLint in SublimeText 3]]
[[File:Eslint-sublimetext3.png|600px|ESLint in SublimeText 3]]


== Be consistent within a file! ==
== Code style ==


== Historical Codestyle ==
Here are some of the main code style rules:


* 80 character JS. Go longer if you need to, but try to keep it to 80.
* lines should be 80 characters maximum (go longer if wrapping hurts readability),
* 2 spaces for indents (no tabs!)
* indent with 2 spaces (no tabs!),
* aim for 24 lines max per function
* 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)
* aArguments aAre the aDevil (don't use them please),
** Would take a patch to bulk delete aArguments
* "use strict"; globally per module,
 
* <code>semicolons; // use them</code>,
/**
* no comma-first,
  * Javadoc Comments
* consider using Task.jsm (<code>Task.async</code>, <code>Task.spawn</code>) for nice-looking asynchronous code instead of formatting endless <code>.then</code> chains,
  * @param Object with a little prose describing type in more detail
* use ES6 syntax:
  *        avoid {Object|arg} syntaxy descriptions
** <code>function setBreakpoint({url, line, column}) { ... }</code>,
  *  - javadoc type ordering suggestion?
** <code>(...args) => { }</code> rest args are awesome, no need for <code>arguments</code>,
  *    - I suggest type first
** <code>for..of</code> loops,
  *    - been using closure compiler style:
* don't use non-standard SpiderMonkey-only syntax:
  * @param {Type} varname Description
** no <code>for each</code> loops,
  *        - https://developers.google.com/closure/compiler/docs/js-for-compiler#tag-param
** no <code>function () implicitReturnVal</code>,
  */
** getters / setters require { },
 
* only import specific, explicitly-declared symbols into your namespace:
== Now the Good Stuff ==
** <code>const { foo, bar } = require("foo/bar");</code>,
 
** <code>const { foo, bar } = Cu.import("…", {});</code>,
"use strict"; !
* use Maps, Sets, WeakMaps when possible.
 
'''semicolons; // use them'''
 
Function names (tags, signatures) sparingly
(function declarations vs. function expressions: http://javascriptweblog.wordpress.com/2010/07/06/function-declarations-vs-function-expressions/) ...vs expression closures, vs. arrow expressions, vs life…
 
Promise.jsm (use that, some bugs still exist?) - bug 881050
* DOM Promises are the longterm winner, we should consider migrating to them
* If you're using SDK promises, make sure usage works when resolution runs on next tick (like Promise.jsm)
 
HOW DO WE FORMAT .THENS?
  return foo().then(v => {
    return foo;
  }).then(v2 => {
    return thing;
  }).then(v3 => {
    return thing;
  }).then(null, console.error);
 
or…
 
  return foo.promise
    .then(() => ... )
    .then(() => ... )
    .then(null, console.error) ?
 
Task.jsm (great for tests)
yield ...;
yield ...;
 
'''DON'T use Mozilla extensions when ES6 alternative is available.'''
* no for each (dcamp will accept patches)
* this also affects function() ... syntax (function expressions)
* don't use moz-specific `function () implicitReturnVal`
* no get foo() this.bar
* it does NOT affect arrow function syntax
* getters / setters require { }!
 
Use maps instead of hashes when possible.
 
new code should be common.js modules not jsms.
 
    comma first lololol no. (go to hell)
    comma middle (gt,fo)
 
=== Imports ===
const { foo, bar } = require("foo/bar");
const { foo, bar } = Cu.import("…", {}); ++
 
to import only specific, explicitly-declared symbols into your namespace from a JavaScript Module (JSM)
 
=== miscellany ===
* e.g., function setBreakpoint({url, line, column}) { ... } is the shizz++
* `(...args) => { }` rest args are awesome, no need for `arguments`
* destructuring needs () in arrow functions
* but don't go crazy with
* function foo({ bar: { baz: { lol: wtf } } })
 
=== tbd ===
Maybe just use an automatic code formatter and be done with it?

Revision as of 09:29, 28 May 2015

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

Code style

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!),
  • 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.
Retrieved from "https://wiki.mozilla.org/index.php?title=DevTools/CodingStandards&oldid=1077169"