Don't clear entire require cache to force request module reload

[bmacnaughton]

stealthyRequire clears the entire require cache which causes problems when a module in the dependency tree executes code or maintains state.

The specific instance that I'm working with is the appoptics-apm performance monitoring code but this has the potential to impact other packages as well.

I replaced stealthyRequire with code that removes the request module from the require cache.

I went through all the ./lib/* dependencies and didn't see anything that was executed nor any state maintained in those modules. It doesn't look like it is necessary to invalidate them.

This should speed up the loading time by not invalidating the cache but I haven't tested that. It does work correctly (as far as I can tell) in the application I'm working with.

[bmacnaughton]

I am running the tests now and see the errors. I will follow up on them.

[bmacnaughton]

OK, I don't believe that I can fix this without changing request itself and that seems a bit out of scope. I am going to try to work around this in our own code, which is my immediate problem.

I believe that using stealthy-require is problematic as no module expects it's own initialization code to be executed multiple times. Given some time I will experiment with a more selective cache invalidation mechanism - something along the lines of (I'm making this concept API up now so apologies if I am missing something obvious):

const reExecuteAndLoadCache = require('reexecute-and-load-cache');

const packages = {
    request: 'node_modules/request/',
    rpn: 'node_modules/request-promise-native/',
    rpb: 'node_modules/request-promise-bluebird/',
    rpc: 'node_modules/request-promise-core/'
}

modules = reExecuteAndLoadCache(packages);

const request = modules.request;
const rpn = modules.rpn;

reExecuteAndLoadCache would only invalidate the cache when the key contains one of the values in the packages object. It would return the newly loaded modules' in an object using the keys supplied.

I can fix the error (correctly I think) with multiple instantiations of tough-cookie not recognizing the other instantiation's constructor by exposing tough on the request object itself. But it seems better not to tie it to tough-cookie specifically, so something like request.cookies would be better, but that's pretty close to the existing request.cookie. But however, because all the variations of request ultimately require request itself this approach seems like a reasonable approach. It would mean that client code should use request.makeCookie() or something similar as opposed to just making their own tough cookies - that's a breaking change. But it does abstract the cookie implementation so it could be changed if needed at some point.

Do any maintainers have any thoughts on this? I don't want to go in directions that make no sense to you, and while I've spent some time looking at the packages, I am just scratching the surface.

[analog-nico]

I am very sorry @bmacnaughton but I can’t remove this stealthy-require part. request-promise-native is patching request itself and thus has to be careful here. Large projects use request before they introduce request-promise-native for convenience. Without this require mechanism, request-promise-native would break these projects.

FWIW, request@3 will have a design that allows itself to be extended easily and with that I will be able to drop the stealthy-require part.

[bmacnaughton]

Thanks for the note. I have created a workaround that prevents stealthy-require from causing my module to be loaded twice, but look forward to request getting rid of stealthy-require.