Engineering Blog

This is the second in a series of articles that describe the incremental steps we took to address issues with our JavaScript, causing last minute scrambling in our development and release processes.  When last we left, we had decided to use Dominic Mitchell’s jslint4java wrapper around Douglas Crawford’s JSLint.  I wrote a Maven profile that allowed developers to run JSLint on our JavaScript so we could find the problems early in our development process. We found about 500 complaints and needed work through them before we could let JSLint fail our build.

Since we have multiple scrums working in this code base, we have a many different coding styles. For instance, JSLint has an option called forin which tells JSLint to look for for loop iterations that look like this:

for(var i in someObject) {
    //do something interesting();
}

Some will define someObject as:

someObject = {};

and use it as an associative array.  Others will use a prototyped collection object of some sort.  JSLint wants you to be intentional.  If you use a prototyped object, then you really should only iterate over the properties with something like:

for (var i in someObject) {
    if(someObject.hasOwnProperty(i) {
        // now do something interesting;
    }
}

Another big area dealt with just relatively benign things like mixing tabs and spaces on lines.  What we found was the some folks use Eclipse and some use IntelliJ.   We were able to configure Eclipse to match what IntelliJ does with white space for JavaScript, and this cleared up 100 or so complaints.

Another big set of issues involved if statements that were simply checking for the existence of something versus evaluating whether the value was null or undefined. They were written like:

if(something != null) {
    // do something
}

JSLint wants you to write this as:

if(something !== null) {
    // do something
}

or

if(!something) {
    // do something
}

It turns out that sometimes we really do want to check for exactly null or exactly undefined (true and false as well). But at other times we are really just checking for the existence of something and you do not care if the value is null or undefined, for this specific purpose.

The last major grouping of complaints centered around in line if statements like:

if(something) blah();

JSLint does not like this for readability reasons.  It can be hard to figure out the intent of something like:

if (something)
    blah();
    blah2();

Were blah() and blah2() meant to be in the same if block, or were they really suppose to be different?  It is hard to tell.  In any case JSLint does not like it.

JSLint also found about five actual errors that would affect the operation of the javascript in some browsers.

Once everything was cleaned up, it was time to include JSLint as part of our build process. This is described in the next article.