From 1ed36aeb53266f8f01e1cd442069024d258f632a Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 24 Aug 2017 09:59:52 -0500 Subject: [PATCH] module: check file ext before dir as documented The documented resolution algorithm started to search for package.json files prior to searching for file extensions when searching for a specifier. Oddly, it did not search for index files at same time it searched for package.json. This restores the documented behavior of searching for file extensions prior to searching directories. PR-URL: https://github.com/nodejs/node/pull/15015 Fixes: https://github.com/nodejs/node/issues/14990 Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Khaidi Chu --- lib/module.js | 15 +++++----- .../module-extension-over-directory/inner.js | 1 + .../inner/package.json | 3 ++ .../test-require-extension-over-directory.js | 30 +++++++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/module-extension-over-directory/inner.js create mode 100644 test/fixtures/module-extension-over-directory/inner/package.json create mode 100644 test/parallel/test-require-extension-over-directory.js diff --git a/lib/module.js b/lib/module.js index 468619f2bf2..6d4e3dbfcf0 100644 --- a/lib/module.js +++ b/lib/module.js @@ -219,6 +219,9 @@ Module._findPath = function(request, paths, isMain) { var exts; var trailingSlash = request.length > 0 && request.charCodeAt(request.length - 1) === CHAR_FORWARD_SLASH; + if (!trailingSlash) { + trailingSlash = /(?:^|\/)\.?\.$/.test(request); + } // For each path for (var i = 0; i < paths.length; i++) { @@ -236,10 +239,6 @@ Module._findPath = function(request, paths, isMain) { } else { filename = toRealPath(basePath); } - } else if (rc === 1) { // Directory. - if (exts === undefined) - exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts, isMain); } if (!filename) { @@ -251,11 +250,13 @@ Module._findPath = function(request, paths, isMain) { } if (!filename && rc === 1) { // Directory. + // try it with each of the extensions at "index" if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts, isMain) || - // try it with each of the extensions at "index" - tryExtensions(path.resolve(basePath, 'index'), exts, isMain); + filename = tryPackage(basePath, exts, isMain); + if (!filename) { + filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain); + } } if (filename) { diff --git a/test/fixtures/module-extension-over-directory/inner.js b/test/fixtures/module-extension-over-directory/inner.js new file mode 100644 index 00000000000..f053ebf7976 --- /dev/null +++ b/test/fixtures/module-extension-over-directory/inner.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/test/fixtures/module-extension-over-directory/inner/package.json b/test/fixtures/module-extension-over-directory/inner/package.json new file mode 100644 index 00000000000..2ac72712f06 --- /dev/null +++ b/test/fixtures/module-extension-over-directory/inner/package.json @@ -0,0 +1,3 @@ +{ + "main": "./package.json" +} diff --git a/test/parallel/test-require-extension-over-directory.js b/test/parallel/test-require-extension-over-directory.js new file mode 100644 index 00000000000..cc896d7b61b --- /dev/null +++ b/test/parallel/test-require-extension-over-directory.js @@ -0,0 +1,30 @@ +'use strict'; +// fixes regression from v4 +require('../common'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const path = require('path'); + +const fixturesRequire = require( + fixtures.path('module-extension-over-directory', 'inner')); + +assert.strictEqual( + fixturesRequire, + require(fixtures.path('module-extension-over-directory', 'inner.js')), + 'test-require-extension-over-directory failed to import fixture' + + ' requirements' +); + +const fakePath = [ + fixtures.path('module-extension-over-directory', 'inner'), + 'fake', + '..' +].join(path.sep); +const fixturesRequireDir = require(fakePath); + +assert.strictEqual( + fixturesRequireDir, + require(fixtures.path('module-extension-over-directory', 'inner/')), + 'test-require-extension-over-directory failed to import fixture' + + ' requirements' +);