JS style guidelines (draft)

[miq-bot]

(This is a draft of a future docs document detailing our Javascript style guidelines for ManageIQ and SSUI.)

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 .

---

This issue was moved to this repository from ManageIQ/manageiq#8781, originally opened by @himdel

[himdel]

For discussion, please see the original issue ManageIQ/manageiq#8781

ES6 update:

the rules mentioned here (and in /.eslintrc.json) should apply to all javascript, except for cases where ES6 brings some relevant change (as in, we won't randomly stop using semicolons for es6, but we do discourage var now)

in cases where app/javascript/.eslintrc.json is doing something different, a rule may have been overriden by a plugin/extend and may need to be explicitly re-added to app/javascript/.eslintrc.json to apply again.

[himdel]

Cc @Hyperkid123 (just because you're also dealing with eslint now)

[mfeifer]

@himdel what's the status on this issue? should we close it?

[himdel]

@mfeifer this is a non-issue.

The rules described here were already implemented,
so this now only serves as documentation why.

Maybe we should move the text to the wiki or guides instead, and close the issue,
just don't want to close it without the info existing somewhere :)

[Fryguy]

Maybe we should move the text to the wiki or guides instead, and close the issue,
just don't want to close it without the info existing somewhere :)

Yeah it should move out of issues into wiki or guides.

[himdel]

Alright, closing, moved to https://github.com/ManageIQ/manageiq-ui-classic/wiki/JS-style-guidelines :)