mirror of
https://github.com/denoland/deno.git
synced 2024-12-24 08:09:08 -05:00
fix(node): duplicate node_module suffixes (#19222)
Noticed that we're checking more module paths than necessary. In particular the module path array contains a couple of entries with a duplicated `node_modules/node_modules` suffix. ```js [ // ... more entries before here, where some also contain duplicate suffixes "/Users/marvinhagemeister/dev/preact-render-to-string/node_modules/.deno/node_modules", "/Users/marvinhagemeister/dev/preact-render-to-string/node_modules/node_modules", // <-- duplicate suffix "/Users/marvinhagemeister/dev/preact-render-to-string/node_modules", "/Users/marvinhagemeister/dev/node_modules", "/Users/marvinhagemeister/node_modules", "/Users/node_modules", "/node_modules", "/node_modules" // <-- duplicate entry ] ``` This was caused by a misunderstanding in how Rust's [`Path::ends_with()`](https://doc.rust-lang.org/std/path/struct.Path.html#method.ends_with) works. It's designed to match on whole path segments and the suffix `/node_modules` is not that, except for the root entry. This meant that our check for if the path already ended with `node_module` always returned `false`. Removing the leading slash fixes that. While we're at it, we can remove the last condition where we explicitly added the root `/node_modules` entry since the while loop prior to that takes care of it already.
This commit is contained in:
parent
5874fc3d0a
commit
8608105208
3 changed files with 42 additions and 9 deletions
|
@ -1,8 +1,12 @@
|
||||||
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
|
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
|
||||||
|
|
||||||
import { Module } from "node:module";
|
import { Module } from "node:module";
|
||||||
import { assertEquals } from "../../../test_util/std/testing/asserts.ts";
|
import {
|
||||||
|
assert,
|
||||||
|
assertEquals,
|
||||||
|
} from "../../../test_util/std/testing/asserts.ts";
|
||||||
import process from "node:process";
|
import process from "node:process";
|
||||||
|
import * as path from "node:path";
|
||||||
|
|
||||||
Deno.test("[node/module _preloadModules] has internal require hook", () => {
|
Deno.test("[node/module _preloadModules] has internal require hook", () => {
|
||||||
// Check if it's there
|
// Check if it's there
|
||||||
|
@ -25,3 +29,32 @@ Deno.test("[node/module runMain] loads module using the current process.argv", (
|
||||||
// deno-lint-ignore no-explicit-any
|
// deno-lint-ignore no-explicit-any
|
||||||
assertEquals((globalThis as any).calledViaRunMain, true);
|
assertEquals((globalThis as any).calledViaRunMain, true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
Deno.test("[node/module _nodeModulePaths] prevents duplicate /node_modules/node_modules suffix", () => {
|
||||||
|
// deno-lint-ignore no-explicit-any
|
||||||
|
const actual: string[] = (Module as any)._nodeModulePaths(
|
||||||
|
path.join(process.cwd(), "testdata", "node_modules", "foo"),
|
||||||
|
);
|
||||||
|
|
||||||
|
assert(
|
||||||
|
!actual.some((dir) => /node_modules[/\\]node_modules/g.test(dir)),
|
||||||
|
"Duplicate 'node_modules/node_modules' suffix found",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
Deno.test("[node/module _nodeModulePaths] prevents duplicate root /node_modules", () => {
|
||||||
|
// deno-lint-ignore no-explicit-any
|
||||||
|
const actual: string[] = (Module as any)._nodeModulePaths(
|
||||||
|
path.join(process.cwd(), "testdata", "node_modules", "foo"),
|
||||||
|
);
|
||||||
|
|
||||||
|
assert(
|
||||||
|
new Set(actual).size === actual.length,
|
||||||
|
"Duplicate path entries found",
|
||||||
|
);
|
||||||
|
const root = path.parse(actual[0]).root;
|
||||||
|
assert(
|
||||||
|
actual.includes(path.join(root, "node_modules")),
|
||||||
|
"Missing root 'node_modules' directory",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
|
@ -128,16 +128,11 @@ where
|
||||||
let mut current_path = from.as_path();
|
let mut current_path = from.as_path();
|
||||||
let mut maybe_parent = Some(current_path);
|
let mut maybe_parent = Some(current_path);
|
||||||
while let Some(parent) = maybe_parent {
|
while let Some(parent) = maybe_parent {
|
||||||
if !parent.ends_with("/node_modules") {
|
if !parent.ends_with("node_modules") {
|
||||||
paths.push(parent.join("node_modules").to_string_lossy().to_string());
|
paths.push(parent.join("node_modules").to_string_lossy().to_string());
|
||||||
current_path = parent;
|
|
||||||
maybe_parent = current_path.parent();
|
|
||||||
}
|
}
|
||||||
}
|
current_path = parent;
|
||||||
|
maybe_parent = current_path.parent();
|
||||||
if !cfg!(windows) {
|
|
||||||
// Append /node_modules to handle root paths.
|
|
||||||
paths.push("/node_modules".to_string());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(paths)
|
Ok(paths)
|
||||||
|
|
|
@ -607,6 +607,11 @@ Module._findPath = function (request, paths, isMain, parentPath) {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get a list of potential module directories
|
||||||
|
* @param {string} fromPath The directory name of the module
|
||||||
|
* @returns {string[]} List of module directories
|
||||||
|
*/
|
||||||
Module._nodeModulePaths = function (fromPath) {
|
Module._nodeModulePaths = function (fromPath) {
|
||||||
return ops.op_require_node_module_paths(fromPath);
|
return ops.op_require_node_module_paths(fromPath);
|
||||||
};
|
};
|
||||||
|
|
Loading…
Reference in a new issue