JS style guidelines

This is a draft document detailing our Javascript style guidelines for ManageIQ and SUI. For history, see https://github.com/ManageIQ/manageiq/issues/8781 and https://github.com/ManageIQ/manageiq-ui-classic/issues/5045.

Do note that this document currently predates es6 support in manageiq, and thus only deals with es5. (Quick note re es6: please always prefer (x) => x, not x => x when using arrow functions, for consistency with the 0 or 2+ arguments variant.)

formatting

indentation

Use 2 spaces per indent level, not 4, nor tabs..

// good
function foo() {
  var x;
  if (x === undefined) {
    console.log('Whee!');
  }
}

// bad
function foo() {
    var x;
    if (x === undefined) {
    console.log("Unwhee");
    }
}

object and array literals

// good
{
  foo: bar,
  baz: quux,
}
[
  foo,
  bar,
  baz,
]

// good (for small objects/arrays)
{ foo: bar, baz: quux }
[ 1, 2, 3 ]

// ok (makes more sense for small values, don't use with anything longer)
[1, 2, 3]

// discouraged
{foo: bar}
[1,2,3,]

trailing commas

Please prefer trailing commas in multiline object or array literals, they are fully supported now and make nicer diffs..

//good
[
  1,
  2,
  3,
]

// discouraged
{
  foo: bar,
  baz: quux
}

// weird
[ 1, 2, 3, ]

function calls

No spaces surrounding the brackets, split long lines after { or [. If you're splitting the call into multiple lines close the parenthesis right after the last argument (same line), and try to keep each argument on a separate line.

// good
foo(1, 2, 3);
bar(foo, {
  a: b,
  c: d,
});

// ok for lots of arguments
foo(1,
    2,
    3)

// bad
quux( { many: many, keys: and, values: true } );

chained methods (esp. then/catch/done/success/etc.)

Please always split chained method calls into multiple lines, starting continuing lines with the dot, and use one indent level for these lines.

// good
$http.get('/foo')
  .then(callback)
  .catch(errback);

var sorted = array
  .map(func)
  .filter(fn)
  .sort();

// discouraged
$http.get('/foo').then(callback).catch(errback);
array.map(func).filter(fn).sort();

if/while blocks

Please use a space before the first parenthesis, use egyptian brackets, and prefer brackets even for single-statement bodies.

// good
if (expression) {
  body;
} else {
  body;
}

// discouraged
if(expression)
  bleh();

expressions

Always surround binary operators with spaces.

// good
(5 + 4 * (3 + 2)) > 7

// bad
5+4*3

switch statements

Unlike in ruby, indent case one indent level more than switch. Also, please explicitly mark case-to-case fall-through with // pass.

// good
switch (x) {
  case 1:
    whatever;
    break;

  case 2:
  case 3:
    whatever;
    // pass
  case 4:
    whatever;
    break;

  default:
    whatever;
}

// bad - case indented badly
switch (x) {
case 5:
  foo;
}

// bad - "hidden" fall-through
switch (x) {
  case 1:
    do_something;
  case 2:
    probably_a_bug;
}

naming

functions

Use camelCase for function/method names.

// good
function miqOnLoad(foo) {...}

// bad
function add_flash(msg) {...}

angular

Prefer full FooController for controllers, FooService for services, FooFactory for factories, foo for directives and foo or foo.bar for modules.

globals

don't

Don't use global global variables, ever.

Also, please try not to create them accidentally by omitting var.

// good
function foo() {
  var bar = 5;
}

// good if you really need a static variable (and don't access it from elsewhere)
function bar() {
  bar.baz = bar.baz || 0;
  bar.baz++;
}

// bad
function foo() {
  bar = 5;
}

// still bad
var whatever = false;
function bar() {
  whatever = true;
}

ManageIQ global object

If you really have to keep global state, add it to the ManageIQ global object, defined in miq_global.js, and add a comment with a purpose.

But please think twice before introducing a new one.

general

comparison operators

Please prefer === and !== to == and !=, but only when it makes sense.

// good
if (x === "5") {...}

// ok, if really checking for undefined or null
if (null_or_undefined == null) { ... }

// usually good enough
if (truthy) { ... }

// discouraged (array.length by itself is perfectly fine there)
if (array.length === 0) { ... }

quoting

Prefer single quotes, unless really inconvenient.

// good
var str = 'We are the Borg';

// bad (be consistent!)
var str = "We" + 'are' + "the" + 'Borg';

// perfectly fine
var str = "I don't want to escape all the apostrophes.";

es6

We do support ES6 now (or a subset thereof - via babel 5) provided the file has an .es6 extension. Please use your judgement and consult config/initializers/sprockets-es6.rb. Also, we support all ES6 library methods (including Promise) in .js files via es6-shim.

/// in .js files..
// bad
const foo = 5;

// ok
var foo = Promise.all([
  $http.get(...),
  $http.get(...),
]);

/// in .es6 files
// ok
const { foo, bar } = { foo: 5, bar: 6, quux: 7 };

// bad
import { foo } from "baar.js";

linting

In manageiq-ui-self_service, you can run gulp vet, which runs jshint and jscs - but the configuration is currently too opinionated. (TODO)

In manageiq, you can run linters manually, as soon as we add their config files.. (TODO)

For anything not specified here, please consult https://github.com/airbnb/javascript .