module.exports colliding / being overwritten in node.js application

Question

module.exports colliding / being overwritten in node.js application

I think I'm badly misunderstanding how to use module.exports. It seems every module is overwriting what the last one spit out.

app.js:

var express = require("express")
    , app = express()
    , routes = require('routes')
    , server = app.listen(1337, "0.0.0.0")
    , io = require('socket.io').listen(server)
    , redis = require("redis")
    , client = redis.createClient();

var moduleA = require("./moduleA")(io, client); (need to pass socket.io and redis client)

var moduleB = require("./moduleB")(io, client); (same)

moduleA.js:

module.exports = function(io, client){ 
    this.test = 'foo';
    return this;
};

moduleB.js:

module.exports = function(io, client){ 
    this.test = 'bar';
    return this;
};

back to app.js:

console.log(moduleB.test); (prints "bar")

console.log(moduleA.test); (also prints "bar")

Could someone explain what I'm doing wrong? I can't think of any other way to do this, since the exports helper (?) itself doesn't seem to accept parameters.

javascript
node.js
express
node-modules

Answer 1

You are exporting a constructor. You need to construct it, not call it.

Change

var moduleA = require("./moduleA")(io, client);

to

var moduleA = new (require("./moduleA"))(io, client);

or (for clarity)

var ModuleA = require("./moduleA");
var a = new ModuleA(io, client);

What you are seeing happen is the usual behavior when calling constructors as functions in sloppy mode: this references the global object. So of course modifying the global object from two locations will overwrite each other, and returning this will just return the global object. You can test this yourself: with your current code,

moduleA === moduleB === this === global

One way to prevent yourself from ever shooting yourself in the foot like this again is to use strict mode instead of sloppy mode. To do so, add the line

"use strict";

at the top of every module you write (before any other code). In strict mode, this for constructors called without new is undefined, so you will get an earlier and much easier-to-understand error.

Strict mode has many benefits of this sort; for an overview see [1], [2], [3].

An alternate solution is to stop using constructors altogether, but instead use factory functions. This would look like:

module.exports = function (io, client) {
    var result = {};
    result.test = "foo";
    return result;
};

You seem to be trying to do something like this anyway, since you return this even though doing so in a constructor is entirely unnecessary. You could just stop using this and use an actual object under your control, instead of one whose semantics change depending on whether your function is being called or constructed.