Do not add methods to your view that return pieces of things to render. Build those as stateless components instead.

Add descriptions to all propTypes using a single line comment above the definition. No need to document the types, but add some context for each property so that other developers understand the intended use.

// Bad
{
propTypes: {
currency:React.PropTypes.string.isRequired,
amount:React.PropTypes.number.isRequired,
isIgnored:React.PropTypes.bool.isRequired
}
}
// Bad
{
propTypes: {
/** * The currency that the reward is in*/
currency:React.PropTypes.string.isRequired,
/** * The amount of reward*/
amount:React.PropTypes.number.isRequired,
/** * If the reward has been ignored or not*/
isIgnored:React.PropTypes.bool.isRequired
}
}
// Good
{
propTypes: {
// The currency that the reward is in
currency:React.PropTypes.string.isRequired,
// The amount of reward
amount:React.PropTypes.number.isRequired,
// If the reward has been ignored or not
isIgnored:React.PropTypes.bool.isRequired
}
}

Inline Ternarys

Use inline ternary statements when rendering optional pieces of templates. Notice the white space and formatting of the ternary.

JavaScript bits of no-ledge

Module Pattern

When you hear "module pattern" in the context of JavaScript think singleton pattern. The way singletons or modules are usually implemented in JS is by using a plain JS object. Typically

We set variables and functions as properties of the object so you won't see prototype anywhere in a module.
Variables are prefixed with _ since they're usually "private" to the module and if they're made available to the outside world it's usually through getters and setters
The PubSub library is a perfect example of this pattern in use.

Immediately Invoked Function Expression (IIFE)

The name is confusing and the syntax may feel strange if you've never seen it before but this is a very simple concept and what it allows you to do is declare a function and immediately call it, as the name suggests.

We only know of one good reason to use IIFEs and that is to take advantage of closures.

What the code above does is define a function that returns a closure and immediately invoke the function. You can think of a closure like a box with two things in it: a function and a context where the context is just a collection of variables that are available to the function. In the case above the IIFE defines (and invokes) a function that returns a closure: the inner function + the variable id.

Closures are used in JS to emulate private variables. In JS there's no such thing as private variables (surprise!) but by leveraging closures you can emulate private variables and this is illustrated in the setTimeout example above where the variable id is inaccessible from outside of that function.

Note that we do not recommend using IIFEs to emulate private variables in Expensify's code bases because the convention of prefixing private variables with an underscore serves the same purpose and allows you to access the variables from outside the function for debugging purposes: think about accessing these from the Safari / Chrome console while debugging our iOS or web app.

For the specific case of setTimeout, creating a module seemed like overkill for this and we need setTimeout to return a number but we don't think we're ever gonna need to inspect the value of id.

Also note that IIFEs make no sense in a modular JS environment since you export only what you want to expose and everything else remains private to the module.

Last, we'd advise against using it for dependency injection unless you're trying not to pollute scope since you can accomplish the same thing with clearer syntax. Consider the following example

Publisher / Subscriber

This one isn't specific to JS but we think it's worth mentioning because

We use it heavily on web and mobile.

We should be using it more on mobile.

We want new hires to understand it.

The publisher / subscriber pattern helps decouple objects from one another by changing the paradigm a bit from having objects directly call one another (thus coupling one to the public API of the other) to having objects publish (send / emit) events or messages while others subscribe (react) to them and using an event bus to send and receive these messages, which in our case is the PubSub library.

This is very useful when you have and event or action that is of interest to different parts of the application that are logically split and in different areas of the code. An example of this would be an event that causes different parts of the UI owned by different parts of the code to be updated.

On mobile for instance we publish the event Event.REPORT_LIST_FETCHED when we load reports from the servers to which both the expenses page and the reports page subscribe to and redraw the cells in the expenses list (we show the report name on the expenses list for reported expenses) and reports lists.

Note that if an object is publishing an event to which only that object is subcribed to, it's better to remove the event entirely and just call the handler directly. Said another way, there's no benefit to having an object adopt the publisher/subscriber pattern to communicate only with itself.

Functions as first class citizens

Probably one of the most important concepts (and features!) to understand of JS is that functions are first-class citizens, which is a fancy way of saying that functions can be assigned to variables, passed as function parameters and returned from functions.

Something we see often is people creating unnecessary anonymous functions and passing them as parameters. Here's an example of what we mean

While the two solutions are semantically equivalent (given that the wrapping anonymous functions have the same contract than the ones that aren't wrapped)
they are logically different as one introduces a new anonymous function that in this particular case is unnecessary. This is unlikely to improve performance or affect the code in any way but we think it's important to understand the difference between the two implementations.

This isn't limited to functions that take no parameters, for example, imagine API.GetReportSummary returns a promise that resolves with a report name and status as two separate strings, that means that the function passed to the done callback takes two parameters (name, status), so

There are times however where a function that is passed as a parameter to another function depends on its context (ie, what this is bound to), and in that case it's important that this is set to the right thing. In the following example, the last two implementations are (semantically) equivalent since .bind returns a function and in the second example we're just creating an anonymous function on the fly.

Note that using Function's bind to set the scope of this is extremely non-performant since it iterates through every single property of the object being bound to, which takes a lot of processing especially with large objects. Thus, using self has a very good performance benefit and thus is our preferred convention. A side tangent here is that with ES6, you can use arrow functions which removes the need for either of these, since arrow functions have a scope dead-zone with no this (using this will always reference this in the parent scope). With web, you can use any ES6 code if you give the file a .jsx extension in the /site directory.