Using jsbeautify in MobileFrontend - Mobile-l

18 Nov 2014

I explored running jsbeautify [1] on the MobileFrontend codebase and
looked at how the output differs from the current styling. It
introduces various rules that MobileFrontend is currently not adhering
too. MobileFrontend already uses jscs [2] so we want to make sure the
outputs of both are compatible. Here is my report on that with the
recommendation that we should use it.

#Rule 1: object properties defined on a single line.
e.g.
{
foo: 2,
bar: 3
}
NOT { foo: 2, bar: 3 }

I think this would be a good idea to adopt. I will explore if jscs can
enforce this.

# Rule 2: variables that are initialised must be followed by a new
line (although I noted a few odd cases e.g. in Page.js after a "?:"
expression and /MobileWebClickTracking.js
e.g.
var Foo, bar = $( 'div' ),
Baz,
Bar;

not:
var Foo, bar = $( 'div' ), Baz, Bar;

This will be fixed if I implement https://trello.com/c/0dkx0ldL

# Rule 3: All chained events should be indented
e.g.
foo()
.bar();

not
foo().
bar();

Seems like a no brainer. One that happens naturally most of the time.

# Rule 4: Spacing in object parameters
e.g.
foo[ 1 ]
[ 1, 2, 3, 4 ]

not:
foo[1]
[1, 2, 3, 4]

This is different to MediaWiki coding conventions but I can implement
https://github.com/beautify-web/js-beautify/issues/424 to give us
this.
We seem a bit inconsistent ourselves with this convention - let me
know how you think this rule should work in our codebase...

# Rule 5: New line after variable declarations

var x, y, z;

z();

not:
var x, y, z;
z();

Also:
function foo() {}

function bar() {}

not:
function foo() {}
function bar() {}

Seems like a no brainer and shouldn't introduce any major issues with
code review.

# Rule 6: Comments must respect the current indentation level

if () {
...
// If i is 5 we do something special
} else if ( i === 5 ){

}
becomes
if () {
...
// If i is 5 we do something special
} else if ( i === 5 ){

}
We'll have to think about what to do with these comments but I don't
think this should be a blocker to using jsbeautify. It will only pop
up occasionally.

# Rule 7: On long if statements the curly brace must be indented.
And the first condition should be on the same line

if ( enableToc || ...
....
&& baz ) {

not:
if (
enableToc || ...
....
&& baz ) {

Again I'm not sure if this will happen too often. This to me is a sign
that we should be using functions rather than long if statements
personally. Again not a blocker.

Actions:
* Implement https://github.com/beautify-web/js-beautify/issues/424
* You guys need to advise me on how we should handle square brackets
in our codebase in such a way we respect MediaWiki coding conventions
and come up with a consistent style we are happy with and can adhere
to.
* I'll implement https://trello.com/c/0dkx0ldL in some form
* I'll explore if jscs can support objects defined on new line
* When the above are done I recommend we turn on jsbeautify for the project.

I've setup a tracking card for the above work:
https://trello.com/c/5btWf2JN/90-snakes-on-a-plane
and will be looking at these problems today.

[1] https://github.com/beautify-web/js-beautify
[2] https://github.com/jscs-dev/node-jscs