From eab836f1c1243554ec000a1d0816210036c88ccb Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 31 Oct 2017 01:51:01 -0400 Subject: [PATCH] module: revert #3384 DEP0019 EOL 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: https://github.com/nodejs/node/pull/3384 PR-URL: https://github.com/nodejs/node/pull/16634 Refs: https://github.com/nodejs/node/pull/3384 Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski Reviewed-By: Colin Ihrig --- doc/api/deprecations.md | 2 +- lib/module.js | 26 +++++++++++++++++++++++++- test/parallel/test-require-dot.js | 6 ++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 10db311e88..8f94f6b248 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -202,7 +202,7 @@ code. ### 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 diff --git a/lib/module.js b/lib/module.js index 921a57a814..e418a1a3e0 100644 --- a/lib/module.js +++ b/lib/module.js @@ -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 = ['']; @@ -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; } @@ -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; @@ -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]); } diff --git a/test/parallel/test-require-dot.js b/test/parallel/test-require-dot.js index 2e8a5c79f6..e2202efec1 100644 --- a/test/parallel/test-require-dot.js +++ b/test/parallel/test-require-dot.js @@ -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');