Sharing code between rewireify and rewire-global

[elicwhite]

I maintain rewire-global, which is very similar to rewireify except for node. Rewireify adds rewire to every module via a browserify transform. rewire-global adds rewire to every module by modifying node's internal require wrapper.

If you look at rewire-global's main file, you'll notice that things are pretty much identical to that of rewireify:
https://github.com/TheSavior/rewire-global/blob/master/index.js

I noticed this commit in rewireify that verifies that an object is extensible. Rewire-global would benefit from that.

I noticed this issue about rewireify not currently injecting the functions when module.exports is a function. Rewire-global currently does that.

It seems like it would be in both of our best interests to figure out how to pull out the necessary wrapping code into a common module and then support it being used via both browserify and node. Thoughts?

[elicwhite]

Adding @speedskater from babel-plugin-rewire and @jhnns from rewire

[i-like-robots]

@TheSavior A global override of the require behaviour would be ideal but when this module was initially written that wasn't possible. At the time Browserify did not expose the require function but this may have changed since Browserify v3. This difference between Rewire and Rewireify is quite fundamental, there is some discussion of it throughout this issue.

In defence of the current approach though; it is very basic. This means that's been easy to maintain and test and it hasn't required any compatibility changes as Browserify has been updated from version 3 through to 11.

[elicwhite]

Yep, I totally agree with that. I think rewire is the base functionality that defines the methods and Apis used to actually rewire things.

Rewireify and Rewire-Global do the same thing for different environments (rewireify browser, rewire-global node), but they could be injecting the same code via their own mechanisms.

Babel-plugin-rewire is in kind of an interesting place. It is a rewrite of rewire's main get and set functions to support es6 type things such as async functions, const values, etc. and then uses babel's transform api to inject it into every module just like rewireify / rewire-global.

It seems like we could find some way to pull the shared functionality together.

[elicwhite]

In regards to rewireify, rewire-global, and babel-plugin-rewire having a different api than rewire since they all inject the code into each and every module instead of requiring the developer to call rewire() instead of require, I believe we actually prefer that.

We use Proxyquire with rewire via rewireify on the client, and rewire-global in node so that we can have isomorphic/universal code that works the same way in tests.

[jhnns]

@TheSavior @i-like-robots @speedskater thank you guys for your effort. I'm amazed about your motivation for bringing rewire to browserify/babel.

However, I'm a bit concerned that things get complicated. Especially if your rewire adoptions have a slightly different API. I would like that users could just drop-in rewireify or the babel-plugin-rewire without needing to know about the differences behind it. So these are my thoughts about streamlining the modules:

• rewire derivatives should use modules inside the lib-folder as much as possible, especially getDefinePropertySrc.js, getImportGlobalsSrc.js and detectStrictMode.js. If something doesn't fit you, we may adjust it in rewire.
• Is rewire-global the preferred way to use rewire? What are its pros/cons? I would like to have a discussion about it. After all we may implement it in rewire as breaking change...

Could you guys give me your opinion about it? Or is the current situation ok?

What are your thoughts about API changes to rewire that might impact you? Especially jhnns/rewire#73

[elicwhite]

@jhnns I absolutely, wholeheartedly agree that our APIs should be identical.

At our company we have three folders of js code, browser, common, and node. We have two test suites, karma which runs browser and common, and mocha which runs common and node tests. We have to make sure that our test suites work exactly the same way so that our developers are fluid in any of those folders (and that common will just work on both test suites).

In our karma test suite we use rewireify to globally inject rewire into all modules via browserify. In our mocha test suite we use rewire-global to globally inject rewire into all modules via node's module require wrapper.

We are looking to switch over to babel, and so we would want to just swap out those layers for babel-plugin-rewireify.

So I very much agree that the APIs for all of these need to be the same. :)

I spoke with @speedskater about using the modules inside the lib folder for babel-plugin-rewire and I believe these were some of the concerns he had.

1. Rewire doesn't support ES6 functionality: const values, async functions, default / non default exports, etc.
2. Babel might change variable and function names during the transformations so it can't just inject the get and set functions from lib. It needs to use the transformation API so it can get reference to specific identifiers and it sets up the internals a bit different.

It seems like the first problem could be solved in Rewire. We add support for these features and that shouldn't cause problems for people who aren't using ES6.

The second problem is a bit more of a problem. One approach I've been toying with in my head is for us to move the babel transforms into rewire, write the core functions as transforms, and then on release run babel to compile the transforms into regular es5 strings like we have now. Then rewireify and rewire-global could continue using strings as we have them, and babel-plugin-rewire would be happy and we would all have the exact same API.

In regards to sharing other code more, I agree that we should definitely be using the other modules in the lib folder as well. I didn't actually know they existed. I'll be updating to that. The other line of code that we could really gain something by sharing is this one:

rewireify:

post += "/* This code was injected by Rewireify */\n"; // the \n on this line is functionally important
post += "if (Object.isExtensible(module.exports)) {\n";

rewire-global: (I haven't updated to Object.isExtensible yet)

var embed = "\nif (typeof(module.exports) === 'object' || typeof(module.exports) === 'function') {\n" +

babel-plugin-rewire:

if ((typeof _defaultExport === 'object' || typeof _defaultExport === 'function') && Object.isExtensible(_defaultExport)) {

That one line is very hard to keep in sync across all three projects but there is no reason that I can think of for why it should be any different. We should try to figure out how we can pull that up into rewire as well.

Which leads me to my proposal for what these different packages do:

1. Rewire becomes the definitions of the base lib files such as __get__, getImportGlobalSrc, etc (via a babel transform hopefully)
2. Rewireify, rewire-global, and babel-plugin-rewire all hook these functions into different ways that it can be used.
3. Rewire pulls the functionality for being able to call rewire() instead of require() out into another package that uses the rewire base functions and would then become siblings of the three packages above.

[speedskater]

@TheSavior @i-like-robots @jhnns
Thank you for the discussion about sharing code and unifying api and interfaces.

Generally I like the idea of swapping out rewire like implementations depending on the environment. Therefore similar behavior is definitively desired.

Regarding actually sharing the code I think i becomes a bit more complex. The reason is that in the case of babel-plugin-rewire we have to generate the rewirering code (set, get....) through AST transformations. Therefore in case of babel-plugin-rewire the functionality of setting and getting a variable value is quite different from the one used by rewire (which basically uses eval functions @jhnns or did i get that wrong)) or rewirefy (which I think uses rewire's functionality).

Therefore I think only some parts of the code can be shared. e.g. storing of the original rewired values, reset functionality or similar. Also lines like the one mentioned by @TheSavior in the previous comment might be ideal candidates for sharing.

Therefore I think for actually unifying and sharing parts of our code we should:

• first create and define a set of samples and tests, testing get and set functionality for all kind of language constructs we want to support and unify.
• in case of babel-plugin-rewire we have to create functionality for rewiring global variables on a per module basis to support the same functionality as rewire (@jhnns is there some other functionality missing).
  I think it should be able to detect global variables through AST transformations and actually bind them to locally scoped ones. @TheSavior what do you think?
• find out what can actually shared between all different implementations -> generate AST nodes representing this code -> These AST nodes can than be used in transformation based libraries like in babel-plugin-rewire or directly generate code which can be included in rewire.js.
• determine a set of "user level" utility functions in a common project like which solely work on the common methods set, get and so on...

@TheSavior @i-like-robots @jhnns @shebson What do you think about this proposal. By the way is someone of you attending jsconf.eu?

[elicwhite]

@speedskater It sounds like as you mention, the biggest problem is needing to have the rewire code get and set written as AST transforms which don't currently exist in rewire which is the thing that babel-plugin-rewire would need to be able to share code. @jhnns, do you have any ideas on this?

It seems like the other things you mention are pretty consistent with my proposal as well. Are there other big problems you foresee?

I am not attending jsconf.eu. I'm based in the San Francisco area so I try to go to most of the conferences out here instead. :)

[elicwhite]

@jhnns Any more thoughts on how we can bring these together and possibly even write the rewire core functions using the tree transformers so that it would work with every project with the exact same api?