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.
---
 cli/tests/unit_node/module_test.ts | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

(limited to 'cli/tests/unit_node/module_test.ts')

diff --git a/cli/tests/unit_node/module_test.ts b/cli/tests/unit_node/module_test.ts
index a5c819d96..3a675c7a1 100644
--- a/cli/tests/unit_node/module_test.ts
+++ b/cli/tests/unit_node/module_test.ts
@@ -1,8 +1,12 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
 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 * as path from "node:path";
 
 Deno.test("[node/module _preloadModules] has internal require hook", () => {
   // 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
   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",
+  );
+});
-- 
cgit v1.2.3