Javascript Style Guide For Mozilla Projects

There doesn’t seem to be one (or I couldn’t find it!). Here are a few guidelines I’ve picked up from recent patch review comments. Please comment if you have additions and corrections, and maybe we can coalesce this into a page on MDC, with examples and the reasoning behind the guidelines.

  • Two space indentation is most common in browser/toolkit code
  • Lines need to wrap at 80 characters
  • Braces are usually same-line, and only used when surrounding a multiline block
  • Use an underscore (“_”) as the private prefix for properties and methods
  • When wrapping a long expression, the operator goes at the end of the line
  • Use spaces instead of tabs
  • Adhere to the style of the code you’re fixing (or fix it, if it’s incorrect!)
  • Use inline comments liberally

References:

  • bz317107, comments 43 and 46
  • bz328159, comment 8 (some of the other comments have good critique as well)

12 Comments on “Javascript Style Guide For Mozilla Projects”

  1. Mossop says:

    The exception to the rule of operator at the end of a wrapped line appears to be the . case. Particularly when instantiating a component where the .createInstance(Components.etc…) appears on the second line.

  2. I’d simplifiy it by just saying: “To the reviewer’s approval”😉

  3. The rules about “lining up the dots” for complicated expressions on multiple lines is applied differently per-reviewed. I personally prefer all operators (including dots) to be at the end of the line, and not to line up the dots (indent the second and subsequent lines by two spaces). I’m in the minority, and most reviewers insist on the dots-lined-up style:

    http://lxr.mozilla.org/mozilla/source/toolkit/components/nsDefaultCLH.js#78

  4. Gijs says:

    I’d tend to agree with comment #2 here by Robert: every project (and reviewer) has their own style. For example, in ChatZilla (and probably Venkman) code, indentation is 4 spaces, and each brace goes onto its own line. Additionally, you don’t use braces if all blocks in an if(/else if/else) construction are one line. You don’t do:

    if (foo)
    someInstruction();
    else
    {
    someOtherInstruction();
    yetAnotherInstruction();
    }

  5. Alex Vincent says:

    There’s a few I believe in:
    * Treat strict warnings as if they were errors. Read http://javascriptkit.com/javatutors/serror.shtml for details.

    * No anonymous functions. function() {} should be forbidden. This causes strict warnings and makes debugging in Venkman harder.

    * If you catch an exception, you must do more than discard them. The following are acceptable:

    try {
    ...
    }
    catch (e) {
    // do nothing, but explain why
    }

    or

    try {
    ...
    }
    catch (e) {
    dump(e);
    }

    (Note Composer code has empty catch blocks everywhere. This is annoying to someone that has never read the code before.)

    * Use JavaDoc liberally. It's just very very good for documentation purposes. (I actually have a partial proposal for writing JavaDoc-style comments for XBL bindings.)

    On adhering to the local code style, I'd suggest that fixing it should take place only for new functions. peterv noted for a C patch I submitted recently that changing indentation only "messes up CVS blame". The same caution applies to JS files.

    Fixing local code style is good, but make that happen in a separate bug.

  6. Joe Hughes says:

    Cool, thanks for bringing this up, Dietrich. I’ve been wanting to codify this stuff for a while (knowing that we might never get 100% agreement on some of the points). For more data points, I’ve gone through my submission history and compiled a list of style nits that reviewers (mostly Ben G.) have brought up:

    * When breaking lines, operators (including dots) should be at the end of the previous line (though I’ve gotten conflicting advice about this from different people in the same review)

    * No blank lines between JavaDoc-style comments and the function definition

    * No anonymous functions. When specifying a method in an object prototype, use the form:
    {
    doSomething: function TN_doSomething() {

    }
    }
    where TN is the initials of the object type name

    * When not assigning, prefer i to i

    * Member variables and private function names should be prefixed with “_”

    * Prefer
    if (a == b)
    to
    if (a==b)

    * Do
    }
    else {
    instead of
    } else {

    * Javascript preprocessor #includes are better at the end of the file so as to not unduly mess up line numbering

    Alex, I’m very interested in better XBL commenting–could you post what you have of your proposal?

  7. Joe Hughes says:

    Ah, I found Alex’s XBL/JavaDoc post in his blog archives.

  8. Ian Thomas says:

    I know its only a little thing, but do we really want to encourage people to leave out the braces on single line if statements etc?

    Maybe its just me, but I usually find code easier to read when the braces are there, and it also removes the need to add (or remove) the braces when an if statement expands to more than one line.

    I’ve seen both styles in mozilla code.

  9. dietrich says:

    Re: comment #4 by Gijs,

    I was hoping that Robert was poking fun at a current truism, and not necessarily encouraging it.

    I agree that projects can, will, and should be able to have different coding styles. Within projects however, I think that “reviewer’s choice” is confusing and demoralizing to contributors.

    Having consistent and published coding styles for projects will lower the barriers to entry for contributors, as well as ease the burden on our overloaded reviewers. It’s a win for everyone.

  10. Jorge says:

    I absolutely agree with Ian on comment #8. I really don’t think that omitting braces in single line expression blocks is a good programming practice. It makes the code a little harder to read because it introduces inconsistency. Comment #4 is a very good example of this.
    Furthermore, it lends itself to problems in debugging and scalability. You have to be very careless not to realize the absence of braces, but it might happen that you want to modify an if or an else and don’t realize it was a one line block without braces. It also makes it more complicated to do automated code replacements, such as a very massive Find/Replace operation.

  11. Ben says:

    As for braces on single line if statements: I don’t do it myself but understand it’s a personal preference on the part of many folk, because it allows them to more easily insert debugging information or other code without having to insert the braces and then remove them…