Skip to content

Commit

Permalink
module: check file ext before dir as documented
Browse files Browse the repository at this point in the history
The documented resolution algorithm was regressed and 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 reverts that
regression and restores the documented behavior of searching for
file extensions prior to searching directories.

Fixes: nodejs#14990
PR-URL: nodejs#15015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
bmeck authored and BridgeAR committed Feb 17, 2018
1 parent 94e8d2a commit 9a43f20
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
16 changes: 9 additions & 7 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,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++) {
Expand All @@ -232,10 +235,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) {
Expand All @@ -244,14 +243,17 @@ Module._findPath = function(request, paths, isMain) {
exts = Object.keys(Module._extensions);
filename = tryExtensions(basePath, exts, 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) {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/module-extension-over-directory/inner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"main": "./package.json"
}
26 changes: 26 additions & 0 deletions test/parallel/test-require-extension-over-directory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'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'
);
4 changes: 2 additions & 2 deletions test/parallel/test-require-extensions-same-filename-as-dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ const content = require(fixtures.path('json-with-directory-name-module',
'module-stub', 'one', 'two',
'three.js'));

assert.notStrictEqual(content.rocko, 'artischocko');
assert.strictEqual(content, 'hello from module-stub!');
assert.strictEqual(content.rocko, 'artischocko');
assert.notStrictEqual(content, 'hello from module-stub!');

0 comments on commit 9a43f20

Please sign in to comment.