CODEWITHSUNDEEP
callbacknode.jsclosures
dsimard
dsimard

Reputation: 7428

Are closures different in node.js?

I have worked quite a lot with javascript but yesterday, I started using node.js. It's a little script that runs jslint on the files of a folder. For this example, I changed the command to call ls instead of jslint.

var sys = require("sys");
var fs = require("fs");
var cp = require('child_process');

var path = fs.realpathSync("./src/");

fs.readdir(fs.realpathSync("./src/"), function(err, files) {
  for (var i = 0; i < files.length; i++) {
    var filename = files[i];
    var complete = path + filename;

    // Run jslint on each file
    var jslint = cp.exec("ls " + complete, function(error, stdout, stderr) {
      console.log(filename + " : " + stdout);
    });    
  }
});

The output is this :

jskata.nofreeze.js : /home/dan/php/jskata/src/jskata.undo.js

jskata.nofreeze.js : /home/dan/php/jskata/src/jskata.nofreeze.js

jskata.nofreeze.js : /home/dan/php/jskata/src/jskata.timezone.js

Why do the line console.log(filename + " : " + stdout); always prints jskata.nofreeze.js when the filename should obviously match the result of the ls? Are closures and scopes different in node.js than in javascript?

Upvotes: 1

Views: 1974

Answers (2)

mcfedr
mcfedr

Reputation: 7975

you have slightly confusing code where you write this

for (var i = 0; i < files.length; i++) {
    var filename = files[i];

because variables in javascript only have scope a the function level, so the filename variable is reused each time around the loop.

writing the code to this:

var filename;
for (var i = 0; i < files.length; i++) {
    filename = files[i];

might make it clearer where the problem is. and why as the previous answer suggests you need to pass filename to your function.

Upvotes: 2

Ivo Wetzel
Ivo Wetzel

Reputation: 46745

No they aren't any different, this is just the most common pitfall with closures in JavaScript.

See, you're in a loop, you're assigning the local variable filename on every iteration of the loop, therefore you're in fact overriding it's value. Since it's always the same local variable, and the closure works per reference, the value of filename inside the callback gets updates on each iteration of the loop.

'jskata.nofreeze.js' just happens to be the last file in the directory and therefore it's also the last value that gets assigned to filename.

To solve it you need to pass the value of filename per value:

// Run jslint on each file
(function(c, f) {
    cp.exec("cat " + c, function(error, stdout, stderr) {
      console.log(f + " : " + stdout);
    });
})(complete, filename);

This makes the whole thing work, even though it's a bit ugly. You can of course move this out into a named function if you don't want to clutter your inner loop with more anonymous functions.

Upvotes: 13

Related Questions