felixge / node-sandboxed-module

A sandboxed node.js module loader that lets you inject dependencies into your modules.
MIT License
342 stars 42 forks source link

locals does not appear to set variable inside the module #45

Open howarddierking opened 9 years ago

howarddierking commented 9 years ago

I'm trying to get locals working and suspect I'm doing something wrong as my scenario feels like it should be very basic.

In this simplified scenario, I have a test that should inject a local variable into my module which should override an existing variable. I then have an assertion that verifies the value was set.

mod.js looks like this

var foo = 'bar';

exports.getFoo = function(key){
    return foo;
};

and modTests.js looks like this

var SandboxedModule = require('sandboxed-module');

var mod = SandboxedModule.require('../lib/mod', {
    locals: { foo: 'baz' }
});

describe('mod', function(){
    it('should have value bar', function(){
        mod.getFoo().should.eql('baz');
    });
});

When I run the tests, I get an error that the function returns undefined. Not sure whether I'm doing something wrong here (I suspect so, as this seems like a really basic scenario) or if there's an issue in the code (I did step into it a bit, but it appeared that the local was at least collected from the options).

domenic commented 9 years ago

You can't override existing variables with locals, only inject new ones.

howarddierking commented 9 years ago

hmm - well, if I take away the module declaration of foo, I get the same result. Is there some difference in scope that is preventing exports.getFoo from reaching the injected variable?

domenic commented 9 years ago

At that point I'm guessing it's a bug :(.

howarddierking commented 9 years ago

ok - thanks for taking a look. Does it make sense to also add a feature request to enable overriding of existing variables? In my actual project, I'm looking for a way to fake out internal methods that coordinate more complex, I/O bound operations with a function that simulates latency. I can work around not having this capability by simply adding the function to the module's exports, but doing so adds otherwise unnecessary surface area to the module.

domenic commented 9 years ago

I don't see how it would be possible to override the existing variables. The existing locals transform is supposed to work by transforming

// your code

into

(function (local1, local2) {
// your code
}(value1, value2));