From 86081052089edbcc2ef2a76ea950ae58f46066b3 Mon Sep 17 00:00:00 2001
From: Marvin Hagemeister <hello@marvinh.dev>
Date: Tue, 23 May 2023 12:46:14 +0200
Subject: 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.
---
 ext/node/ops/require.rs          | 11 +++--------
 ext/node/polyfills/01_require.js |  5 +++++
 2 files changed, 8 insertions(+), 8 deletions(-)

(limited to 'ext')

diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs
index 9e13681ae..eb092ab86 100644
--- a/ext/node/ops/require.rs
+++ b/ext/node/ops/require.rs
@@ -128,16 +128,11 @@ where
   let mut current_path = from.as_path();
   let mut maybe_parent = Some(current_path);
   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());
-      current_path = parent;
-      maybe_parent = current_path.parent();
     }
-  }
-
-  if !cfg!(windows) {
-    // Append /node_modules to handle root paths.
-    paths.push("/node_modules".to_string());
+    current_path = parent;
+    maybe_parent = current_path.parent();
   }
 
   Ok(paths)
diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js
index 394015e31..508a32e12 100644
--- a/ext/node/polyfills/01_require.js
+++ b/ext/node/polyfills/01_require.js
@@ -607,6 +607,11 @@ Module._findPath = function (request, paths, isMain, parentPath) {
   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) {
   return ops.op_require_node_module_paths(fromPath);
 };
-- 
cgit v1.2.3