javascript
comment 0

Buggy JavaScript Code: The 10 Most Common Mistakes JavaScript Developers Make

Today, JavaScript is at the core of virtually all modern web applications. The past several years in particular have witnessed the proliferation of a wide array of powerful JavaScript-based libraries and frameworks for single page application (SPA) development, graphics and animation, and even server-side JavaScript platforms. JavaScript has truly become ubiquitous in the world of web app development and is therefore an increasingly important skill to master.

At first blush, JavaScript may seem quite simple. And indeed, to build basic JavaScript functionality into a web page is a fairly straightforward task for any experienced software developer, even if they’re new to JavaScript. Yet the language is significantly more nuanced, powerful, and complex than one would initially be lead to believe. Indeed, many of JavaScript’s subtleties lead to a number of common problems that keep it from working – 10 of which we discuss here – that are important to be aware of and avoid in one’s quest to become a master JavaScript developer.

This developer's JavaScript is full of problems and mistakes.

Common Mistake #1: Incorrect references to this

I once heard a comedian say:

“I’m not really here, because what’s here, besides there, without the ‘t’?”

That joke in many ways characterizes the type of confusion that often exists for developers regarding JavaScript’s this keyword. I mean, is this really this, or is it something else entirely? Or is it undefined?

As JavaScript coding techniques and design patterns have become increasingly sophisticated over the years, there’s been a corresponding increase in the proliferation of self-referencing scopes within callbacks and closures, which are a fairly common source of “this/that confusion”.

Consider this example code snippet:

Game.prototype.restart = function () {
  this.clearLocalStorage();
  this.timer = setTimeout(function() {
    this.clearBoard();    // what is "this"?
  }, 0);
};

Executing the above code results in the following error:

Uncaught TypeError: undefined is not a function

Why?

It’s all about context. The reason you get the above error is because, when you invoke setTimeout(), you are actually invoking window.setTimeout(). As a result, the anonymous function being passed to setTimeout() is being defined in the context of the window object, which has no clearBoard() method.

A traditional, old-browser-compliant solution is to simply save your reference to this in a variable that can then be inherited by the closure; e.g.:

Game.prototype.restart = function () {
  this.clearLocalStorage();
  var self = this;   // save reference to 'this', while it's still this!
  this.timer = setTimeout(function(){
    self.clearBoard();    // oh OK, I do know who 'self' is!
  }, 0);
};

Alternatively, in newer browsers, you can use the bind() method to pass in the proper reference:

Game.prototype.restart = function () {
  this.clearLocalStorage();
  this.timer = setTimeout(this.reset.bind(this), 0);  // bind to 'this'
};

Game.prototype.reset = function(){
    this.clearBoard();    // ahhh, back in the context of the right 'this'!
};

Common Mistake #2: Thinking there is block-level scope

As discussed in our JavaScript Hiring Guide, a common source of confusion among JavaScript developers (and therefore a common source of bugs) is assuming that JavaScript creates a new scope for each code block. Although this is true in many other languages, it is not true in JavaScript. Consider, for example, the following code:

for (var i = 0; i < 10; i++) {
  /* ... */
}
console.log(i);  // what will this output?

If you guess that the console.log() call would either output undefined or throw an error, you guessed incorrectly. Believe it or not, it will output 10. Why?

In most other languages, the code above would lead to an error because the “life” (i.e., scope) of the variable i would be restricted to the for block. In JavaScript, though, this is not the case and the variable i remains in scope even after the for loop has completed, retaining its last value after exiting the loop. (This behavior is known, incidentally, as variable hoisting).

It is worth noting, though, that support for block-level scopes is making its way into JavaScript through the new let keyword. The let keyword is already available in JavaScript 1.7 and is slated to become an officially supported JavaScript keyword as of ECMAScript 6.

Common Mistake #3: Creating memory leaks

Memory leaks are almost inevitable JavaScript problems if you’re not consciously coding to avoid them. There are numerous ways for them to occur, so we’ll just highlight a couple of their more common occurrences.

Memory Leak Example 1: Dangling references to defunct objects

Consider the following code:

var theThing = null;
var replaceThing = function () {
  var priorThing = theThing;  // hold on to the prior thing
  var unused = function () {
    // 'unused' is the only place where 'priorThing' is referenced,
    // but 'unused' never gets invoked
    if (priorThing) {
      console.log("hi");
    }
  };
  theThing = {
    longStr: new Array(1000000).join('*'),  // create a 1MB object
    someMethod: function () {
      console.log(someMessage);
    }
  };
};
setInterval(replaceThing, 1000);    // invoke `replaceThing' once every second

If you run the above code and monitor memory usage, you’ll find that you’ve got a massive memory leak, leaking a full megabyte per second! And even a manual GC doesn’t help. So it looks like we are leaking longStr every time replaceThing is called. But why?

Let’s examine things in more detail:

Each theThing object contains its own 1MB longStr object. Every second, when we call replaceThing, it holds on to a reference to the prior theThing object in priorThing. But we still wouldn’t think this would be a problem, since each time through, the previously referenced priorThing would be dereferenced (when priorThing is reset via priorThing = theThing;). And moreover, is only referenced in the main body of replaceThing and in the function unused which is, in fact, never used.

So again we’re left wondering why there is a memory leak here!?

To understand what’s going on, we need to better understand how things are working in JavaScript under the hood. The typical way that closures are implemented is that every function object has a link to a dictionary-style object representing its lexical scope. If both functions defined inside replaceThing actually used priorThing, it would be important that they both get the same object, even if priorThing gets assigned to over and over, so both functions share the same lexical environment. But as soon as a variable is used by any closure, it ends up in the lexical environment shared by all closures in that scope. And that little nuance is what leads to this gnarly memory leak. (More detail on this is available here.)

Memory Leak Example 2: Circular references

Consider this code fragment:

function addClickHandler(element) {
    element.click = function onClick(e) {
        alert("Clicked the " + element.nodeName)
    }
}

Here, onClick has a closure which keeps a reference to element (via element.nodeName). By also assigning onClick to element.click, the circular reference is created; i.e.: element -> onClick -> element -> onClick -> element

Interestingly, even if element is removed from the DOM, the circular self-reference above would prevent element and onClick from being collected, and hence, a memory leak.

Avoiding Memory Leaks: What you need to know

JavaScript’s memory management (and, in paticular, garbage collection) is largely based on the notion of object reachability.

The following objects are assumed to be reachable and are known as “roots”:

  • Objects referenced from anywhere in the current call stack (that is, all local variables and parameters in the functions currently being invoked, and all the variables in the closure scope)
  • All global variables

Objects are kept in memory at least as long as they are accessible from any of the roots through a reference, or a chain of references.

There is a Garbage Collector (GC) in the browser which cleans memory occupied by unreachable objects; i.e., objects will be removed from memory if and only if the GC believes that they are unreachable. Unfortunately, it’s fairly easy to end up with defunct “zombie” objects that are in fact no longer in use but that the GC still thinks are “reachable”.

Common Mistake #4: Confusion about equality

One of the conveniences in JavaScript is that it will automatically coerce any value being referenced in a boolean context to a boolean value. But there are cases where this can be as confusing as it is convenient. Some of the following, for example, have been known to bite many a JavaScript developer:

// All of these evaluate to 'true'!
console.log(false == '0');
console.log(null == undefined);
console.log(" \t\r\n" == 0);
console.log('' == 0);

// And these do too!
if ({}) // ...
if ([]) // ...

With regard to the last two, despite being empty (which might lead one to believe that they would evaluate to false), both {} and [] are in fact objects and any object will be coerced to a boolean value of true in JavaScript, consistent with the ECMA-262 specification.

As these examples demonstrate, the rules of type coercion can sometimes be clear as mud. Accordingly, unless type coercion is explicitly desired, it’s typically best to use === and !== (rather than == and !=), so as to avoid any unintended side-effects of type coercion. (== and != automatically perform type conversion when comparing two things, whereas === and !== do the same comparison without type conversion.)

And completely as a sidepoint – but since we’re talking about type coercion and comparisons – it’s worth mentioning that comparing NaN with anything (even NaN!) will always return false. You therefore cannot use the equality operators (==, ===, !=, !==) to determine whether a value is NaN or not. Instead, use the built-in global isNaN() function:

console.log(NaN == NaN);    // false
console.log(NaN === NaN);   // false
console.log(isNaN(NaN));    // true

Common Mistake #5: Inefficient DOM manipulation

JavaScript makes it relatively easy to manipulate the DOM (i.e., add, modify, and remove elements), but does nothing to promote doing so efficiently.

A common example is code that adds a series of DOM Elements one at a time. Adding a DOM element is an expensive operation. Code that adds multiple DOM elements consecutively is inefficient and likely not to work well.

One effective alternative when multiple DOM elements need to be added is to use document fragments instead, thereby improving both efficiency and performance.

For example:

var div = document.getElementsByTagName("my_div");

var fragment = document.createDocumentFragment();

for (var e = 0; e < elems.length; e++) {  // elems previously set to list of elements
    fragment.appendChild(elems[e]);
}
div.appendChild(fragment.cloneNode(true));

In addition to the inherently improved efficiency of this approach, creating attached DOM elements is expensive, whereas creating and modifying them while detached and then attaching them yields much better performance.

Common Mistake #6: Incorrect use of function definitions inside for loops

Consider this code:

var elements = document.getElementsByTagName('input');
var n = elements.length;    // assume we have 10 elements for this example
for (var i = 0; i < n; i++) {
    elements[i].onclick = function() {
        console.log("This is element #" + i);
    };
}

Based on the above code, if there were 10 input elements, clicking any of them would display “This is element #10”! This is because, by the time onclick is invoked for any of the elements, the above for loop will have completed and the value of i will already be 10 (for all of them).

Here’s how we can correct the above code problems, though, to achieve the desired behavior:

var elements = document.getElementsByTagName('input');
var n = elements.length;    // assume we have 10 elements for this example
var makeHandler = function(num) {  // outer function
     return function() {   // inner function
         console.log("This is element #" + num);
     };
};
for (var i = 0; i < n; i++) {
    elements[i].onclick = makeHandler(i+1);
}

In this revised version of the code, makeHandler is immediately executed each time we pass through the loop, each time receiving the then-current value of i+1 and binding it to a scoped num variable. The outer function returns the inner function (which also uses this scoped num variable) and the element’s onclick is set to that inner function. This ensures that each onclick receives and uses the proper i value (via the scoped num variable).

Common Mistake #7: Failure to properly leverage prototypal inheritance

A surprisingly high percentage of JavaScript developers fail to fully understand, and therefore to fully leverage, the features of prototypal inheritance.

Here’s a simple example. Consider this code:

BaseObject = function(name) {
    if(typeof name !== "undefined") {
        this.name = name;
    } else {
        this.name = 'default'
    }
};

Seems fairly straightforward. If you provide a name, use it, otherwise set the name to ‘default’; e.g.:

var firstObj = new BaseObject();
var secondObj = new BaseObject('unique');

console.log(firstObj.name);  // -> Results in 'default'
console.log(secondObj.name); // -> Results in 'unique'

But what if we were to do this:

delete secondObj.name;

We’d then get:

console.log(secondObj.name); // -> Results in 'undefined'

But wouldn’t it be nicer for this to revert to ‘default’? This can easily be done, if we modify the original code to leverage prototypal inheritance, as follows:

BaseObject = function (name) {
    if(typeof name !== "undefined") {
        this.name = name;
    }
};

BaseObject.prototype.name = 'default';

With this version, BaseObject inherits the name property from its prototype object, where it is set (by default) to 'default'. Thus, if the constructor is called without a name, the name will default to default. And similarly, if the name property is removed from an instance of BaseObject, the prototype chain will then be searched and the name property will be retrieved from the prototype object where its value is still 'default'. So now we get:

var thirdObj = new BaseObject('unique');
console.log(thirdObj.name);  // -> Results in 'unique'

delete thirdObj.name;
console.log(thirdObj.name);  // -> Results in 'default'

Common Mistake #8: Creating incorrect references to instance methods

Let’s define a simple object, and create and instance of it, as follows:

var MyObject = function() {}

MyObject.prototype.whoAmI = function() {
    console.log(this === window ? "window" : "MyObj");
};

var obj = new MyObject();

Now, for convenience, let’s create a reference to the whoAmI method, presumably so we can access it merely by whoAmI() rather than the longer obj.whoAmI():

var whoAmI = obj.whoAmI;

And just to be sure everything looks copacetic, let’s print out the value of our new whoAmI variable:

console.log(whoAmI);

Outputs:

function () {
    console.log(this === window ? "window" : "MyObj");
}

OK, cool. Looks fine.

But now, look at the difference when we invoke obj.whoAmI() vs. our convenience reference whoAmI():

obj.whoAmI();  // outputs "MyObj" (as expected)
whoAmI();      // outputs "window" (uh-oh!)

What went wrong?

The headfake here is that, when we did the assignment var whoAmI = obj.whoAmI;, the new variable whoAmI was being defined in the global namespace. As a result, its value of this is window, not the obj instance of MyObject!

Thus, if we really need to create a reference to an existing method of an object, we need to be sure to do it within that object’s namespace, to preserve the value of this. One way of doing this would be, for example, as follows:

var MyObject = function() {}

MyObject.prototype.whoAmI = function() {
    console.log(this === window ? "window" : "MyObj");
};

var obj = new MyObject();
obj.w = obj.whoAmI;   // still in the obj namespace

obj.whoAmI();  // outputs "MyObj" (as expected)
obj.w();       // outputs "MyObj" (as expected)

Common Mistake #9: Providing a string as the first argument to setTimeout or setInterval

For starters, let’s be clear on something here: Providing a string as the first argument to setTimeout or setInterval is not itself a mistake per se. It is perfectly legitimate JavaScript code. The issue here is more one of performance and efficiency. What is rarely explained is that, under the hood, if you pass in a string as the first argument to setTimeout or setInterval, it will be passed to the function constructor to be converted into a new function. This process can be slow and inefficient, and is rarely necessary.

The alternative to passing a string as the first argument to these methods is to instead pass in a function. Let’s take a look at an example.

Here, then, would be a fairly typical use of setInterval and setTimeout, passing a string as the first parameter:

setInterval("logTime()", 1000);
setTimeout("logMessage('" + msgValue + "')", 1000);

The better choice would be to pass in a function as the initial argument; e.g.:

setInterval(logTime, 1000);   // passing the logTime function to setInterval

setTimeout(function() {       // passing an anonymous function to setTimeout
    logMessage(msgValue);     // (msgValue is still accessible in this scope)
  }, 1000);

Common Mistake #10: Failure to use “strict mode”

As explained in our JavaScript Hiring Guide, “strict mode” (i.e., including 'use strict'; at the beginning of your JavaScript source files) is a way to voluntarily enforce stricter parsing and error handling on your JavaScript code at runtime, as well as making it more secure.

While, admittedly, failing to use strict mode is not a “mistake” per se, its use is increasingly being encouraged and its omission is increasingly becoming considered bad form.

Here are some key benefits of strict mode:

  • Makes debugging easier. Code errors that would otherwise have been ignored or would have failed silently will now generate errors or throw exceptions, alerting you sooner to problems in your code and directing you more quickly to their source.
  • Prevents accidental globals. Without strict mode, assigning a value to an undeclared variable automatically creates a global variable with that name. This is one of the most common errors in JavaScript. In strict mode, attempting to do so throws an error.
  • Eliminates this coercion. Without strict mode, a reference to a this value of null or undefined is automatically coerced to the global. This can cause many headfakes and pull-out-your-hair kind of bugs. In strict mode, referencing a a this value of null or undefined throws an error.
  • Disallows duplicate property names or parameter values. Strict mode throws an error when it detects a duplicate named property in an object (e.g., var object = {foo: "bar", foo: "baz"};) or a duplicate named argument for a function (e.g., function foo(val1, val2, val1){}), thereby catching what is almost certainly a bug in your code that you might otherwise have wasted lots of time tracking down.
  • Makes eval() safer. There are some differences in the way eval() behaves in strict mode and in non-strict mode. Most significantly, in strict mode, variables and functions declared inside of an eval() statement are not created in the containing scope (they are created in the containing scope in non-strict mode, which can also be a common source of problems).
  • Throws error on invalid usage of delete. The delete operator (used to remove properties from objects) cannot be used on non-configurable properties of the object. Non-strict code will fail silently when an attempt is made to delete a non-configurable property, whereas strict mode will throw an error in such a case.

Wrap-up

As is true with any technology, the better you understand why and how JavaScript works and doesn’t work, the more solid your code will be and the more you’ll be able to effectively harness to true power of the language. Conversely, lack of proper understanding of JavaScript paradigms and concepts is indeed where many JavaScript problems lie.

Thoroughly familiarizing yourself with the language’s nuances and subtleties is the most effective strategy for improving your proficiency and increasing your productivity. Avoiding many common JavaScript mistakes will help when your JavaScript is not working.

This post is originally published in Toptal’s blog written by Ryan J Peterson. He is a software engineer from Toptal.

Filed under: JavaScript

About the Author

Posted by

Tony is a web analyst and developer for Stampede. He is highly inquisitive, a quick learner and always ready to take on new challenges while trailblazing through the tricky forests of web programming. A family man - in his free time, he spends time with his lovely wife and daughter.

Leave a Reply