From 0b76a6465e13f18e5061ff07b23368aab185bc55 Mon Sep 17 00:00:00 2001 From: FND Date: Fri, 2 Aug 2019 11:26:05 +0200 Subject: [PATCH] removed support for implicit local modules `require.resolve` behavior was changed in Node v12 to comply with Node resolution algorithm (see https://github.com/nodejs/node/issues/29139) - not sure why we thought this behavior was desirable in the first place (introduced in 3142f2eb8c3b32bb78ef83b177f613637493d3a6 - probably cargo-culting `require.resolve`'s observed behavior) however, on legacy Node versions behavior appears unchanged if the current working directory is the same as `rootDir` - I didn't bother enforcing modern behavior because legacy isn't worth the effort in the process, fixed legacy detection --- .travis.yml | 1 + lib/util/resolve.js | 4 ++-- test/test_manager.js | 31 ++++++++++++++++++++++--------- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index a87a14c..fe93b05 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ language: node_js node_js: +- 8.9.3 - 8 - 10 diff --git a/lib/util/resolve.js b/lib/util/resolve.js index 1bc74df..a31d0fa 100644 --- a/lib/util/resolve.js +++ b/lib/util/resolve.js @@ -9,7 +9,7 @@ if(!legacy) { // account for bug in Node v8.9.4 and below let { version } = process; if(version.substr(0, 3) === "v8.") { let [minor, patch] = version.substr(3).split(".").map(i => parseInt(i, 10)); - legacy = minor < 8 || patch <= 4; + legacy = minor <= 9 && patch <= 4; } } @@ -31,7 +31,7 @@ function resolveModulePath(filepath, rootDir) { if(legacy) { legacy = process.env.NODE_PATH; // cache previous value rootDir = rootDir.replace(/\/{1,}$/, ""); // strip trailing slashes, to be safe - process.env.NODE_PATH = `${rootDir}:${rootDir}/node_modules`; + process.env.NODE_PATH = `${rootDir}/node_modules`; require("module").Module._initPaths(); } diff --git a/test/test_manager.js b/test/test_manager.js index 74c67ff..cd243a4 100644 --- a/test/test_manager.js +++ b/test/test_manager.js @@ -7,32 +7,45 @@ let assert = require("assert"); let assertSame = assert.strictEqual; +let [major] = process.version.substr(1).split(".").map(i => parseInt(i, 10)); +let MODERN = major >= 12; // legacy doesn't throw due to working directory + describe("asset manager", _ => { let root = path.resolve(__dirname, "fixtures"); let cwd; + let { exit } = process; before(() => { cwd = process.cwd(); process.chdir(root); + process.exit = code => { + throw new Error(`exit ${code}`); + }; }); after(() => { process.chdir(cwd); + process.exit = exit; }); - it("resolves file paths for local modules and third-party packages", () => { + it("resolves file paths for third-party packages", () => { let { resolvePath } = new AssetManager(root); - let filepath = resolvePath("dummy/src.js"); + let filepath = resolvePath("dummy/pkg.js"); + assertSame(path.relative(root, filepath), "node_modules/dummy/pkg.js"); + + filepath = resolvePath("./dummy/src.js"); assertSame(path.relative(root, filepath), "dummy/src.js"); - filepath = resolvePath("dummy/pkg.js"); - assertSame(path.relative(root, filepath), "node_modules/dummy/pkg.js"); + if(MODERN) { + assert.throws(() => { + resolvePath("dummy/src.js"); + }, /exit 1/); - // local modules take precedence over third-party packages - ["dummy", "dummy/index", "dummy/index.js"].forEach(module => { - let filepath = resolvePath(module); - assertSame(path.relative(root, filepath), "dummy/index.js"); - }); + ["dummy", "dummy/index", "dummy/index.js"].forEach(module => { + let filepath = resolvePath(module); + assertSame(path.relative(root, filepath), "node_modules/dummy/index.js"); + }); + } }); });