Add duplicate package detection to rewatch (#8180)

* Add duplicate package detection to rewatch

* Add changelog

* Warn to stderr

* Set runtime env var

* fmt

Author: nojaf

CHANGELOG.md

@@ -24,6 +24,7 @@
 #### :bug: Bug fix
 
 - Reanalyze: fix reactive/server stale results when cross-file references change without changing dead declarations (non-transitive mode). https://github.com/rescript-lang/rescript/pull/8173
+- Add duplicate package detection to rewatch. https://github.com/rescript-lang/rescript/pull/8180
 
 #### :memo: Documentation
 

rewatch/src/build/packages.rs

@@ -284,6 +284,7 @@ pub fn read_dependency(
 ///    registered for the parent packages. Especially relevant for peerDependencies.
 /// 2. In parallel performs IO to read the dependencies config and
 ///    recursively continues operation for their dependencies as well.
+/// 3. Detects and warns about duplicate packages (same name, different paths).
 fn read_dependencies(
     registered_dependencies_set: &mut AHashSet<String>,
     project_context: &ProjectContext,
@@ -302,6 +303,37 @@ fn read_dependencies(
         .iter()
         .filter_map(|package_name| {
             if registered_dependencies_set.contains(package_name) {
+                // Package already registered - check for duplicate (different path)
+                // Re-resolve from current package and from root to compare paths
+                if let Ok(current_path) = read_dependency(package_name, package_config, project_context)
+                    && let Ok(chosen_path) = read_dependency(package_name, &project_context.current_config, project_context)
+                    && current_path != chosen_path
+                {
+                    // Different paths - this is a duplicate
+                    let root_path = project_context.get_root_path();
+                    let chosen_relative = chosen_path
+                        .strip_prefix(root_path)
+                        .unwrap_or(&chosen_path);
+                    let duplicate_relative = current_path
+                        .strip_prefix(root_path)
+                        .unwrap_or(&current_path);
+                    let current_package_path = package_config
+                        .path
+                        .parent()
+                        .map(|p| p.to_path_buf())
+                        .unwrap_or_else(|| PathBuf::from("."));
+                    let parent_relative = current_package_path
+                        .strip_prefix(root_path)
+                        .unwrap_or(&current_package_path);
+
+                    eprintln!(
+                        "Duplicated package: {} ./{} (chosen) vs ./{} in ./{}",
+                        package_name,
+                        chosen_relative.to_string_lossy(),
+                        duplicate_relative.to_string_lossy(),
+                        parent_relative.to_string_lossy()
+                    );
+                }
                 None
             } else {
                 registered_dependencies_set.insert(package_name.to_owned());
@@ -481,6 +513,7 @@ This inconsistency will cause issues with package resolution.\n",
 fn read_packages(project_context: &ProjectContext, show_progress: bool) -> Result<AHashMap<String, Package>> {
     // Store all packages and completely deduplicate them
     let mut map: AHashMap<String, Package> = AHashMap::new();
+
     let current_package = {
         let config = &project_context.current_config;
         let folder = config
@@ -500,6 +533,7 @@ fn read_packages(project_context: &ProjectContext, show_progress: bool) -> Resul
         show_progress,
         /* is local dep */ true,
     ));
+
     dependencies.iter().for_each(|d| {
         if !map.contains_key(&d.name) {
             let package = make_package(d.config.to_owned(), &d.path, false, d.is_local_dep);

tests/build_tests/duplicated_symlinked_packages/input.js

@@ -1,43 +1,27 @@
 // @ts-check
 
-import * as fs from "node:fs/promises";
+import * as assert from "node:assert";
+import { runtimePath } from "#cli/runtime";
 import { setup } from "#dev/process";
 
-const { execBuildLegacy, execCleanLegacy } = setup(import.meta.dirname);
+// Set runtime path for rewatch to find
+process.env.RESCRIPT_RUNTIME = runtimePath;
 
-const expectedFilePath = "./out.expected";
-
-const updateTests = process.argv[2] === "update";
-
-/**
- * @param {string} output
- * @return {string}
- */
-function postProcessErrorOutput(output) {
-  return output.trimEnd().replace(new RegExp(import.meta.dirname, "gi"), ".");
-}
+const { execBuild, execClean } = setup(import.meta.dirname);
 
 if (process.platform === "win32") {
   console.log("Skipping test on Windows");
   process.exit(0);
 }
 
-await execCleanLegacy();
-const { stderr } = await execBuildLegacy();
+await execClean();
+const { stderr } = await execBuild();
+
+const expectedWarning =
+  "Duplicated package: z ./node_modules/z (chosen) vs ./a/node_modules/z in ./a";
 
-const actualErrorOutput = postProcessErrorOutput(stderr.toString());
-if (updateTests) {
-  await fs.writeFile(expectedFilePath, actualErrorOutput);
-} else {
-  const expectedErrorOutput = postProcessErrorOutput(
-    await fs.readFile(expectedFilePath, { encoding: "utf-8" }),
+if (!stderr.includes(expectedWarning)) {
+  assert.fail(
+    `Expected duplicate package warning not found in stderr.\nExpected: ${expectedWarning}\nActual stderr:\n${stderr}`,
   );
-  if (expectedErrorOutput !== actualErrorOutput) {
-    console.error(`The old and new error output aren't the same`);
-    console.error("\n=== Old:");
-    console.error(expectedErrorOutput);
-    console.error("\n=== New:");
-    console.error(actualErrorOutput);
-    process.exit(1);
-  }
 }