diff --git a/CHANGELOG.md b/CHANGELOG.md index 38ae938e3b..6ddafb0a0d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index f01fba1897..1779fd499e 100644 --- a/rewatch/src/build/packages.rs +++ b/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, 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(¤t_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(¤t_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> { // Store all packages and completely deduplicate them let mut map: AHashMap = 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); diff --git a/tests/build_tests/duplicated_symlinked_packages/input.js b/tests/build_tests/duplicated_symlinked_packages/input.js index a3871d23e0..3124a036af 100644 --- a/tests/build_tests/duplicated_symlinked_packages/input.js +++ b/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); - } } diff --git a/tests/build_tests/duplicated_symlinked_packages/out.expected b/tests/build_tests/duplicated_symlinked_packages/out.expected deleted file mode 100644 index 57e0a503f3..0000000000 --- a/tests/build_tests/duplicated_symlinked_packages/out.expected +++ /dev/null @@ -1,2 +0,0 @@ -Duplicated package: z ./node_modules/z (chosen) vs ./node_modules/a/node_modules/z in ./node_modules/a -Duplicated package: z ./node_modules/z (chosen) vs ./node_modules/a/node_modules/z in ./node_modules/a \ No newline at end of file