Skip to content

Commit

Permalink
removed support for implicit local modules
Browse files Browse the repository at this point in the history
`require.resolve` behavior was changed in Node v12 to comply with Node
resolution algorithm (see nodejs/node#29139) -
not sure why we thought this behavior was desirable in the first place
(introduced in 3142f2e - 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
  • Loading branch information
FND committed Aug 16, 2019
1 parent 75555fb commit dc5e207
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
language: node_js
node_js:
- 8.9.3
- 8
- 10

Expand Down
4 changes: 2 additions & 2 deletions lib/util/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand All @@ -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();
}

Expand Down
30 changes: 21 additions & 9 deletions test/test_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,44 @@ 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");
assertSame(path.relative(root, filepath), "dummy/src.js");
if(modern) {
assert.throws(() => {
resolvePath("dummy/src.js");
}, /exit 1/);
}

filepath = resolvePath("dummy/pkg.js");
let filepath = resolvePath("dummy/pkg.js");
assertSame(path.relative(root, filepath), "node_modules/dummy/pkg.js");

// 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");
});
if(modern) {
["dummy", "dummy/index", "dummy/index.js"].forEach(module => {
let filepath = resolvePath(module);
assertSame(path.relative(root, filepath), "node_modules/dummy/index.js");
});
}
});
});

0 comments on commit dc5e207

Please sign in to comment.