Fix issue where rewire can hold reference to its own extension

[dhensby]

This attempts to fix a slightly strange issue I had when using rewire with ts-node. For some reason (I think because the ts-node loader is doing some magic with the js loader too) if I include rewire several times in my files it ends up holding a reference to itself as the originalExtension instead of the true original.

Error:

RangeError: Maximum call stack size exceeded
    at restoreExtensions (node_modules\rewire\lib\moduleEnv.js:99:27)
    at Object.jsExtension [as js] (node_modules\rewire\lib\moduleEnv.js:135:5)
    ...

This patch fixes the problem for me locally, but I'm not sure if there's a wider impact to this fix nor if there's a more rigorous fix for this.

[jacobjmarks]

Seconded. I have also been encountering this issue and can confirm this change resolves the problem.

[jhnns]

Thank you for your PR. I don't understand how this change fixes the problem. Do you have a test case to reproduce the issue? Maybe it's a better idea to use the pirates package (as already noted as TODO comment in the file).

[dhensby]

I'm not entirely sure how the problem I experience comes about and so reproduction is a bit tricky. I suspect that rewire is somehow pulling in a rewire (or an uncached require???) call itself, so it's essentially a nested rewire call. That causes the "original" extension to be re-assigned before the restoreExtension() function is called, which means the reference to the truly original extension is lost and replaced with the rewire one, preventing the original extension from ever being brought back.

This PR turns the originalExtensions into a stack, instead of a single reference, and that means that even if registerExtension() and restoreExtension() aren't called in expected order, then you still always get back to the original extension eventually.

The change makes the library resilient to a set of calls like this: registerExtension() -> registerExtension() -> restoreExtension() -> restoreExtension(), which at the moment it is not capable of handling.

Regarding pirates: I hadn't seen the comment or heard of pirates before, I had seen require-in-the-middle but that doesn't let you replace the compile step so isn't any good, but pirates could be a good option... As a side-note, I've also been experiencing some other really bizarre errors (again, hard to reproduce) where the "context" (for want of a better word) of the require call is lost and it tries to resolve files in completely incorrect directories. I've been trying to get to the bottom of it and I'll open a new issue with some context when I'm able to spend some time digging into it.

[jhnns]

You know what's strange with this error? restoreExtensions doesn't contain any function call (and afaik it never has). So how can it trigger a Maximum call stack error? The only situation I can think of is that someone registered setter functions on require.extensions which get triggered once we assign something. Can you check with Object.getOwnPropertyDescriptors(require.extensions)?

Personally I see two options of how to proceed:

• You manage to provide a reproducible example so that we can understand what's going on
• We use something like pirates in the hope of fixing this strange error

Since I don't understand why the array solution is fixing this behavior, I'm not convinced of this change yet 😞. I couldn't guarantee not to break it in the future again since there is also no test for it.

[dhensby]

Absolutely agree - I'd really like to get to the bottom of it and I also suspect there is some other library that is also hooking into requires which is causing the problem. I've tried deep diving into it with a debugger but it quickly becomes difficult to diagnose what's going on because the calls can literally come from anywhere...

If I find some time, I'll try to dig deeper and even see if I can produce a minimal reproduction of it (which no doubt will help me find the culprit).

[dhensby]

I'm going to close this for the time being... A repo I was using my fork on now works with v7, so I'll only reopen this once I have a minimal replication that we can investigate.