Coding Conventions

Design Space uses some specific coding conventions. All of the pull requests that come in should adhere to the following rules:

Basics

• Use 4 space indents (spaces, not tabs)

• All JavaScript files have a single module definition using the following AMD-style format:

define(function (require, exports, module) {

    "use strict";

    // Load dependent modules
    var Module = require("Module");

    var someFunction = function () {
        // ...
    };

    // Export public APIs
    exports.someFunction = someFunction;
});

• As indicated by the "use strict"; declaration in the example above, all modules should be declared to use strict mode.

• Must pass JSHint. These are the default settings for JSHint.

Globals

Do not use globals except when absolutely necessary. An example of a global that is absolutely necessary is the define variable defined in the global namespace by RequireJS to load AMD modules. All third-party libraries should be expressed as AMD modules. They should be loaded in module-local scopes. For example, do not load jQuery into the global namespace; instead, load it as an AMD module with require: var $ = require("jquery").

Browser globals should be referenced via the window object, e.g. window.setTimeout() instead of setTimeout().

Function Declaration

There are two subtly different ways to define functions in JavaScript:

1. With a function declaration: function foo(arg) { /* function body */ }
2. With a variable declaration initialized to an anonymous function expression: var foo = function (arg) { /* function body */}

Do not use the former, function-declaration style of defining functions. Only use the latter, variable-declaration style. The semantics of the former is unnecessarily confusing in many cases.

Self versus bind

Until ES6 arrow functions become available to us, we need to work around the problem of this not being lexically scoped in JavaScript. There are two common patterns for working around this. The first is to use a self variable in a closure to refer to a parent function's receiver:

var outer = function () {
  var self = this;

  var inner = function () {
    self.foo();
  };
};

The second is to explicitly bind the parent function's receiver to inner functions:

var outer = function () {
  var inner = function () {
    this.foo();
  }.bind(this);
};

There are pros and cons to both approaches. The good part about the self variable is that it can be more concise than explicit binding when there are many functions that should share the same receiver; it's more compatible with older browser; and it is also slightly faster. Nonetheless, we prefer to explicitly bind receivers because explicitly binding receivers yields more maintainable code. If we fail to correctly bind the receiver to a function then it ensures that ALL code paths through the inner function are broken instead of just some paths. On the other hand, when using the self pattern, it's possible that later we'll forget to use self instead of this and not notice it for some time. When we explicitly bind the receiver then it's impossible to create subtly broken behavior; either everything will be bound correctly, which is good, or everything will be bound incorrectly, which is bad but the bug should manifest obviously and quickly.

Naming and Syntax

Variable and function names use camelCase (not under_scores):

Do this:

var editorHolder; 
var getText() = function () {};

Not this:

var editor_holder; // Don't use underscore!
var get_text = function () {}; // Don't use underscore!

Use $ prefixes ONLY on variables referring to jQuery objects: > Do this: > > ``` > var $sidebar = $("#sidebar"); > ``` > > Not this: > > ``` > var sidebar = $("#sidebar"); // Use '$' to prefix variables referring to jQuery objects > ```
Use _ prefixes on private variables/methods: > Do this: > > ``` > var _privateVar = 42; > var _privateFunction = function () {}; > ``` > > Not this: > > ``` > var privateVar = 42; // Private vars should start with _ > var privateFunction = {} (); // Private functions should start with _ > ```
For word separation in classes and id's in HTML/CSS use all lower-case with dashes (-), not camelCase or under_scores. DO, use *double underscores* to separate classes into either a `block__element__modifier` format, or more simply `namespace__name`: > Do this: > > ``` >

> > ``` > > Not this: > > ``` >

// Don't use camel-case for ids > // Don't use underscore for normal separation > ```
Use semicolons: > Do this: > > ``` > var someVar; > someVar = 2 + 2; > ``` > > Not this: > > ``` > var someVar // Missing semicolon! > someVar = 2 + 2 // Missing semicolon! > ```
Use double quotes in JavaScript. If a JavaScript string literal _contains_ code within it, use single quotes within the string to avoid escaping. > Do this: > > ``` > var aString = "Hello"; > someFunction("This is awesome!"); > > var htmlCode = "

"; > ``` > > Not this: > > ``` > var aString = 'Hello'; // Use double quotes! > someFunction('This is awesome!'); // Use double quotes! > > var htmlCode = '

'; // Use double quotes! > var htmlCode = "

"; // Within string, use single quotes! > ```

Code Structure

Prototypical inheritance pattern:

function MyClass() { // constructor
    Parent.call(this); // if this is a subclass
    // ...
}

// the following are only necessary if this is a subclass
MyClass.super_ = Parent;
MyClass.prototype = Object.create(Parent.prototype);
MyClass.prototype.constructor = MyClass;

// or, even better:
var inherits = require("adapter/util").inherits;
inherits(MyClass, Parent);

/**
 * Declare instance variables so we can JSDoc them.
 * @type {string}
 */
MyClass.prototype.myVar = null;

/**
 * @override
 * For override methods, use this pattern for super calls.
 */
MyClass.prototype.myMethod = function (arg1, arg2) {
     // - don't use this.parentClass -- won't work if hierarchy is more than one deep
     // - use apply(this, arguments) instead of call(this, arg1, arg2) so we don't have to fix it up
     //   every time we add a new argument
     MyClass.prototype.parentClass.myMethod.apply(this, arguments);
     // ... 
};

APIs to Use or Avoid

On Arrays, use Array.prototype.forEach or _.each rather than for loops:

Do this:

var anArray = [1, 2, 3, 4];
anArray.forEach(function (value, index) {  // OK!
    // ...
});

Or this:

var _ = require("lodash");
_.each(anArray, function (value, index) {  // OK
    // ...
})

Not this:

for (var i = 0; i < anArray.length; i++) {  // Use Array.prototype.forEach()
    // ...
}

To iterate object keys, use `_.forEach()` rather than `for-in` loops. (This avoids various pitfalls when the keys could have arbitrary values, and matches `Array.prototype.forEach()`'s callback API better). > Do this: > ``` > var anObject = { foo: "a", bar: "b", baz: "c" }; > _.forEach(anObject, function (value, key) { > // ... > }); > ``` > > Not this: > ``` > for (var key in anObject) { // Use _.forEach() > if (anObject.hasOwnProperty(key)) { /* ... */ } > } > ```

When in doubt, use lodash!

Comments

• All comments should be C++ single line style //comment.
• Even multiline comments should use // at the start of each line
• Use C style /* comments */ for notices at the top and bottom of the file
• Use JSDoc tags for annotations with Google's Closure Compiler Type Expressions, see https://developers.google.com/closure/compiler/docs/js-for-compiler
• Annotations should use the /** annotation */
• Annotate all non-anonymous functions

Exceptions/Asserts/Logging

• Use exceptions when encountering an unexpected error and recovery is not possible. In other words, the error means that Playground will be noticeably broken or data could be corrupted if execution continues.
• Use console.error() or console.assert() when encountering an unexpected error that can be recovered from. In other words, an error occurred and the current operation may not complete successfully, but the overal integrity of the program is not compromised.
• Use console.log() to note unexpected behavior that is not necessarily an error.

NOTE console.log() should not be used to log general information.

Example 1 - throw

The Document constructor throws an error if a document already exists for the current file. Creating a new document that points to the same file could lead to file corruption if both documents are edited.

function Document(file, initialTimestamp, rawText) {
    ...
    if (_openDocuments[file.fullPath]) {
        throw new Error("Creating a document when one already exists, for: " + file);
    }
    ...
}

Example 2 - console.assert()

In DocumentManager.closeFullEditor(), after opening the next file, we check that the currentDocument property is updated correctly. This is an error if not true, and may lead to unexpected behavior, but it is not catastrophic.

CommandManager.execute(Commands.FILE_OPEN, { fullPath: nextFile.fullPath })
    .done(function () {
        // (Now we're guaranteed that the current document is not the one we're closing)
        console.assert(!(_currentDocument && _currentDocument.file.fullPath === file.fullPath));
    })

Example 3 - console.log()

The CommandManager.register() function uses console.log() to note an unexpected (but not dangerous) condition -- the specified command id has already been registered. The code can continue safely from here.

function register(name, id, commandFn) {
    if (_commands[id]) {
        console.log("Attempting to register an already-registered command: " + id);
        return null;
    }
    ...
}

Asynchronous APIs

We prefer both using and presenting asynchronous instead of synchronous APIs. We present asynchronous APIs by exposing functions that return promises instead of callbacks. Clients can rely on the promises that we return from our API calls to satisfy the Promises A/+ specification as well as the ES6 Promise specification. Internally we use the Bluebird promise library, but external clients should not rely on features specific to that implementation.

Two notes for contributors who are primarily used to working with jQuery promises:

1. jQuery promises are not guaranteed to be asynchronous, whereas Bluebird (and other A/+ conforming promise implementations) are. This means that getPromise().then(fn) will never execute fn in the same tick of the event loop as getPromise.
2. The use of deferred objects is to be avoided when possible. When possible, use the new Promise(function(resolve, reject) { /* ... */ }) pattern instead.