thlorenz / proxyquire

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

Slightly confusing first example in readme #264

Closed sohale closed 2 years ago

sohale commented 2 years ago

The first example confused me the first time read it. It left an ambiguity and did not clarify a main difference between rewire and proxyquire when it is read for the first time: Current: (Note that the variable path is the same as the package name, hence, the reader may think the variable is stubbed (as in rewire) -- especially users who are used to rewire.

var path = require('path');

module.exports.extnameAllCaps = function (file) {
  return path.extname(file).toUpperCase();
};

module.exports.basenameAllCaps = function (file) {
  return path.basename(file).toUpperCase();
};

Suggestion: (note variable name is not path)

var p = require('path');

module.exports.extnameAllCaps = function (file) {
  return p.extname(file).toUpperCase();
};

module.exports.basenameAllCaps = function (file) {
  return p.basename(file).toUpperCase();
};

PS. I tried to create a pull request, but could not

▷ git push --set-upstream origin doc-improve-confusing-example

ERROR: Permission to thlorenz/proxyquire.git denied to sohale.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
bendrucker commented 2 years ago

Hey so it's not really proxyquire's obligation to distinguish itself from rewire and demonstrate that it doesn't mutate private scopes. That said, docs PRs are always welcome and if they're obvious improvements or well explained we'll merge.

PRs are branches pushed to your fork and then opened against an upstream. You don't have access to push anything directly here.