Permalink
Browse files

a JavaScript Style Guide

  • Loading branch information...
1 parent e20b831 commit 9b640fbea6a941046b8f880f8c3c082c62290a52 @seanmonstar seanmonstar committed May 30, 2012
Showing with 216 additions and 9 deletions.
  1. +1 −9 coding.rst
  2. +215 −0 js-style.rst
View
@@ -222,15 +222,7 @@ See :ref:`packaging`.
Javascript
----------
-* Use JSHint_ — it's like JSLint_ but a bit more reasonable. JSHint
- has options for assuming jQuery, node.js, and other options of use
- to web developers writing JavaScript.
-* Write QUnit tests when possible.
-* Do not write JS in the HTML.
-* Prefer single quotes over double.
-
-.. _JSHint: http://www.jshint.com/
-.. _JSLint: http://www.jslint.com/
+See :ref:`js-style`.
.. index:: code;html5 coding style
View
@@ -0,0 +1,215 @@
+JS Style Guide
+==============
+
+First and Foremost
+------------------
+
+**ALWAYS** use JSHint_ on your code.
+
+- There are some exceptions for which JSHint complains about things in
+ node that you can ignore, like how it doesn't know what 'const' is
+ and complains about not knowing what 'require' is. You can add
+ keywords to ignore to a `.jshintrc` file.
+
+.. _JSHint: http://www.jshint.com/
+
+Variable Formatting:
+-------------------
+
+::
+
+ // Classes: CapitalizedWords
+ var MyClass = ...
+
+ // Variables and Functions: camelCase
+ var myVariable = ...
+
+ // Constants: UPPER_CASE_WITH_UNDERSCORES
+ // Backend
+ const MY_CONST = ...
+
+ // Client-side
+ var MY_CONST = ...
+
+Indentation:
+~~~~~~~~~~~~
+
+2-space indents (no tabs)
@cvan
cvan May 30, 2012 Member

we do four on AMO 😊

@potch
potch May 30, 2012 Member

Why 2?

@seanmonstar
seanmonstar May 30, 2012 Member

I prefer 4, but they made me do it... I think.

@fwenzel
fwenzel May 30, 2012 Member

Not that this is a matter of voting, but 4 has been "webdev standard"[TM] for a long time.

@pmclanahan
pmclanahan May 30, 2012 Member

js === 4 spaces
html === 2 spaces

@darkwing
darkwing May 30, 2012 Member

2 doesn't provide most people enough of a visual indentation, myself included.

@tofumatt
tofumatt May 30, 2012 Member

It does for me (and most Rubyists). And CoffeeScript uses 2 as well.

@fwenzel
fwenzel May 30, 2012 Member

looks like we might have to change this guideline to:

"Indent."

@seanmonstar
seanmonstar May 30, 2012 Member

CoffeeScript does whatever Ruby does. It's not JavaScript.

@potch
potch May 31, 2012 Member

"CoffeeScript does x" is as good an argument as I need against something :D

@lmorchard
lmorchard May 31, 2012 Member

I'd say "Follow what your team does". But, if I'm in there first, I'm starting with 4-space indent.

@tofumatt
tofumatt May 31, 2012 Member

Seemingly only @ednapiranha and I are fans of 2 spaces. Maybe we should do 4 spaces instead. I know @seanmonstar likes tabs, but seemingly 4 spaces is preferred to 2 by nearly everyone except the weird wooby Canucks. I wave a white flag*.

  • So Canadian, eh?
@ednapiranha
ednapiranha May 31, 2012 Member

goddd, let's just get this over with and merged. EVERYBODY HUG.

@lmorchard
lmorchard May 31, 2012 Member

What color should the syntax highlighting be?

@seanmonstar
seanmonstar May 31, 2012 Member

Only solarized is acceptable.

@ednapiranha
ednapiranha May 31, 2012 Member

also devtools does 2 spaces. do we hate devtools? :(

@lmorchard
lmorchard May 31, 2012 Member

You can have 2 spaces, but only if you bust out the One True GNU Brace Style:

if (condition)
  {
    statement1;
    statement2;
  }
@ednapiranha
ednapiranha May 31, 2012 Member

omg that's heathen. DOWN WITH SATAN!!!

@darkwing
darkwing May 31, 2012 Member

I much, much, much, much prefer tabs too.

@robcee
robcee May 31, 2012

Why do you all hate nice things?

@fwenzel
fwenzel May 31, 2012 Member

On a more serious note, perhaps we have to just allow both here. "Indent with 2 or 4 spaces. On an existing project, stick with what the project already does; no mixing indentation styles."

@potch
potch May 31, 2012 Member

The realistic answer is that nobody's going to go back and re-indent an existing project, so I'm with Fred.

@seanmonstar
seanmonstar via email May 31, 2012 Member
+
+For our projects, always assign var on a newline, not comma separated::
+
+ // Not good
+ var a = 1,
@cvan
cvan May 30, 2012 Member

what?! this is totally good!

@potch
potch May 30, 2012 Member

Completely disagree.

@potch
potch May 30, 2012 Member

with 4 space indents, you get the right effect.

@seanmonstar
seanmonstar May 30, 2012 Member

I'm of the opinion that the style for declaring var's doesn't really need to be defined. Either way works fine.

@andymckay
andymckay May 30, 2012 Member

There's lots of pros and cons on this for example its easy to accidentally create a global var. I think it's worth defining.

@seanmonstar
seanmonstar via email May 30, 2012 Member
+ b = 2,
+ c = 3;
+
+ // Good
+ var a = 1;
+ var b = 2;
+ var c = 3;
+
+Use ``[]`` to assign a new array, not ``new Array()``.
+
+Use ``{}`` for new objects, as well.
+
+Two scenarios for ``[]`` (one can be on the same line, with discretion
+and the other not so much)::
+
+ // Okay on a single line
+ var stuff = [ 1, 2, 3 ];
@cvan
cvan May 30, 2012 Member

leading and trailing whitespace?

@seanmonstar
seanmonstar May 30, 2012 Member

Ya @ednapiranha, what's that about? :D

@ednapiranha
ednapiranha May 30, 2012 Member

looks nicer. fine change it to snuggle up against the [] for all i care!

@potch
potch May 30, 2012 Member

I actually kinda like the whitespace.

@tofumatt
tofumatt May 30, 2012 Member

I don't like it much if this is a democracy, but I don't care enough that I'll defend it in follow-up comments or complain if it stays.

@darkwing
darkwing May 30, 2012 Member

For me, the leading and trailing spaces in arrays and parens only make the line longer and contradicts cutting down on indentation. I don't necessarily think that the spaces improve readability either.

+
+ // Never on a single line, multiple only
+ var longerStuff = [
+ 'some longer stuff',
+ 'other longer stuff'
+ ];
+
+Never assign multiple variables on the same line.
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Not good::
+
+ var a = 1, b = 'foo', c = 'wtf';
+
+DO NOT line up variable names.
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Not good::
+
+ var wut = true;
+ var boohoo = false;
+
+Semi-colons
+-----------
+
+**Use them.**
+
+Not because ASI is black-magic, or whatever. I'm sure we all understand
+ASI. Just do it for consistency.
+
+Conditionals and Loops::
+-----------------------
+
+ // Not good
+ if (something)
+ doStuff()
+
+ // Good
+ if (something) {
+ doStuff();
+ }
+
+Space after keyword, and space before curly::
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ // Not good
+ if(bad){
+
+ }
+
+ // Good
+ if (something) {
@darkwing
darkwing May 30, 2012 Member

Much like my comments about not having leading and trailing parens and brackets, I guess I don't see what value the additional space after the "if" provides. I do prefer a space after the closing parent with functions though.

@fwenzel
fwenzel May 30, 2012 Member

It's mostly because "if" is not a function, it's a language construct. You're not calling if.

@tofumatt
tofumatt May 30, 2012 Member

And I like the spaces here, because I find stuff like if(!foo){ or even if(!foo) { harder to parse than if (!foo) {. It makes it clear it isn't being called and, instead, is a conditional. Kinda nice.

@jlongster
jlongster May 22, 2013 Member

I think it's pretty easy to recognize if as a statement; lots of things in languages are ambiguous (think Python's scope) for nice syntax. I never have a problem if thinking if is being called. I'm a glut for terse syntax though.

@ednapiranha
ednapiranha May 22, 2013 Member

we wish you a merry xmas! we wish you a merry xmas! we wish you a merry xmas and a happy new beer!

@tofumatt
tofumatt May 22, 2013 Member

A year ago. A goddamn year ago. You necromancer.

+
+ }
+
+Functions
+---------
+
+Named Functions
+~~~~~~~~~~~~~~~
+
+Always try to name functions, even if assigned to another variable or
+property. This improves error stacks when debugging.
+
+No space between name and opening paren. Space between closing paren
+and brace::
@lmorchard
lmorchard May 31, 2012 Member

I tend to disagree with this, and like a space before and after the function name, eg. function doSomething (argOne, argTwo) { }. That way, maybe it's more clear that doSomething is being defined and not being called (ie. doSomething())

See also, if (foo) { } vs if (foo){ }

@tofumatt
tofumatt May 31, 2012 Member

I think the keyword function beforehand covers it, plus the { /* function contents */ } afterward. I dislike the aesthetics of spaces on either side, and have never been confused by this when defining functions.

+
+ var method = function doSomething(argOne, argTwo) {
+
+ }
+
@lonnen
lonnen May 30, 2012 Member

I don't think there's much to gain by naming a function when it is being assigned to another variable.

@tofumatt
tofumatt May 30, 2012 Member

I find it kind of confusing to read but would love to hear why it's easier for debugging.

What about anonymous functions?

doSomething({
  onSuccess: function successfulThing(arg) {
    return 'yay';
  }
});

Seems a bit verbose and harder to tell something that it's anonymous. Does it improve stack traces for them too? If so, why not name our anonymous functions too?

Sorry for being thick here if I am being it.

@potch
potch May 30, 2012 Member

when an function shows up in a stacktrace it will include its name if it was part of the function declaration.

for example:

$.when(doSomething).done(function(foo) {...});

will show up in a stacktrace as function ?(foo)

$.when(doSomething).done(function somethingDone(foo) {...});

will show up as function somethingDone(foo)

Can be invaluable in a sea of callbacks.

@seanmonstar
seanmonstar via email May 30, 2012 Member
@tofumatt
tofumatt May 30, 2012 Member

Don't you usually get line numbers in said stacktraces too? That would be enough to find the error, no?

(Related: should we say anything about deferred objects or something to avoid callback soup in the guide?)

@tofumatt
tofumatt May 30, 2012 Member

OK, cool, thanks then. Let's amend the anonymous function below (and elsewhere in the guide if they exist) to be named then.

@lonnen
lonnen May 30, 2012 Member

I think you mean remove. If an anonymous function is named it's no longer anonymous.

@tofumatt
tofumatt May 30, 2012 Member

Yeah, that's what I meant. I made a bad in my English. Basically: we shouldn't allow anonymous functions if we're saying this.

+Anonymous Functions
+~~~~~~~~~~~~~~~~~~~
+
+No space between function and opening paren. Space between closing paren
+and brace::
@lmorchard
lmorchard May 31, 2012 Member

Similar to the last comment... I liked when JSLint would yell at me to write function (err, res) { }, because function is a keyword and not a function to be called as function(). It can also be vaguely confused with new Function("..."), which is something else entirely.

@tofumatt
tofumatt May 31, 2012 Member

Given the above discussion: we shouldn't be using anonymous functions at all. Though that JSLint rule is a bit harsh, I think.

+
+ function(err, res) {
+
+ }
+
+
+Operators
+---------
+
+Always use ``===`` (If in doubt, go with the triple).
@tofumatt
tofumatt May 30, 2012 Member

The second sentence (in parenthesis) seems redundant. Just say "always test for equality without type coercion" and be done with it.

+
+Only exception is when testing for null and undefined.
+
+Example::
+
+ if (value != null) {
+
+ }
+
+
+Quotes
+------
+
+Always use single quotes: ``'not double'``
+
+- Only exception: ``"don't escape single quotes in strings. use double quotes"``
+
+Comments
+--------
+
+For node functions, always provide a clear comment in this format::
+
+ /* Briefly explains what this does
+ * Expects: whatever parameters
+ * Returns: whatever it returns
+ */
+
+If comments are really long, also do it in the ``/* ... */`` format
+like above. Otherwise make short comments like::
+
+ // This is my short comment and it ends in a period.
+
+Ternaries
+---------
+
+Try not to use them.
+
+If a ternary uses multiple lines, don't use a ternary::
+
+ // Not good
+ var foo = (user.lastLogin > new Date().getTime() - 16000) ? user.lastLogin - 24000 : 'wut';
+
+ // Good
+ return user.isLoggedIn ? 'yay' : 'boo';
+
+
+General Good Practices
+----------------------
+
+If you see yourself repeating something that can be a constant, refactor
+it as a single constant declaration at the top of the file.
+
+Cache regex into a constant.
+
+Always check for truthiness::
@fwenzel
fwenzel May 30, 2012 Member

This makes my Pythonista self feel warm and fuzzy.

@pmclanahan
pmclanahan May 30, 2012 Member

Github nees a "Like this line" button :)

@tofumatt
tofumatt May 30, 2012 Member

In Ruby, 0 is truthy. I just came here to say that.

@potch
potch May 30, 2012 Member

I just came here to laugh at Ruby a little bit and then wonder if sometimes that's not the right behavior.

@fwenzel
fwenzel May 30, 2012 Member

Man these Ruby people neither have real gems nor real truthiness. WHAT'S WRONG WITH YOU

@willkg
willkg May 30, 2012 Member

I'd like to say that I had to reload this page twice before github decided to show the comments. But I persevered through github hoo-hah because I found all this pretty interesting.

@ednapiranha
ednapiranha May 31, 2012 Member

fyi i wrote that. someone give me some cotton candy and bourbon!

+
+ // Not good
+ if (blah !== false) { ...
+
+ // Good
+ if (blah) { ...
+
+
+If code is really long, try to break it up to the next line or
+refactor (try to keep within the 80-col limit but if you go a bit past
+it's not a big deal). Indent the subsequent lines one indent
+(2-spaces) in.
+
+If it looks too clever, it probably is, so just make it simple.
+
@cvan
cvan May 30, 2012 Member

trailing whitespace

@fwenzel
fwenzel May 22, 2013 Member

always.

3 comments on commit 9b640fb

@potch
Member
potch commented on 9b640fb May 30, 2012

Additional style points we use on AMO:

  • we use $ before variables that are known jQuery objects (a bit hungarian, but prevents us from "double dollaring" things which can get expensive)

for example:

var $el = $('#el');
  • Something about using $.Deferred (or other promise implementations) to standardize APIs around async/callback flows
@pmclanahan
Member

+1 to what @potch said about $vars. I've been doing that for years; really helps.

@darkwing
Member

+1 as well. Bonus points for not using jQuery :D

Please sign in to comment.