Meteor Style Guide - acemtp/meteor-france GitHub Wiki

This is a set of guidelines for writing clear, organized, high-quality code that is consistent with our current code.

Please before reading further, install a linter on your favorite editor.

Also, If you need a JS feature that you think is common enough to be already available somewhere, check out the docs of the JS libs we are using for that purpose:

• Underscore.js - Most general methods such as contains(), toArray() and so forth.

The first line of a commit should be 50 characters or less. This is a soft limit, so that git's command line output is more readable. Use this line as a clear summary of the change. Try using imperative language in this line. A good commit message is "Add reactive variable for CPU clock speed". Worse examples include "cpu clock speed" or "Added Meteor.CPU_speed so that we can avoid a setTimeout in the main example."

Subsequent lines (if any) can be fairly verbose and detailed. Use your judgement.

• 2 space indents.
• spaces, not literal tabs.
• no trailing whitespace. (install a plugin to automate this task)
• adds a trailing newline at the end of every file. (automated on Atom)

Emacs users should check out js2-mode for a nice way to avoid silly javascript errors, and help enforce standards.

Try to stick to 80 characters per line. Comments should be line-wrapped to under 80 characters. It's OK to occasionally go over 80 characters if it substantially improves later indentation, or if there is no good way to break the line.

Separate tokens with a single space as in these examples:

a = b + 1, not a=b+1

function (a, b, c) { return 1; }, not function(a,b,c) {return 1;}

for (i = 0; i < 3; i++), not for(i=0;i<3;i++)

a(1, 2, 3), not a(1,2,3)

{ a: 1, b: 2 }, not {a:1,b:2}

if (a), not if(a)

Unary negation also takes a space:

if (! a), not if (!a)

But increment and decrement operators don't take a space:

a++, not a ++

--b, not -- b

When functions or objects fit entirely on a single line, put a space inside the enclosing braces:

stack.push({ parent: node, red: true }), not stack.push({parent:node, red: true})

a(function () { return true; }), not a(function () {return true;})

Single-line arrays don't get that space though:

samples.concat([1, 2, 3]), not samples.concat([ 1, 2, 3 ])

Code should be well commented. Make sure to document the most important pieces of information for both users of the code and future maintainers of the code.

Most functions should begin with a comment explaining what they do, what arguments they take, and what they return. A good function comment allows developers to use a function without having to read it's source.

Most files should start with a comment explaining what functionality is in the file. A good file comment allows developers to understand how the different pieces in the system fit together without having to read a lot of code.

Also comment liberally inside functions. For example, objects saved as class members should note what the object keys and values are. Any tricky bits of code should have a comment explaining what is going on. Focus on documenting why the code is the way it is, not what it does.

Generally prefer // comments to /*. Use /* comments when putting a comment in the middle of a statement. Javadoc /** style comments are OK for function definitions.

Bad:

var Whirlygig = function (name) {
  var self = this;
  self.name = name;
  self.values = {};
};

_.extend(Whirlygig.prototype, {
  /* add a value */
  addValue: function (x) {
    self.values[x.key] = x.value - 1;  // subtract 1 from value
  },

  /* get serialized list */
  serialize: function () {
    return _.map(self.values, function (v, k) {
      var newVal = mungeValue(v, false);
      return {key: k, value: v + 1};  // add 1 to values
    });
  }
});

Good:

// A Whirlygig represents a remote client's view of the global Weasel.
// It can receive arbitrary values from the network and save them
// for later use.
var Whirlygig = function (name) {
  var self = this;
  self.name = name;  // name of the remote weasel
  self.values = {};  // remote key name -> 0-indexed value

};

_.extend(Whirlygig.prototype, {
  // Take a key/value pair from the remote Weasel and save it locally.
  addValue: function (x) {
    // Weasels use 1-indexed arrays. Subtract 1 to convert to 0-indexed.
    self.values[x.key] = x.value - 1;
  },

  // Return a list of stored values in a format suitable for sending to
  // a Weasel.
  serialize: function () {
    return _.map(self.values, function (v, k) {
      var newVal = mungeValue(v, false /* foldValue */);
      // Weasels use 1-indexed arrays. Add 1 to convert back to 1-indexed.
      newVal = newVal + 1;
      return {key: k, value: newVal};
    });
  }
});

Functions should be named like doAThing, not do_a_thing. Other variable names get the same treatment: isConnected, not is_connected.

If a method or property name starts with _, it's a private implementation detail and shouldn't be used from outside the module where it is defined. It could disappear or change at any time.

Use this brace style:

if (a < 0) {
  console.log("a is negative");
  handleNegativeA();
} else if (a > 0) {
  console.log("a is positive");
  handlePositiveA();
} else {
  console.log("a is zero or NaN");
  handleOtherA();
}

You may leave out the curly braces when the block's condition and its body are each short enough to fit on one line:

if (! alreadyDone)
  doSomething();

while (node.parentNode)
  node = node.parentNode;

// BUT NOT:
if (somethingVeryVeryVeryVeryLong &&
    somethingElseLong)
  shortBody();
// AND NOT:
if (something)
  someLongFunctionCall(
      withThisArg,
      andThatArg);

If a function takes more arguments than there are formal parameters (via arguments), comment like so:

_.extend(Foo.prototype, {
  myVariadicFunction: function (/* arguments */) { ... },
  anotherVariadicFunction: function (x, y, z, /* arguments */) { ... },
});

(Not /* varargs */ -- that's a C concept.)

For methods that are more than a line or two long, always begin with var self = this, and always use self, never this. Ideally, do this everywhere. (Rationale: if the method contains callbacks or other local functions, it is far too easy to write this inside the function, expecting to get the outer this. You need to use self in those cases. At that point, it's too hard to remember when to use self and when to use this, so you need to use self everywhere.)

For example, don't name a method 'return', even if that's allowed by the Javascript grammar. Some emacs syntax highlighting modes will choke on that. Even if that doesn't happen, it's still grating to see it highlighted as a keyword. What's more, old browsers may fail to parse the code!

This goes both for actual JavaScript keywords, as well as random words don't currently have special meaning in JS, but which at some point were reserved, or rumored to be reserved, or whatever, and thus have ended up in people's syntax highlighting tables. For example, 'class'. Lists of such words can be found here.

Sometimes you will have to work with a third-party library that does not follow this rule. A prime offender is Future from the node-fibers library, which has a method named return. In this case you should use a string constant to protect the keyword, like so:

var f = new Future;
f['return'](123); // not f.return(123) since return is a keyword

It's important that you do this, because if the code ended up in a file that is used on both the client and the server, then it could fail to parse on old browsers. The parse failure will happen even if the code can not actually ever run on the client.

Use the native Javascript system: new and prototypes. Begin your constructor function with (1) var self = this, and then (2) define every single attribute that will ever be defined on the object, in straight-line code, commenting each one. If any of them are public, indicate this in the comments. Then, define methods on the class by extending its prototype. Private methods should start with _. For public (API) classes, avoid private methods if possible/convenient, but don't go to extreme lengths.

Don't use global variables (or a closure) instead of an object just because the object would be a singleton. That is: if you have something that can logically be seen as an object, with attributes and methods that act on the attributes, make it a proper object even if there will only ever be one instance of that object.

Don't make the attributes into file-scope variables and make the methods into exported functions, and don't use the closure-based module pattern from "Javascript: The Good Parts".

Instead, use the normal class pattern for your object, and then instantiate the object once at the end of the file.

Good criteria for deciding whether you should use an object or file-scope variables:

• In the future, is it even remotely conceivable that someone would ever want to have two instances of this thing?
• Even if not, does this functionality add up to something that is logically an object? (Is it a set of methods that control access to some private state? If not, would it be clearer if it were expressed that way?)

Reserve undefined to mean "argument not provided" or "no such key in object", or for cases where null would signify some other valid value. Use null in other cases, especially the case where a variable's type is "Foo or nothing", eg, "Array<String> or null", "String or null" (analogous to NULL in C, null in SQL, foo option in ML...)

Note that undefined is not a valid EJSON or JSON value, while null is.

Always use ===, not ==.

If you want to compare a number to a string version of said number, use x === parseInt(y), x.toString() === y, or x === +y. It is much more clear what is going on. (Note that those alternatives differ in how they handle non-numeric characters or leading zeros in the string. Only the third alternative gets all the edge cases right.)

If you have a value that could be truthy or falsey, and you want to convert it to true or false, the preferred idiom is !!:

var getStatus = function () {
  // Return status (a non-empty string) if available, else null
};

var isStatusAvailable = function () {
  return !! getStatus();
};

|| is handy for introducing default values:

this.name = this.name || "Anonymous";
_.each(this.items || [], function () { ... });

But be careful. This doesn't work if false is a valid value.

&& can be handy when conditionally calling a function for its value:

// Preferred
return event && handleEvent(event);

// Not preferred
return event ? handleEvent(event) : null;

We also like to use it to call a function only if some other value is present:

// Preferred
for (var node = first; node; node = node.parent) {
  node.label && node.callback && node.callback(node.label);
}

// Not preferred (unless it's clearer in a particular case)
for (var node = first; node; node = node.parent) {
  if (node.label && node.callback)
    node.callback(node.label);
}

It's true that this confuses people who haven't seen && used in this way before. But once you're used to it it's easy to read and convenient. We think it's worth the tradeoff.

This is an idiom borrowed from Python. It does the same thing as a ? b : c, except that if a and b are both false, it returns c. You should use a && b || c only where a ? b : c won't do the job.

// If we have a name, return the entry for it in Template, if any. Otherwise return {}.
return name && Template[name] || {};

// This is subtly different. It will return undefined if name is truthy but isn't present in Template.
return name ? Template[name] : {};

Take a moment to convince yourself that a && b || c works as advertised, then memorize its behavior and don't give it another thought.

If a function takes a lot of arguments, especially if many of those arguments are optional, or several of the arguments are functions, don't try to put all of the arguments in the function's signature. Instead, add a final argument, options.

var makeCube = function (owner, options) {
  // Provide default values for the options
  options = _.extend({
    color: 'grey',
    weight: 1.0,
    material: 'aluminum',
    onRender: function () {},
    onDestroy: function () {}
  }, options);

  console.log(owner + ", here is your " + options.color + "cube.");
  options.onRender();
  ...
});

Keep the most important arguments in the signature (eg, owner above.) Ideally, the arguments in the signature are the "cake" (the main substance and structure of the operation), and the options are the "icing" (frills and modifiers.)

However, this is just a guideline. You should do whatever will make life easiest for the person calling the function. Consider the situation from their point of view. What are they trying to accomplish by calling your function? What will make the callsite look clearest to someone reading the code a year later who doesn't understand it?

All errors that are thrown or passed to callbacks should all be instances of Error. They should not be instances of Meteor.Error unless they are errors meant to go over the wire via DDP. That is, only use a Meteor.Error to specifically tell the client something, not to indicate a programming error or an unexpected condition in common code.

Either: all returns in a function should either explicitly return a value (perhaps undefined), and function should end with an explicit return or throw; or there should be no returns in the function which return a value. (js2-mode.el will complain if you don't, and often its complaints indicate an actual bug where you meant to return false or something.)

var BAD1 = function () {
  if (x)
    return 5;
};
var BAD2 = function () {
  if (x)
    return;
  return 5;
};
var GOOD1 = function () {
  if (x)
    return 5;
  throw Error("Nope");
};
var GOOD2 = function () {
  if (x)
    return 5;
  return undefined;
};