Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add throwOnNotFound method #225

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
17 changes: 17 additions & 0 deletions lib/proxyquire.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ Proxyquire.prototype.preserveCache = function () {
return this.fn
}

/**
* Stubbing a module that does not exist (from the perspective of the proxyquire
* subject) will throw an error.
* @name throwOnUnresolved
* @function
* @private
* @return {object} The proxyquire function to allow chaining
*/
Proxyquire.prototype.throwOnUnresolved = function () {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throwOnNotFound is the best I can come up with. Handling unresolved stubs is more of a description of how proxyquire works as opposed to what the user wants.

this._throwOnUnresolved = true
return this.fn
}

/**
* Loads a module using the given stubs instead of their normally resolved required modules.
* @param request The requirable module path to load.
Expand Down Expand Up @@ -140,6 +153,10 @@ Proxyquire.prototype._resolveModule = function (baseModule, pathToResolve) {
paths: Module.globalPaths
})
} catch (err) {
if (this._throwOnUnresolved) {
throw new ProxyquireError(err.message)
}

// If this is not a relative path (e.g. "foo" as opposed to "./foo"), and
// we couldn't resolve it, then we just let the path through unchanged.
// It's safe to do this, because if two different modules require "foo",
Expand Down
14 changes: 14 additions & 0 deletions test/proxyquire-argumentvalidation.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,18 @@ describe('Illegal parameters to resolve give meaningful errors', function () {
throws(act, /Invalid stub: "myStub" cannot be undefined/)
})
})

describe('when I pass a stub with a key that is not required on the proxyquired object', function () {
function act () {
const proxyquireThrowOnUnresolved = proxyquire.throwOnUnresolved()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const proxyquireThrowOnUnresolved = proxyquire.throwOnUnresolved()
proxyquire.throwOnUnresolved()

The return value is the main fn itself—assigning the return value implies a new copy of proxyquire which isn't the case here. This test mutates state it doesn't own and I'd rather that be apparent in case it causes future issues.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, thanks for pointing that out! I've been using a generator function most of the time around proxyquire to have a "clean slate" for each test run so forgot it was actually mutating here!


proxyquireThrowOnUnresolved('./samples/foo', {
nonExistent: () => {}
})
}

it('throws an exception with the stub key', function () {
throws(act, /Cannot find module 'nonExistent'/)
})
})
})