search

i-like-robots / rewireify

Rewireify is a port of Rewire for Browserify that adds setter and getter methods to each module so that their behaviour can be modified for better unit testing.
https://www.npmjs.com/package/rewireify
Other
59 stars 10 forks source link

Rewireify exposes __get__ and __set__ via Object.keys #13

Closed elicwhite closed 9 years ago

elicwhite commented 9 years ago

Currently with rewireify, __get__ and __set__ are part of the returned array when using Object.keys on a module that returns an object.

For example:

// foo.js

var Foo = {
  key: 'val'
};

module.exports = Foo;
var Foo = require('foo');
assert.sameMembers(Object.keys(Foo), ['key', '__get__', '__set__']);

However, I don't want my code to see anything about __get__ or __set__, such that my assert would be:

assert.sameMembers(Object.keys(Foo), ['key']);

I ran into this problem when building a port of rewireify for node, and used Object.defineProperty to fix it: https://github.com/TheSavior/rewire-global/blob/master/index.js

elicwhite commented 9 years ago

And this is the test I have for it: https://github.com/TheSavior/rewire-global/blob/master/test/tests/tests.js#L51

i-like-robots commented 9 years ago

Thanks for the heads up and sharing your code. This is something I'd considered and forgotten about, I think it's certainly worth adding.

elicwhite commented 9 years ago

It's causing one of our tests to fail right now and I'm uncomfortable manually adding get and set to our assertion. What do you believe should be the next steps? I'd be happy to try to put out a PR making rewireify use Object.defineProperty just like rewire-global. Do you agree with that approach?

i-like-robots commented 9 years ago

Very happy to accept a PR as my connection is super-flakey over 3G right now!