Skip to content

Commit

Permalink
module: handle NODE_PATH in require('.')
Browse files Browse the repository at this point in the history
This commit restores the functionality of adding a module's path to
NODE_PATH and requiring it with require('.'). As NODE_PATH was never
intended to be used as a pointer to a module directory (but instead, to
a directory containing directories of modules), this feature is also
being deprecated in turn, to be removed at a later point in time.

PR-URL: #1363
Fixes: #1356
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
silverwind committed Apr 16, 2015
1 parent 431673e commit 3ad82c3
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
20 changes: 19 additions & 1 deletion lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ function tryExtensions(p, exts) {
}


const noopDeprecateRequireDot = util.deprecate(function() {},
"warning: require('.') resolved outside the package directory. " +
"This functionality is deprecated and will be removed soon.");

This comment has been minimized.

Copy link
@Fishrock123

Fishrock123 Apr 17, 2015

Contributor

The linter doesn't like this.. do we normally \' escape single quotes or something?

This comment has been minimized.

Copy link
@silverwind

silverwind Apr 17, 2015

Author Contributor

Whoops, I thought that style was okay. Yeah, I think \' is predominant, or a template string.

This comment has been minimized.

Copy link
@silverwind

silverwind Apr 17, 2015

Author Contributor

Style PR coming up shortly.

This comment has been minimized.

Copy link
@rlidwka

rlidwka Apr 17, 2015

Contributor

Maybe change the linter settings? Using double quotes to avoid escaping is a widespread practice in javascript.

This comment has been minimized.

Copy link
@silverwind

silverwind Apr 17, 2015

Author Contributor

@rlidwka the linter already allows this in a single string. It won't allow "''" + "" (It fails on the second string).

This comment has been minimized.

Copy link
@rlidwka

rlidwka Apr 17, 2015

Contributor

Oh I see, ideally only the second string should be single quoted then.

Doesn't matter very much though.

This comment has been minimized.

Copy link
@silverwind

silverwind Apr 17, 2015

Author Contributor

Yeah, I thought that would look odd, so I escaped it.



Module._findPath = function(request, paths) {
var exts = Object.keys(Module._extensions);

Expand Down Expand Up @@ -169,6 +174,8 @@ Module._findPath = function(request, paths) {
}

if (filename) {
// Warn once if '.' resolved outside the module dir
if (request === '.' && i > 0) noopDeprecateRequireDot();
Module._pathCache[cacheKey] = filename;
return filename;
}
Expand Down Expand Up @@ -205,12 +212,23 @@ Module._resolveLookupPaths = function(request, parent) {
}

var start = request.substring(0, 2);
if (start !== '.' && start !== './' && start !== '..') {
if (start !== './' && start !== '..') {
var paths = modulePaths;
if (parent) {
if (!parent.paths) parent.paths = [];
paths = parent.paths.concat(paths);
}

// Maintain backwards compat with certain broken uses of require('.')
// by putting the module's directory in front of the lookup paths.
if (request === '.') {
if (parent && parent.filename) {
paths.splice(0, 0, path.dirname(parent.filename));
} else {
paths.splice(0, 0, path.resolve(request));
}
}

return [request, paths];
}

Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-require-dot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
var common = require('../common');
var assert = require('assert');
var module = require('module');

var a = require(common.fixturesDir + '/module-require/relative/dot.js');
var b = require(common.fixturesDir + '/module-require/relative/dot-slash.js');

assert.equal(a.value, 42);
assert.equal(a, b, 'require(".") should resolve like require("./")');

process.env.NODE_PATH = common.fixturesDir + '/module-require/relative';
module._initPaths();

var c = require('.');

assert.equal(c.value, 42, 'require(".") should honor NODE_PATH');
8 changes: 1 addition & 7 deletions test/parallel/test-require-extensions-main.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
var common = require('../common');
var assert = require('assert');

require(common.fixturesDir + '/require-bin/bin/req.js');

var a = require(common.fixturesDir + '/module-require/relative/dot.js');
var b = require(common.fixturesDir + '/module-require/relative/dot-slash.js');

assert.equal(a.value, 42);
assert.equal(a, b, 'require(".") should resolve like require("./")');
require(common.fixturesDir + '/require-bin/bin/req.js');

0 comments on commit 3ad82c3

Please sign in to comment.