So you're implementing a JEP.

Most of this applies to the high-level APIs only, the ones covered by JEPs. Low-level modules, which have mostly been written by Atul, follow different conventions.

Landing your Implementation

The process from JEP to landing has worked something like this so far:

• JEP first draft on wikimo.
• Ask Myk for review.
• He'll start a conversation in the user group so everybody can offer feedback.
• Iterate on the JEP, updating the wiki, based on the conversation.
• Final JEP draft agreed upon.
• Ask Myk to review your patch. He'll check that it more or less implements the JEP.
• Ask Atul to review your patch. He'll check that it uses the Jetpack platform OK.
• Ask a domain expert to review your patch.

Modules

The module you're writing is a CommonJS module and is similar to a Mozilla JavaScript module (.jsm).

All the properties you attach to the exports object are visible to the outside world. Everything else is private.

Loading

Instances of your module are created by the Cuddlefish loader, which is part of the Jetpack platform.

A new instance is created the first time an extension require()s it, but thereafter that instance is generally cached for that extension. However, the loader is capable of creating multiple concurrent instances of your module should the need arise, so you shouldn't write your module such that it depends on there being only one instance of it at a time. For each instance, the loader creates a Cu.Sandbox and evaluates your module's code inside it.

Unloading

Your module can be unloaded at any time, so it should be prepared, and it should clean up after itself. (Restartless extensions, yeah?) When exactly your module is unloaded is up to the Cuddlefish loader.

Cleaning up on unload is easy. The unload module provides two helper functions: when() and ensure(). when() takes a callback, and it's called when your module is unloaded. The callback should perform any necessary cleanup. ensure() takes an object that defines an unload() method. When you call require("unload").ensure(obj), obj.unload() is called once and only once when your module is unloaded.

Note that all (proper) modules already clean up after themselves, and if the only cleanup your module needs to do is related to the resources of other modules, you might not even need to do anything at all.

If however your module creates its own resources, it should clean them up when it's unloaded. Your unit test should ensure that it does. See the XHR test for an example of manually loading and unloading a module.

(Technically speaking when() and obj.unload() are called when the unload module is unloaded, not when your module is unloaded. But since your module must first obtain an instance of the unload module to use when() and ensure(), your module's unload causes the unload instance to be unloaded.)

Private Parts

You make "private" members in Mozilla JavaScript code by prefixing their names with an underscore (e.g., this._dontTouchMe), but that's no good for Jetpack, where we're trying to enforce, you know, a security model.

Whenever you define a property on an object, stop and think if that object is exported -- if it's public. Yes? Is that property "private"? Yes? Then it's not really private at all.

Also, clients can apply your public methods to any arbitrary object by using call() and apply(). You therefore can't be sure that this really refers to your object. It might be an object that does something untoward when applied to the code you've written for yours.

For these reasons, define all public methods on this inside your class's constructor, and instead of using this inside those methods, use variables in the scope of the constructor.

// BAD
function MyExportedObject(someProperty) {
  this._someProperty = someProperty;
};
MyExportedObject.prototype = {
  publicMethod: function () {
    return this.alsoPublic();
  },
  alsoPublic: function () {
    return this._privateHelper();
  },
  _privateHelper: function () {
    return this._someProperty;
  }
};

// GOOD
function MyExportedObject(someProperty) {
  const self = this;

  this.publicMethod = function () {
    return self.alsoPublic();
  };

  this.alsoPublic = function () {
    return privateHelper();
  };

  function privateHelper() {
    return someProperty;
  }
};

Client Callbacks

try-catch-log

Always try-catch callbacks passed into your API and log the error with console.exception(). Two reasons:

1. You don't want user exceptions interrupting control of your own code.
2. Jetpack automatically logs all errors originating in the main event loop, but it misses others, like those in event listeners.

For the second reason, you should also try-catch-log your own code that's not called on the main event loop.

There is an errors module that currently provides some related helpers and could stand to be expanded. catchAndLog() returns a function that wraps a callback in a try-catch-log.

// BAD
someObj.callback();

// GOOD
try {
  someObj.callback();
}
catch (err) {
  console.exception(err);
}

this

Think about what object this should refer to inside your client's callback. (It might be obvious, but if not, it should be specified in the JEP.) In particular, be careful about calling a callback that isn't attached to an object. If you call it as you normally would a function, this is the current global context. What's the global context inside your module code? It's not your module, by which I mean the exports object. It's the Cu.Sandbox that Cuddlefish used to evaluate your code! Not something that your clients should be able to access.

// BAD
callback();

// BETTER
callback.call(exports);

// BEST
someRelevantObject.callback();
callback.call(someRelevantObject);

Exceptions

Never throw raw strings, because console.exception(), which is ultimately used to log any exceptions in Jetpack, can't introspect a string to get at any additional information about where the error came from. Instead, throw a new Error(), which automatically adds metadata about the state of the stack at the time the error was thrown. This allows us to provide helpful tracebacks to developers.

Being friendly to developers is an important part of Jetpack. Therefore, all error messages that developers might see should be informative and punctuated full sentences.

XPCOM exceptions generally aren't friendly, so they should be caught and friendly errors thrown in their place -- within reason of course. You don't need to catch every possible XPCOM exception that might be thrown by a line.

// BAD
let stream = Cc['@mozilla.org/network/file-output-stream;1'].
             createInstance(Ci.nsIFileOutputStream);
stream.init(file, openFlags, permFlags, 0);

// GOOD
let stream = Cc['@mozilla.org/network/file-output-stream;1'].
             createInstance(Ci.nsIFileOutputStream);
try {
  stream.init(file, openFlags, permFlags, 0);
}
catch (err if err.result === Cr.NS_ERROR_FILE_NOT_FOUND) {
  throw new Error("The file " + fileName + " does not exist.");
}

Testing

Your module should come with unit tests.

If you use the unload module to clean up resources you've created, you should test that those resources are actually cleaned up when your module is unloaded. See the XHR test for an example of manually loading and unloading a module.

Modularity

Might parts of your code be useful to others? Break them out into their own bugs so others can benefit.

See Also

• Atul wrote an older best practices doc, but it's still totally relevant.
• Myk also wrote down some recommendations that you should follow.
• Jetpack MXR: http://mxr.mozilla.org/labs-central/source/jetpack-sdk/
• jetpack-core modules
• Run cfx docs to see documented modules.
• Need to learn how to use an undocumented module? Study how its unit test uses it.