-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
20f2811
commit 58d0f86
Showing
8 changed files
with
250 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,3 +101,4 @@ UpgradeLog*.XML | |
# Ignore others | ||
*.userprefs | ||
*.mdb | ||
.DS_Store |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// begin example.js | ||
var count = 0; | ||
|
||
var doSomethingShared = function(){ | ||
count += 1; | ||
console.log("Shared call count: %d", count); | ||
}; | ||
|
||
var doSomethingNotShared = function(){ | ||
console.log("Other modules don't know about me!"); | ||
}; | ||
|
||
module.exports.shared = exports.shared = doSomethingShared; | ||
module.exports.unshared = exports.unshared = doSomethingNotShared; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// begin fs-example.js | ||
var fs = require('fs'); | ||
|
||
// Naive expectation: read files in this directory asynchronously | ||
// and output head -1 and tail -1 for each. | ||
fs.readdir(__dirname, function(err, files){ | ||
if(err){ | ||
console.log('Error reading path %s: %j', __dirname, files); | ||
} else { | ||
files.forEach(function(file){ | ||
require('./processor')(file, function(d){ | ||
console.log('Contents for %s are:\n\t%s\n', file, d); | ||
}); | ||
}); | ||
} | ||
}); | ||
// end fs-example.js |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// Begin head.js | ||
var spawn = require('child_process').spawn; | ||
var head = function(path, callback){ | ||
if('function' !== typeof callback) { | ||
callback = function(d){ }; | ||
} | ||
|
||
setTimeout(function(){ | ||
var head = spawn("head", ["-1", path]); | ||
head.stdout.on("data", callback); | ||
}, Math.random() * 1000); | ||
}; | ||
|
||
module.exports = exports = head; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
<h1>CommonJS Modules, node's require() and private members</h1> | ||
|
||
Interestingly, node.js <a href="http://nodejs.org/docs/latest/api/modules.html" alt="node.js modules">module documentation</a> doesn't even mention CommonJS or the specification proposal it implements. I won't go over CommonJS <a href="http://wiki.commonjs.org/wiki/Modules/1.0" alt="CommonJS Modules 1.0">Modules 1.0</a> in-depth in this post, but I suggest you read both of these links if you plan to explore node.js development. | ||
|
||
An important take-away from this blog post (so important that I state it first) is that node.js caches modules. A module's exports are nothing more than an object. When you set a function or property to be exported, you're setting it on an object. | ||
|
||
Conceptually, this: | ||
|
||
[js] | ||
var someFunction = function(){ /* implement */ }; | ||
module.exports.someFunction = exports.someFunction = someFunction; | ||
[/js] | ||
|
||
...is the same as doing this outside of CommonJS modules... | ||
|
||
[js] | ||
var someFunction = function(){ /* implement */ }; | ||
var exports = {}; | ||
exports.someFunction = someFuntion; | ||
[/js] | ||
|
||
I know, that seems like it should be common sense. When node.js caches the exports object at the application level, I think understanding this can become hairy -- especially if you come from a compiled language background. | ||
|
||
For example, consider the following module. | ||
|
||
[js] | ||
var count = 0; | ||
|
||
var doSomethingShared = function(){ | ||
count += 1; | ||
console.log("Shared call count: %d", count); | ||
}; | ||
|
||
var doSomethingNotShared = function(){ | ||
console.log("Other modules don't know about me!"); | ||
}; | ||
|
||
module.exports.shared = exports.shared = doSomethingShared; | ||
module.exports.unshared = exports.unshared = doSomethingNotShared; | ||
[/js] | ||
|
||
We can require this module easily and play around with it in node REPL: | ||
|
||
[plain] | ||
$ node | ||
> var example = require('./example.js'); | ||
undefined | ||
> example.shared() | ||
Shared call count: 1 | ||
undefined | ||
> example.shared() | ||
Shared call count: 2 | ||
undefined | ||
> example.shared() | ||
Shared call count: 3 | ||
undefined | ||
> example.shared() | ||
Shared call count: 4 | ||
undefined | ||
> example.unshared() | ||
Other modules don't know about me! | ||
undefined | ||
[/plain] | ||
|
||
In the same REPL instance, you can re-require the module and see that 'count' is indeed shared: | ||
|
||
[plain] | ||
> var example2 = require('./example.js'); | ||
undefined | ||
> example2.shared() | ||
Shared call count: 5 | ||
undefined | ||
> example2.shared() | ||
Shared call count: 6 | ||
undefined | ||
[/plain] | ||
|
||
You can even require the shared function directly: | ||
[js] | ||
> var shared = require('./example.js').shared; | ||
undefined | ||
> shared() | ||
Shared call count: 7 | ||
undefined | ||
> shared() | ||
Shared call count: 8 | ||
undefined | ||
[/js] | ||
|
||
I think you get the point; private variables which aren't exported are shared. In this example, 'count' is a private static property of the module. | ||
|
||
<h2>Why this matters</h2> | ||
|
||
This is very important, because when working in the asynchronous environment of node.js you have to be aware about *what* is being cached privately at the module level. In addition to this, if you plan to cache functions for whatever reason, you have to be incredibly sure you know what you're doing. I recommend to <strong>NEVER CACHE CALLBACK FUNCTIONS</strong>. | ||
|
||
As an example of this statement, I've created a sample file processor script in the github repo as fs-example.js. That script reads this blog's directory in the repo, then loops over the files and executes 'processor', which is a custom broken module displaying the above problem. fs-example.js does an inline require, which as you've seen above doesn't make a difference. | ||
|
||
The problem I'll display here exists because async calls are likely to take different lengths of time to execute. My example is using the linux commands 'head -1' and 'tail -1' on each file to get the first and last lines, respectively. These commands are nearly instantaneous, so I'm using a random setTimeout to display the problem with caching functions. You could easily modify these scripts to 'head' and 'tail' different web sites. But, whatever. Because head.js and tail.js are simple spawn wrappers, I'll focus on processor.js, which is where the potential bugginess exists. | ||
|
||
[js] | ||
// begin processor.js | ||
var head = require('./head'), | ||
tail = require('./tail'); | ||
|
||
// don't do this IRL | ||
var callback = null; | ||
|
||
var processor = function(file, cb){ | ||
if('function' === typeof cb){ | ||
callback = cb; | ||
} | ||
|
||
// these fail | ||
head(file, function(d){ | ||
callback(d) | ||
}); | ||
tail(file, function(d){ | ||
callback(d) | ||
}); | ||
|
||
// these succeed | ||
// head(file, callback); | ||
// tail(file, callback); | ||
}; | ||
|
||
module.exports = exports = processor; | ||
[/js] | ||
|
||
This is a simple file for having such a subtle bug. The file includes the customized head and tail scripts. Then, it creates a function that tags a file, caches the callback function, and calls head and tail to execute the callback. You'll notice the commented-out lines at the end that claim 'these succeed'. Here's why... | ||
|
||
When a callback is passed directly to a function as is done with <code>head(file, callback)</code>, the function itself is passed as a reference. Meaning, when the head module executes that callback, it will always execute the referenced callback. Now, what if another developer wants to add logging or other processing of the data? You'll likely get something like the example, where a function is created and callback is called directly (rather than the cb passed into the function originally). Because 'callback' is cached at the module level and shared across calls to processor, the scope created by the processor function is shared with the anonymous function callback and points to whatever is the most recent callback function rather than what one might expect to be the intended function. | ||
|
||
We could avoid the problem in the above non-commented code by referencing the 'cb' parameter rather than the cached 'callback' variable. This is because processor creates a closure over 'callback', but 'cb' would always be within the current execution context. | ||
|
||
You can play around with the code from this post, which is available on <a href="https://github.com/jimschubert/blogs" alt="jimschubert github">github</a>. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// begin processor.js | ||
var head = require('./head'), | ||
tail = require('./tail'); | ||
|
||
// don't do this IRL | ||
var callback = null; | ||
|
||
var processor = function(file, cb){ | ||
if('function' === typeof cb){ | ||
callback = cb; | ||
} | ||
|
||
// these fail | ||
head(file, function(d){ | ||
callback(d) | ||
}); | ||
tail(file, function(d){ | ||
callback(d) | ||
}); | ||
|
||
// these succeed | ||
// head(file, callback); | ||
// tail(file, callback); | ||
|
||
// so do these | ||
// head(file, function(d){ | ||
// cb(d) | ||
// }); | ||
// tail(file, function(d){ | ||
// cb(d) | ||
// }); | ||
}; | ||
|
||
module.exports = exports = processor; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// begin tail.js | ||
var spawn = require('child_process').spawn; | ||
var tail = function(path, callback){ | ||
if('function' !== typeof callback) { | ||
callback = function(d){ }; | ||
} | ||
|
||
setTimeout(function(){ | ||
var tail = spawn("tail", ["-1", path]); | ||
tail.stdout.on("data", callback); | ||
}, Math.random() * 1000); | ||
}; | ||
|
||
module.exports = exports = tail; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
#!/usr/bin/env node | ||
var fs = require('fs'); | ||
var date = new Date(); | ||
var newDir = date.toISOString().split('T')[0]; | ||
|
||
fs.exists(newDir, function(exists){ | ||
if(exists){ | ||
process.stdout.write(newDir + ' already exists!\n'); | ||
process.exit(); | ||
} else { | ||
fs.mkdir(newDir, 0700, function(err){ | ||
if(err){ | ||
process.stdout.write(newDir + ' could not be created: ' + err.message + '\n'); | ||
process.exit(1); | ||
} else { | ||
process.stdout.write(newDir + ' has been created.\n'); | ||
process.exit(); | ||
} | ||
}); | ||
} | ||
}); |
58d0f86
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wtf is up with that commit message? I even verified in my bash history: