Skip to content

Commit

Permalink
module: revert #3384 DEP0019 EOL
Browse files Browse the repository at this point in the history
The original commit was landed without running CITGM. Unfortunately
this change breaks the module `d` which has over 500k downloads a day.

It is worth mentioning that the compatibility hack can be removed
without breaking anything.

We should definitely revisit for the next Semver-Major but shipping
this today will cause non trivial ecosystem breakages.

Refs: #3384

PR-URL: #16634
Refs: #3384
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
MylesBorins authored and jasnell committed Oct 31, 2017
1 parent dfcaf28 commit 82790d8
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
2 changes: 1 addition & 1 deletion doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ code.
<a id="DEP0019"></a>
### DEP0019: require('.') resolved outside directory

Type: End-of-Life
Type: Runtime

In certain cases, `require('.')` may resolve outside the package directory.
This behavior is deprecated and will be removed in a future major Node.js
Expand Down
26 changes: 25 additions & 1 deletion lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ function tryExtensions(p, exts, isMain) {
return false;
}

var warned = false;
Module._findPath = function(request, paths, isMain) {
if (path.isAbsolute(request)) {
paths = [''];
Expand Down Expand Up @@ -232,6 +233,18 @@ Module._findPath = function(request, paths, isMain) {
}

if (filename) {
// Warn once if '.' resolved outside the module dir
if (request === '.' && i > 0) {
if (!warned) {
warned = true;
process.emitWarning(
'warning: require(\'.\') resolved outside the package ' +
'directory. This functionality is deprecated and will be removed ' +
'soon.',
'DeprecationWarning', 'DEP0019');
}
}

Module._pathCache[cacheKey] = filename;
return filename;
}
Expand Down Expand Up @@ -334,7 +347,8 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {
}

// Check for relative path
if (request.charCodeAt(0) !== 46/*.*/ &&
if (request.length < 2 ||
request.charCodeAt(0) !== 46/*.*/ ||
(request.charCodeAt(1) !== 46/*.*/ &&
request.charCodeAt(1) !== 47/*/*/)) {
var paths = modulePaths;
Expand All @@ -345,6 +359,16 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {
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.unshift(path.dirname(parent.filename));
} else {
paths.unshift(path.resolve(request));
}
}

debug('looking for %j in %j', request, paths);
return (newReturn ? (paths.length > 0 ? paths : null) : [request, paths]);
}
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-require-dot.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ const b = require(fixtures.path('module-require', 'relative', 'dot-slash.js'));
assert.strictEqual(a.value, 42);
assert.strictEqual(a, b, 'require(".") should resolve like require("./")');

// require('.') should not lookup in NODE_PATH
process.env.NODE_PATH = fixtures.path('module-require', 'relative');
m._initPaths();
assert.throws(() => { require('.'); }, Error, "Cannot find module '.'");

const c = require('.');

assert.strictEqual(c.value, 42, 'require(".") should honor NODE_PATH');

0 comments on commit 82790d8

Please sign in to comment.