From 5e5411a3121c8737b515440b2275ff7ff1136136 Mon Sep 17 00:00:00 2001 From: Jason Walton Date: Wed, 28 Feb 2018 23:20:13 -0500 Subject: [PATCH 1/3] Make it so relative proxyquired paths are relative to the module being required. --- lib/proxyquire.js | 57 +++++++++++++++++++++----- package.json | 1 + test/proxyquire-relative-paths.js | 13 ++++++ test/samples/relative-paths/a/index.js | 4 ++ test/samples/relative-paths/a/util.js | 1 + test/samples/relative-paths/b/index.js | 1 + test/samples/relative-paths/b/util.js | 1 + 7 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 test/proxyquire-relative-paths.js create mode 100644 test/samples/relative-paths/a/index.js create mode 100644 test/samples/relative-paths/a/util.js create mode 100644 test/samples/relative-paths/b/index.js create mode 100644 test/samples/relative-paths/b/util.js diff --git a/lib/proxyquire.js b/lib/proxyquire.js index f1ac608..51fc1d0 100644 --- a/lib/proxyquire.js +++ b/lib/proxyquire.js @@ -4,6 +4,7 @@ var Module = require('module') var resolve = require('resolve') var dirname = require('path').dirname +var pathResolve = require('path').resolve var ProxyquireError = require('./proxyquire-error') var is = require('./is') var assert = require('assert') @@ -129,17 +130,45 @@ Proxyquire.prototype.load = function (request, stubs) { return this._withoutCache(this._parent, stubs, request, this._parent.require.bind(this._parent, request)) } +// Resolves a stub relative to a module. Note that this only resolves +// "relative" paths (e.g. "./foo") but not non-relative modules (e.g. "react"). +// Also, if noPreserveCache(), then we won't throw if the module can't +// be resolved. +Proxyquire.prototype._resolveRelativePath = function (path, moduleFilename) { + try { + if (path[0] === '.') { + return resolve.sync(path, { + basedir: dirname(moduleFilename), + extensions: Object.keys(require.extensions), + paths: Module.globalPaths + }) + } else { + return path + } + } catch (err) { + if (this._preserveCache) { + throw err + } + + // If !this._preserveCache, then we don't want to throw, since we can + // resolve modules that don't exist. Resolve as best we can. + return pathResolve(dirname(moduleFilename), path) + } +} + // This replaces a module's require function -Proxyquire.prototype._require = function (module, stubs, path) { +Proxyquire.prototype._require = function (module, resolvedStubs, path) { assert(typeof path === 'string', 'path must be a string') assert(path, 'missing path') - if (hasOwnProperty.call(stubs, path)) { - var stub = stubs[path] + var resolvedPath = this._resolveRelativePath(path, module.filename) + + if (hasOwnProperty.call(resolvedStubs, resolvedPath)) { + var stub = resolvedStubs[resolvedPath] if (stub === null) { // Mimic the module-not-found exception thrown by node.js. - throw moduleNotFoundError(path) + throw moduleNotFoundError(resolvedPath) } if (hasOwnProperty.call(stub, '@noCallThru') ? !stub['@noCallThru'] : !this._noCallThru) { @@ -154,7 +183,7 @@ Proxyquire.prototype._require = function (module, stubs, path) { // Only ignore the cache if we have global stubs if (this._containsRuntimeGlobal) { - return this._withoutCache(module, stubs, path, Module._load.bind(Module, path, module)) + return this._withoutCache(module, resolvedStubs, path, Module._load.bind(Module, path, module)) } else { return Module._load(path, module) } @@ -163,9 +192,16 @@ Proxyquire.prototype._require = function (module, stubs, path) { Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { // Temporarily disable the cache - either per-module or globally if we have global stubs var restoreCache = this._disableCache(module, path) + var resolvedPath = Module._resolveFilename(path, module) + var self = this + var resolvedStubs = {} + Object.keys(stubs).forEach(function (stubPath) { + var resolvedStubPath = self._resolveRelativePath(stubPath, resolvedPath) + resolvedStubs[resolvedStubPath] = stubs[stubPath] + }) // Override all require extension handlers - var restoreExtensionHandlers = this._overrideExtensionHandlers(module, stubs) + var restoreExtensionHandlers = this._overrideExtensionHandlers(module, resolvedStubs) try { // Execute the function that needs the module cache disabled @@ -175,17 +211,16 @@ Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { if (this._preserveCache) { restoreCache() } else { - var id = Module._resolveFilename(path, module) var stubIds = Object.keys(stubs).map(function (stubPath) { try { return resolve.sync(stubPath, { - basedir: dirname(id), + basedir: dirname(resolvedPath), extensions: Object.keys(require.extensions), paths: Module.globalPaths }) } catch (_) {} }) - var ids = [id].concat(stubIds.filter(Boolean)) + var ids = [resolvedPath].concat(stubIds.filter(Boolean)) ids.forEach(function (id) { delete require.cache[id] @@ -251,7 +286,7 @@ Proxyquire.prototype._disableModuleCache = function (path, module) { } } -Proxyquire.prototype._overrideExtensionHandlers = function (module, stubs) { +Proxyquire.prototype._overrideExtensionHandlers = function (module, resolvedStubs) { /* eslint node/no-deprecated-api: [error, {ignoreGlobalItems: ["require.extensions"]}] */ var originalExtensions = {} @@ -266,7 +301,7 @@ Proxyquire.prototype._overrideExtensionHandlers = function (module, stubs) { // Override the default handler for the requested file extension require.extensions[extension] = function (module, filename) { // Override the require method for this module - module.require = self._require.bind(self, module, stubs) + module.require = self._require.bind(self, module, resolvedStubs) return originalExtensions[extension](module, filename) } diff --git a/package.json b/package.json index 8817be1..5351cc5 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "author": "Thorsten Lorenz", "license": "MIT", "devDependencies": { + "jshint": "^2.9.5", "mocha": "~3.1", "native-hello-world": "^1.0.0", "should": "~3.3", diff --git a/test/proxyquire-relative-paths.js b/test/proxyquire-relative-paths.js new file mode 100644 index 0000000..9abab64 --- /dev/null +++ b/test/proxyquire-relative-paths.js @@ -0,0 +1,13 @@ +'use strict' + +/* jshint asi:true */ +/* global describe, it */ + +var proxyquire = require('..') + +describe('When requiring relative paths, they should be relative to the proxyrequired module', function () { + it('should return the correct result', function () { + var result = proxyquire('./samples/relative-paths/a/index.js', {'./util': {c: 'c'}}) + result.should.eql({a: 'a', c: 'c'}) + }) +}) diff --git a/test/samples/relative-paths/a/index.js b/test/samples/relative-paths/a/index.js new file mode 100644 index 0000000..1532193 --- /dev/null +++ b/test/samples/relative-paths/a/index.js @@ -0,0 +1,4 @@ +var util = require('./util') +require('../b') + +module.exports = util diff --git a/test/samples/relative-paths/a/util.js b/test/samples/relative-paths/a/util.js new file mode 100644 index 0000000..9852960 --- /dev/null +++ b/test/samples/relative-paths/a/util.js @@ -0,0 +1 @@ +exports.a = 'a' diff --git a/test/samples/relative-paths/b/index.js b/test/samples/relative-paths/b/index.js new file mode 100644 index 0000000..5f8ab40 --- /dev/null +++ b/test/samples/relative-paths/b/index.js @@ -0,0 +1 @@ +require('./util') diff --git a/test/samples/relative-paths/b/util.js b/test/samples/relative-paths/b/util.js new file mode 100644 index 0000000..378ac76 --- /dev/null +++ b/test/samples/relative-paths/b/util.js @@ -0,0 +1 @@ +exports.b = 'b' From cbceb8fcd612102b600262af523ecf6f28bf8e6f Mon Sep 17 00:00:00 2001 From: Jason Walton Date: Thu, 1 Mar 2018 12:56:00 -0500 Subject: [PATCH 2/3] Fixes from code inspect. --- lib/proxyquire.js | 61 ++++++++++++++++++++++------------------------- package.json | 1 - 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/lib/proxyquire.js b/lib/proxyquire.js index 51fc1d0..8d903e8 100644 --- a/lib/proxyquire.js +++ b/lib/proxyquire.js @@ -136,35 +136,37 @@ Proxyquire.prototype.load = function (request, stubs) { // be resolved. Proxyquire.prototype._resolveRelativePath = function (path, moduleFilename) { try { - if (path[0] === '.') { - return resolve.sync(path, { - basedir: dirname(moduleFilename), - extensions: Object.keys(require.extensions), - paths: Module.globalPaths - }) - } else { - return path - } + return resolve.sync(path, { + basedir: dirname(moduleFilename), + extensions: Object.keys(require.extensions), + paths: Module.globalPaths + }) } catch (err) { - if (this._preserveCache) { - throw err + // 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. + if (path[0] !== '.') { + return path } // If !this._preserveCache, then we don't want to throw, since we can // resolve modules that don't exist. Resolve as best we can. - return pathResolve(dirname(moduleFilename), path) + if (!this._preserveCache) { + return pathResolve(dirname(moduleFilename), path) + } + + throw err } } // This replaces a module's require function -Proxyquire.prototype._require = function (module, resolvedStubs, path) { +Proxyquire.prototype._require = function (module, stubs, path) { assert(typeof path === 'string', 'path must be a string') assert(path, 'missing path') var resolvedPath = this._resolveRelativePath(path, module.filename) - if (hasOwnProperty.call(resolvedStubs, resolvedPath)) { - var stub = resolvedStubs[resolvedPath] + if (hasOwnProperty.call(stubs, resolvedPath)) { + var stub = stubs[resolvedPath] if (stub === null) { // Mimic the module-not-found exception thrown by node.js. @@ -183,7 +185,7 @@ Proxyquire.prototype._require = function (module, resolvedStubs, path) { // Only ignore the cache if we have global stubs if (this._containsRuntimeGlobal) { - return this._withoutCache(module, resolvedStubs, path, Module._load.bind(Module, path, module)) + return this._withoutCache(module, stubs, path, Module._load.bind(Module, path, module)) } else { return Module._load(path, module) } @@ -193,15 +195,17 @@ Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { // Temporarily disable the cache - either per-module or globally if we have global stubs var restoreCache = this._disableCache(module, path) var resolvedPath = Module._resolveFilename(path, module) - var self = this - var resolvedStubs = {} - Object.keys(stubs).forEach(function (stubPath) { - var resolvedStubPath = self._resolveRelativePath(stubPath, resolvedPath) - resolvedStubs[resolvedStubPath] = stubs[stubPath] - }) + // Resolve all stubs to absolute paths. + stubs = Object.keys(stubs) + .reduce(function (result, stubPath) { + var resolvedStubPath = this._resolveRelativePath(stubPath, resolvedPath) + result[resolvedStubPath] = stubs[stubPath] + return result + }.bind(this), {}) + // Override all require extension handlers - var restoreExtensionHandlers = this._overrideExtensionHandlers(module, resolvedStubs) + var restoreExtensionHandlers = this._overrideExtensionHandlers(module, stubs) try { // Execute the function that needs the module cache disabled @@ -211,17 +215,8 @@ Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { if (this._preserveCache) { restoreCache() } else { - var stubIds = Object.keys(stubs).map(function (stubPath) { - try { - return resolve.sync(stubPath, { - basedir: dirname(resolvedPath), - extensions: Object.keys(require.extensions), - paths: Module.globalPaths - }) - } catch (_) {} - }) + var stubIds = Object.keys(stubs) var ids = [resolvedPath].concat(stubIds.filter(Boolean)) - ids.forEach(function (id) { delete require.cache[id] }) diff --git a/package.json b/package.json index 5351cc5..8817be1 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,6 @@ "author": "Thorsten Lorenz", "license": "MIT", "devDependencies": { - "jshint": "^2.9.5", "mocha": "~3.1", "native-hello-world": "^1.0.0", "should": "~3.3", From 64cfcc669d880ffce6a66482af7098ddecd430c8 Mon Sep 17 00:00:00 2001 From: Jason Walton Date: Thu, 1 Mar 2018 21:42:52 -0500 Subject: [PATCH 3/3] More changes from code inspect. --- lib/proxyquire.js | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/proxyquire.js b/lib/proxyquire.js index 8d903e8..ba4e43f 100644 --- a/lib/proxyquire.js +++ b/lib/proxyquire.js @@ -2,9 +2,8 @@ /* jshint laxbreak:true, loopfunc:true */ var Module = require('module') +var path = require('path') var resolve = require('resolve') -var dirname = require('path').dirname -var pathResolve = require('path').resolve var ProxyquireError = require('./proxyquire-error') var is = require('./is') var assert = require('assert') @@ -130,28 +129,33 @@ Proxyquire.prototype.load = function (request, stubs) { return this._withoutCache(this._parent, stubs, request, this._parent.require.bind(this._parent, request)) } -// Resolves a stub relative to a module. Note that this only resolves -// "relative" paths (e.g. "./foo") but not non-relative modules (e.g. "react"). -// Also, if noPreserveCache(), then we won't throw if the module can't -// be resolved. -Proxyquire.prototype._resolveRelativePath = function (path, moduleFilename) { +// Resolves a stub relative to a module. +// `baseModule` is the module we're resolving from. `pathToResolve` is the +// module we want to resolve (i.e. the string passed to `require()`). +Proxyquire.prototype._resolveModule = function (baseModule, pathToResolve) { try { - return resolve.sync(path, { - basedir: dirname(moduleFilename), + return resolve.sync(pathToResolve, { + basedir: path.dirname(baseModule), extensions: Object.keys(require.extensions), paths: Module.globalPaths }) } catch (err) { // 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. - if (path[0] !== '.') { - return path + // It's safe to do this, because if two different modules require "foo", + // they both expect to get back the same thing. + if (pathToResolve[0] !== '.') { + return pathToResolve } - // If !this._preserveCache, then we don't want to throw, since we can - // resolve modules that don't exist. Resolve as best we can. + // If `pathToResolve` is relative, then it is *not* safe to return it, + // since a file in one directory that requires "./foo" expects to get + // back a different module than one that requires "./foo" from another + // directory. However, if !this._preserveCache, then we don't want to + // throw, since we can resolve modules that don't exist. Resolve as + // best we can. if (!this._preserveCache) { - return pathResolve(dirname(moduleFilename), path) + return path.resolve(path.dirname(baseModule), pathToResolve) } throw err @@ -163,14 +167,13 @@ Proxyquire.prototype._require = function (module, stubs, path) { assert(typeof path === 'string', 'path must be a string') assert(path, 'missing path') - var resolvedPath = this._resolveRelativePath(path, module.filename) - + var resolvedPath = this._resolveModule(module.filename, path) if (hasOwnProperty.call(stubs, resolvedPath)) { var stub = stubs[resolvedPath] if (stub === null) { // Mimic the module-not-found exception thrown by node.js. - throw moduleNotFoundError(resolvedPath) + throw moduleNotFoundError(path) } if (hasOwnProperty.call(stub, '@noCallThru') ? !stub['@noCallThru'] : !this._noCallThru) { @@ -199,7 +202,7 @@ Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { // Resolve all stubs to absolute paths. stubs = Object.keys(stubs) .reduce(function (result, stubPath) { - var resolvedStubPath = this._resolveRelativePath(stubPath, resolvedPath) + var resolvedStubPath = this._resolveModule(resolvedPath, stubPath) result[resolvedStubPath] = stubs[stubPath] return result }.bind(this), {}) @@ -215,8 +218,7 @@ Proxyquire.prototype._withoutCache = function (module, stubs, path, func) { if (this._preserveCache) { restoreCache() } else { - var stubIds = Object.keys(stubs) - var ids = [resolvedPath].concat(stubIds.filter(Boolean)) + var ids = [resolvedPath].concat(Object.keys(stubs).filter(Boolean)) ids.forEach(function (id) { delete require.cache[id] })