thlorenz / proxyquire

🔮 Proxies nodejs require in order to allow overriding dependencies during testing.
MIT License
2.75k stars 99 forks source link

Did `preserveCache` stop working and `noPreserveCache` become the default? #222

Open Druotic opened 5 years ago

Druotic commented 5 years ago

A coworker was asking me about noPreserveCache and when it is/isn't needed, and I was a bit surprised to find the example in the docs for preserveCache are no longer accurate. In short, it seems like the answer is now 'noPreserveCache' is never explicitly needed, because it's the default :)

Based on https://github.com/thlorenz/proxyquire#forcing-proxyquire-to-reload-modules:

var proxyquire = require('proxyquire').preserveCache();
var assert = require('assert');

var stubs = {};
var foo1 = proxyquire('./foo', stubs);
var foo2 = proxyquire('./foo', stubs);
var foo3 = require('./foo');

assert.equal(foo1, foo2);
assert.equal(foo1, foo3);

For testing, I created a simple foo.js with a single line module.exports = {}; The asserts fail with an error like:

AssertionError [ERR_ASSERTION]: {} == {}

Is this expected? I haven't found any recently issues/PRs (yet) that justify this change in behavior.

This was using proxyquire v2.1.0

bendrucker commented 5 years ago

Hi there, there was definitely quite a bit of ambiguity/bugs around cache and some recent changes related to it. Might've been a regression or maybe we just need to update the docs.

When it comes to preserving cache, the key ambiguity is whether proxyquire is asked to preserve:

I believe the first is working properly but the latter may be broken. It's not clear to me how people are using these features.

I'd be interested in getting it right in a proxyquire@3 so if you can share your use case that'd be helpful!

Druotic commented 5 years ago

Hey, sorry for the delayed response and thanks for the speedy response.

My use case isn't very unique. I was just proxyquiring modules and I did not want the stubbed modules to get cached, so I used the .noPreserveCache() function. I've been doing this for at least a couple of years now, because I almost always want to ensure the stubbed version does not stick around in the require cache. I only want it to be used for that one test, with the next test starting fresh to avoid any side effects.

One day, I tried omitting the noPreserveCache() and that's when I thought to myself 'wait... what? This works?! Why have I been adding this function call to every test file for so long?' As a sanity check, I tested out the examples in the README, and now here I am 😄

To be clear - I think this behavior is just fine, and it's what I originally expected the default to be when I first started using Proxyquire. I only used noPreserveCache() because proxyquire originally didn't do what I expected. I'm just making the observation that the README doesn't reflect reality anymore - I wasn't sure if that was a design decision somewhere along the way (outdated docs) or a regression.