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

instanceof Function broken inside of sandboxed module #44

Open OliverJAsh opened 9 years ago

OliverJAsh commented 9 years ago

I have a sandboxed module with the following contents:

var foo = function () {}
console.log(1, typeof foo);
console.log(2, foo instanceof Function);
// => 1 'function'
// => 2 false

foo instanceof Function should return true, but it returns false.

OliverJAsh commented 9 years ago

I imagine this happens because globals are carried across from the outside of the sandboxed-module? Will instanceof be broken with other types then?

OliverJAsh commented 9 years ago

@tristanls That works, but this is third party code. I don't want to have to fork every dependency that uses instanceof <global>.

tristanls commented 9 years ago

@OliverJAsh yes, I removed the comment :)

domenic commented 9 years ago

Argh, this is frustrating. We copy over the globals because #13 (which was how I got involved in the project it looks like). There, the tests would do e.g. instanceof Error, but the system under test would be sandboxed, and so the tests would fail since the sutGlobal.Error wasn't equal to the testGlobal.Error.

I am unsure how we could satisfy both situations. Maybe just offer an option, but that kind of sucks :(.

joshperry commented 9 years ago

This actually causes more subtle issues. Here's another example:

sandboxed.js

var defaults = { foo: 'bar' }
exports.dostuff = function dostuff(options) {
  if(!options) {
    options = {};
  }

  options = _.merge(options, defaults);

  return options;
}

test.js

it('should use defaults with no params', function() {
  var opts = mod.dostuff();
  opts.should.contain({ foo: 'bar' });
});

This throws an error because should is not on the object created in the sandboxed module.

domenic commented 9 years ago

@joshperry that's basically the definition of sandboxing, though. You aren't affected by modifications to the environment's prototypes; instead you run in your own clean sandbox.

joshperry commented 9 years ago

@domenic Yeah, sorry. This was being used as a way to mock required modules in another project that I'm trying to clean up. I assumed that this lib was interested in being friendly to testing, considering your change for #13; looks as though I assumed wrong. I'll change my code to use proxyquire.

elliotstokes commented 9 years ago

I'm having this issue as well. A quick and dirty workaround is to inject the dependency - so I was getting the issue with the moment date lib.

test = sandboxedModule.require('filetotest', {
       requires: {
        'moment' : require('moment')
     },
});
joshperry commented 9 years ago

@elliotstokes Great idea.

tristanls commented 8 years ago

I ran into this again, this time without being able to work around it. My code was using Joi to do some validation:

...
  thing: Joi.object().type(Map)
...

and that runs again into the instanceof problem.

As @domenic pointed out before, this module sandboxes, and that works great for most tests as I've happily written hundreds of tests using this module. This is the only corner case I couldn't work around.

For testing, using proxyquire resolved my instanceof issue.