From 5b487c9b7315b3a637125a58592d7a325b20ca3d Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Tue, 16 Jan 2018 16:31:49 -0500 Subject: [PATCH 1/5] Failing test for discrepancy with require.resolve --- test/shadowed_core.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/shadowed_core.js diff --git a/test/shadowed_core.js b/test/shadowed_core.js new file mode 100644 index 00000000..84d6bddf --- /dev/null +++ b/test/shadowed_core.js @@ -0,0 +1,21 @@ +var test = require('tape'); +var resolve = require('../'); +var path = require('path'); + +test('shadowed core module names agree with require.resolve()', function (t) { + t.plan(2); + + resolve('util', { basedir: path.join(__dirname, 'shadowed_core') }, function (err, res) { + t.ifError(err); + t.equal(res, 'util'); + }); +}); + +test('shadowed core module names can be accessed with `name/`', function (t) { + t.plan(2); + + resolve('util/', { basedir: path.join(__dirname, 'shadowed_core') }, function (err, res) { + t.ifError(err); + t.equal(res, path.join(__dirname, 'shadowed_core/node_modules/util/index.js')); + }); +}); From 9feebdfc7b5383cac840ffb0622b232b364db1de Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Thu, 18 Jan 2018 11:31:43 -0500 Subject: [PATCH 2/5] Update gitignore so fixture can be committed --- test/shadowed_core/.gitignore | 1 + test/shadowed_core/node_modules/util/index.js | 0 2 files changed, 1 insertion(+) create mode 100644 test/shadowed_core/.gitignore create mode 100644 test/shadowed_core/node_modules/util/index.js diff --git a/test/shadowed_core/.gitignore b/test/shadowed_core/.gitignore new file mode 100644 index 00000000..b337ca4a --- /dev/null +++ b/test/shadowed_core/.gitignore @@ -0,0 +1 @@ +!/node_modules diff --git a/test/shadowed_core/node_modules/util/index.js b/test/shadowed_core/node_modules/util/index.js new file mode 100644 index 00000000..e69de29b From 5686c445a2ddbb6c7b5bdc4c5525870fc189c1b5 Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Thu, 18 Jan 2018 11:36:17 -0500 Subject: [PATCH 3/5] Update test names --- test/shadowed_core.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/shadowed_core.js b/test/shadowed_core.js index 84d6bddf..bcd9b519 100644 --- a/test/shadowed_core.js +++ b/test/shadowed_core.js @@ -2,7 +2,7 @@ var test = require('tape'); var resolve = require('../'); var path = require('path'); -test('shadowed core module names agree with require.resolve()', function (t) { +test('shadowed core modules still return core module', function (t) { t.plan(2); resolve('util', { basedir: path.join(__dirname, 'shadowed_core') }, function (err, res) { @@ -11,7 +11,7 @@ test('shadowed core module names agree with require.resolve()', function (t) { }); }); -test('shadowed core module names can be accessed with `name/`', function (t) { +test('shadowed core modules return shadow when appending `/`', function (t) { t.plan(2); resolve('util/', { basedir: path.join(__dirname, 'shadowed_core') }, function (err, res) { From adb70baa157ae716c8fb82d020ae53de4ee7e989 Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Fri, 23 Mar 2018 15:13:21 -0400 Subject: [PATCH 4/5] =?UTF-8?q?Fixes=20#147=E2=80=94Ensures=20core=20modul?= =?UTF-8?q?es=20beat=20shadowed=20ones?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Given 'util' as an example native module which is also installed as an npm package, then in order for browserify/resolve to act consistently with native `require.resolve`, these should be true: ``` resolve.sync('util') // returns 'util' resolve.sync('util/') // returns 'node_modules/util/index.js' ``` --- lib/async.js | 2 +- lib/sync.js | 2 ++ test/shadowed_core.js | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/async.js b/lib/async.js index dd889dc7..a73b6a1f 100644 --- a/lib/async.js +++ b/lib/async.js @@ -44,8 +44,8 @@ module.exports = function resolve(x, options, callback) { } else loadAsFile(res, opts.package, onfile); } else loadNodeModules(x, basedir, function (err, n, pkg) { if (err) cb(err); - else if (n) cb(null, n, pkg); else if (core[x]) return cb(null, x); + else if (n) return cb(null, n, pkg); else { var moduleError = new Error("Cannot find module '" + x + "' from '" + basedir + "'"); moduleError.code = 'MODULE_NOT_FOUND'; diff --git a/lib/sync.js b/lib/sync.js index 2e0d5f24..97f89b5d 100644 --- a/lib/sync.js +++ b/lib/sync.js @@ -32,6 +32,8 @@ module.exports = function (x, options) { if (x === '..' || x.slice(-1) === '/') res += '/'; var m = loadAsFileSync(res) || loadAsDirectorySync(res); if (m) return m; + } else if (core[x]) { + return x; } else { var n = loadNodeModulesSync(x, basedir); if (n) return n; diff --git a/test/shadowed_core.js b/test/shadowed_core.js index bcd9b519..98c52a76 100644 --- a/test/shadowed_core.js +++ b/test/shadowed_core.js @@ -11,6 +11,14 @@ test('shadowed core modules still return core module', function (t) { }); }); +test('shadowed core modules still return core module [sync]', function (t) { + t.plan(1); + + var res = resolve.sync('util', { basedir: path.join(__dirname, 'shadowed_core') }); + + t.equal(res, 'util'); +}); + test('shadowed core modules return shadow when appending `/`', function (t) { t.plan(2); @@ -19,3 +27,12 @@ test('shadowed core modules return shadow when appending `/`', function (t) { t.equal(res, path.join(__dirname, 'shadowed_core/node_modules/util/index.js')); }); }); + +test('shadowed core modules return shadow when appending `/` [sync]', function (t) { + t.plan(1); + + var res = resolve.sync('util/', { basedir: path.join(__dirname, 'shadowed_core') }); + + t.equal(res, path.join(__dirname, 'shadowed_core/node_modules/util/index.js')); +}); + From 9e3104b85de8b9000fc92800c2b453d9b8fa13f4 Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Fri, 23 Mar 2018 15:18:22 -0400 Subject: [PATCH 5/5] Delete the two tests added for issue #25 These are obviated by the shadowed_core test added in #148 --- test/resolver.js | 11 ----------- test/resolver_sync.js | 11 ----------- 2 files changed, 22 deletions(-) diff --git a/test/resolver.js b/test/resolver.js index b82927b2..116bb242 100644 --- a/test/resolver.js +++ b/test/resolver.js @@ -268,17 +268,6 @@ test('without basedir', function (t) { }); }); -test('#25: node modules with the same name as node stdlib modules', function (t) { - t.plan(1); - - var resolverDir = path.join(__dirname, 'resolver/punycode'); - - resolve('punycode', { basedir: resolverDir }, function (err, res, pkg) { - if (err) t.fail(err); - t.equal(res, path.join(resolverDir, 'node_modules/punycode/index.js')); - }); -}); - test('#52 - incorrectly resolves module-paths like "./someFolder/" when there is a file of the same name', function (t) { t.plan(2); diff --git a/test/resolver_sync.js b/test/resolver_sync.js index 4a4fd477..5b8057fe 100644 --- a/test/resolver_sync.js +++ b/test/resolver_sync.js @@ -168,17 +168,6 @@ test('incorrect main', function (t) { t.end(); }); -test('#25: node modules with the same name as node stdlib modules', function (t) { - var resolverDir = path.join(__dirname, 'resolver/punycode'); - - t.equal( - resolve.sync('punycode', { basedir: resolverDir }), - path.join(resolverDir, 'node_modules/punycode/index.js') - ); - - t.end(); -}); - var stubStatSync = function stubStatSync(fn) { var fs = require('fs'); var statSync = fs.statSync;