Learning a fun lesson about JavaScript method parameters

I'll be honest – I don't do much front-end stuff. I've watched the odd PluralSight course on modern JavaScript, I've worked out the basics of Gulp, and I can hack together a VueJS UI if I need to. But it's certainly not something I'd ever say I was good at. But despite being offically a C# developer, occasionally I find myself looking at bug tickets that relate to some front-end code. I had one of them this week, where some javascript had stopped working. The front-end dev was stuck, so I took a look – and discovered something new. Well new to me at least...

The issue

The page in question contained a chunk of code to implement some "suggest as you type" functionality in a search box. The back-end code supplied a chunk of json which described the data set, and the front-end applied some filtering to this based on what the user typed, in order to show helpful suggestions. It had all been working, but after a round of PRs into the codebase it had stopped working.

Stripping away all the client-specifc noise, what it basically consisted of was a chunk of data obtained from the back-end:

var dataSet = [
	{ name: "One A", data: "1" },
	{ name: "Two B", data: "2" },
	{ name: "Three C", data: "3" },
];

					

Followed by some code that iterated this data, to do some processing and mapping. It happens that this code was recycled from elsewhere, and made use of jQuery. The actual processing logic doesn't matter here – but when I looked at the code, the iteration behaviour looked like this:

$.each(dataSet, function(item) {
	doSomething(item.name);
});

					

That looked ok to me at first glance – and none of the other devs had spotted an issue – but overall the code was no longer working. The code in doSomething() would fail because the incoming parameter was undefined - despite the data clearly having the right property. What had gone wrong?

An answer

After a lot of messing about in the debugger (there was vastly more code than I'm showing here in the overall solution) I noticed that the doSomething() function wasn't receiving a string as I would have expected, and if I looked at the value of the item variable inside the for-each loop, it wasn't an object but an integer. Combining this with some digging into the git history, and some helpful comments from the front-end dev who was looking at the bug too, it turned out the issue was how that for-each loop was dealing with its parameters...

If you look at the documentation for .each() on the jQuery site then it talks about the function declared there taking two parameters:

function
Type: Function( Integer index, Element element )
A function to execute for each matched element.

After a bit of thinking that explained the issue for me: one of the previous commits in the project had removed an "unused" parameter from the inner function. When the code was working, it looked like:

$.each(dataSet, function(index, item) {
	doSomething(item.name);
});

					

The integer index parameter wasn't being used, and the item parameter contained the expected object. But some over-zealous refactoring had removed the apparently unused parameter. I'm guessing some sort of jshint warning was being thrown up? And it was changed to:

$.each(dataSet, function(item) {
	doSomething(item.name);
});

					

And jQuery / JavaScript didn't really care. The array index value was happily mapped into item and the array element had no parameter to be mapped into so was ignored. That meant any reference to item.name just returned nothing, as that property doesn't exist on an integer...

What did I learn?

I turns out that function parameters in JavaScript don't work the way my C# brain expects. This is valid code:

function test(x, y)
{
	console.log("x: " + x);
	console.log("y: " + y);
}

test(10);

					

It may not be good code – but it runs without any errors. The parameter you omit is just passed as "undefined". No warnings in the console, no errors at runtime.

So mostly I learned that JavaScript is just flexible enough to let you reach down and shoot yourself in the foot...

↑ Back to top