Different behavior between 1.7.4 and 1.7.9

[ilyavolodin]

Just upgraded proxyquire to 1.7.9 and my tests are no longer passing. My code runs in Rhino with custom implementation of CommonJS, but tests are ran in Node, that's why I have to use proxyquire to mock some requires. Below is a short version of failing code in 1.7.9, it was working fine in 1.7.4:

src/categories.js

'use strict';

var helper = require('~/dwHelpers');
var URLUtils = require('dw/web/URLUtils');

....

test/categories.js

var helper = require('src/dwHelpers');
var proxyquire = require('proxyquire').noCallThru().noPreserveCache();
var urlUtilsMock = {};

describe('categories', function () {
    var Categories = null;
    beforeEach(function () {
        Categories = proxyquire('src/categories', {
            '~/dwHelpers': helper,
            'dw/web/URLUtils': urlUtilsMock
        });
    });
    it(...)
});

With 1.7.9 beforeEach fails with the following error:

  1) categories "before each" hook for "should convert API response to an object":
     Error: Cannot find module '~/dwHelpers' from '/Users/ivolodin/Documents/Code/mfsg/src/categores'
      at Function.module.exports [as sync] (node_modules/proxyquire/node_modules/resolve/lib/sync.js:33:11)
      at node_modules/proxyquire/lib/proxyquire.js:187:24
      at Array.map (native)
      at Proxyquire._withoutCache (node_modules/proxyquire/lib/proxyquire.js:186:40)
      at Proxyquire.load (node_modules/proxyquire/lib/proxyquire.js:136:15)
      at Context.<anonymous> (test/unit/app_storefront_base/categories.js:34:22)

My solution for now is to pin to 1.7.4, but I'm not sure that's expected behavior.

[bendrucker]

Hmm, I have to assume it's got something to do with the ~ in the path. Can you try replacing that with an absolute path?

[ilyavolodin]

Sorry, I forgot to mention, if I comment out the line with ~, I get the same exact error for dw/web/URLUtils as well. That path and file doesn't exist, it's a Rhino abstraction for Java class that's running under the hood (it's actually dw.web.URLUtils Java class). Unfortunately I can't remove ~ from the path, as I mentioned, my prod instance is running on Rhino with custom implementation of RequireJS.

I understand that doesn't align with your use-cases, and maybe locking my dependency down to 1.7.4 is the way to go, but it was working before, and doesn't work with newer version, it's possible that whatever changed during that time affects other things as well.

[bendrucker]

Ah, okay. This change was definitely caused by the addition of node-resolve instead of trying to use internal Module methods for tricky lookups. You should definitely pin to 1.7.4 since I don't think we'll be able to reasonably add proper testing around this behavior.

[thlorenz]

Actually ideally we implement the module lookup algorithm exactly the way it works for node, i.e. if node can resolve the above module, proxyquire should too. If node-resolve behaves differently then maybe someone could PR there to make it behave more like node's lookup mechanism? As it stands now it looks like a regression to me.

[ilyavolodin]

@thlorenz Examples that I posted above would not work in node. As I understand it (probably incorrectly), before 1.7.5 require match was just a string match, now it's actual file lookup. Those files do not exist in my repository, so Node wouldn't be able to find them (Rhino does, since it's running custom implementation of CommonJS). So if the goal is to mimic Node's CJS implementation, then I have no choice other then to pin to 1.7.4, although it feels very limiting to me.

[thlorenz]

Thanks for the clarification @ilyavolodin in that case I agree with @bendrucker that the best option for you here is pinning to 1.7.4 since that (solely by chance) has the behavior that you need.

[bendrucker]

This goes back to #102 where the only solution was to begin resolving the stubs in order to manipulate the cache. The require.cache uses absolute paths and is global to all your modules. You're running into this because you're using noPreserveCache. In order to actually not preserve the cache, proxyquire has to resolve those stub paths and then delete them from the cache.

Here's the code that throws: https://github.com/thlorenz/proxyquire/blob/master/lib/proxyquire.js#L187-L191

I think it's reasonable to try/catch that so that bad stub paths behave identically when using noPreserveCache versus with it. A bad path is a noop everywhere else in the library. After re-reading everything here I think you're right that this shouldn't break entirely in v1.

You may still get some weirdness since your stubs can't be deleted from the cache.