DevTools/CodingStandards: Difference between revisions
(Added ESLint sublimetext integration section) |
(Fixed screenshot) |
||
Line 23: | Line 23: | ||
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. | 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. | ||
[[File: | [[File:Eslint-sublimetext3.png|600px|ESLint in SublimeText 3]] | ||
== Be consistent within a file! == | == Be consistent within a file! == |
Revision as of 08:46, 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.
- Learn more about ESLint,
- learn how to integrate ESLint in your code editor,
- or run it on 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.
Be consistent within a file!
Historical Codestyle
- 80 character JS. Go longer if you need to, but try to keep it to 80.
- 2 spaces for indents (no tabs!)
- aim for 24 lines max per function
- aArguments aAre the aDevil (don't use them please)
- Would take a patch to bulk delete aArguments
/** * Javadoc Comments * @param Object with a little prose describing type in more detail * avoid {Object|arg} syntaxy descriptions * - javadoc type ordering suggestion? * - I suggest type first * - been using closure compiler style: * @param {Type} varname Description * - https://developers.google.com/closure/compiler/docs/js-for-compiler#tag-param */
Now the Good Stuff
"use strict"; !
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?