diff --git a/changes/clean-invalid-module-graphs.fix.md b/changes/clean-invalid-module-graphs.fix.md new file mode 100644 index 00000000000..a59339a61cf --- /dev/null +++ b/changes/clean-invalid-module-graphs.fix.md @@ -0,0 +1 @@ +- Allow `rescript clean` to remove generated files when the source module graph is invalid, including out-of-source `lib/es6` and `lib/js` outputs in the project and its dependencies. https://github.com/rescript-lang/rescript/pull/8640 diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 9661f2911d2..8de3128eaef 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -1,12 +1,11 @@ use super::build_types::*; use super::packages; -use crate::build; use crate::build::packages::Package; -use crate::config::{Config, SourceMapCommand}; +use crate::config::Config; use crate::helpers; use crate::helpers::emojis::*; use crate::project_context::ProjectContext; -use ahash::AHashSet; +use ahash::{AHashMap, AHashSet}; use anyhow::Result; use console::style; use rayon::prelude::*; @@ -68,37 +67,36 @@ fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files(build_state: &BuildState, root_config: &Config) { - // get all rescript file locations - let rescript_file_locations = build_state - .modules - .values() - .filter_map(|module| match &module.source_type { - SourceType::SourceFile(source_file) => { - build_state.packages.get(&module.package_name).map(|package| { - root_config - .get_package_specs() - .into_iter() - .filter_map(|spec| { - if spec.in_source { - Some(( - package.path.join(&source_file.implementation.path), - match spec.suffix { - None => root_config.get_suffix(&spec), - Some(suffix) => suffix, - }, - )) - } else { - None - } - }) - .collect::>() - }) +fn clean_source_files(packages: &AHashMap, root_config: &Config) { + let mut rescript_file_locations = Vec::new(); + for package in packages.values() { + let Some(source_files) = &package.source_files else { + continue; + }; + for source_file in source_files.keys().filter(|source_file| { + source_file + .extension() + .and_then(|extension| extension.to_str()) + .is_some_and(helpers::is_implementation_file) + }) { + for spec in root_config.get_package_specs() { + let output_base = if spec.in_source { + package.path.join(source_file) + } else { + package + .path + .join("lib") + .join(spec.get_out_of_source_dir()) + .join(source_file) + }; + let suffix = match &spec.suffix { + None => root_config.get_suffix(&spec), + Some(suffix) => suffix.clone(), + }; + rescript_file_locations.push((output_base, suffix)); } - _ => None, - }) - .flatten() - .collect::>(); + } + } rescript_file_locations .par_iter() @@ -342,7 +340,6 @@ pub fn cleanup_after_build(build_state: &BuildCommandState) { #[instrument(name = "clean.clean", skip_all)] pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) -> Result<()> { let project_context = ProjectContext::new(path)?; - let compiler_info = build::get_compiler_info(&project_context)?; // `clean` always acts on the full set of source directories regardless of which features are // active. We explicitly pass `None` so every tagged source folder is included and its // artifacts can be removed, even for features the user hasn't enabled for this build. @@ -376,27 +373,13 @@ pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) - } let timing_clean_mjs = Instant::now(); - let mut build_state = BuildState::new(project_context, packages, compiler_info, SourceMapCommand::Build); - packages::parse_packages(&mut build_state)?; - let root_config = build_state.get_root_config(); - let suffix_for_print = match root_config.package_specs { - None => match &root_config.suffix { - None => String::from(".js"), - Some(suffix) => suffix.clone(), - }, - Some(_) => root_config - .get_package_specs() - .into_iter() - .filter_map(|spec| { - if spec.in_source { - spec.suffix.or_else(|| root_config.suffix.clone()) - } else { - None - } - }) - .collect::>() - .join(", "), - }; + let root_config = project_context.get_root_config(); + let suffix_for_print = root_config + .get_package_specs() + .iter() + .map(|spec| root_config.get_suffix(spec)) + .collect::>() + .join(", "); if !plain_output && show_progress { print!( @@ -408,7 +391,7 @@ pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) - let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, root_config); + clean_source_files(&packages, root_config); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !plain_output && show_progress { diff --git a/rewatch/tests/clean/05-clean-invalid-module-graph.sh b/rewatch/tests/clean/05-clean-invalid-module-graph.sh new file mode 100755 index 00000000000..c761e8fae33 --- /dev/null +++ b/rewatch/tests/clean/05-clean-invalid-module-graph.sh @@ -0,0 +1,58 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: Clean succeeds when the module graph is invalid" + +rewatch clean > /dev/null 2>&1 +cp rescript.json rescript.json.clean-invalid.bak + +restore_fixture() { + rm -rf src/dupe-a src/dupe-b + if [ -f rescript.json.clean-invalid.bak ]; then + mv rescript.json.clean-invalid.bak rescript.json + fi + rewatch build > /dev/null 2>&1 || true +} +trap restore_fixture EXIT + +node -e ' + const fs = require("fs"); + const path = process.argv[1]; + const config = JSON.parse(fs.readFileSync(path, "utf8")); + config["package-specs"] = [ + {module: "esmodule", "in-source": true}, + {module: "esmodule", "in-source": false, suffix: ".out.mjs"}, + ]; + fs.writeFileSync(path, `${JSON.stringify(config, null, 2)}\n`); +' rescript.json + +rewatch build > /dev/null 2>&1 + +if [ ! -f src/Test.mjs ] || [ ! -f lib/es6/src/Test.out.mjs ]; then + error "Test setup did not produce both in-source and out-of-source outputs" + exit 1 +fi + +mkdir -p src/dupe-a src/dupe-b +echo 'let value = 1' > src/dupe-a/DuplicateModule.res +echo 'let value = 2' > src/dupe-b/DuplicateModule.res + +error_output=$(rewatch clean 2>&1) +clean_status=$? + +if [ $clean_status -ne 0 ]; then + error "Clean failed for an invalid module graph" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +if [ -f src/Test.mjs ] || [ -f lib/es6/src/Test.out.mjs ]; then + error "Clean left a generated output behind" + exit 1 +fi + +restore_fixture +trap - EXIT +success "Invalid module graph cleaned" diff --git a/rewatch/tests/clean/06-clean-duplicate-names-across-packages.sh b/rewatch/tests/clean/06-clean-duplicate-names-across-packages.sh new file mode 100755 index 00000000000..501b08b550b --- /dev/null +++ b/rewatch/tests/clean/06-clean-duplicate-names-across-packages.sh @@ -0,0 +1,61 @@ +#!/bin/bash +set -e +cd "$(dirname "$0")" +source "../utils.sh" + +bold "Test: Clean removes same-named modules from every package" + +test_dir=$(pwd) +fixture=$(mktemp -d) +trap 'cd "$test_dir"; rm -rf "$fixture"' EXIT +cd "$fixture" + +node <<'NODE' +const fs = require("fs"); +const config = name => ({ + name, + sources: ["src"], + namespace: false, + "package-specs": {module: "esmodule", "in-source": true}, + suffix: ".mjs", +}); +const dependencies = ["clean-duplicate-a", "clean-duplicate-b"]; +fs.mkdirSync("src"); +fs.writeFileSync("src/Main.res", "let value = 0\n"); +fs.writeFileSync("rescript.json", JSON.stringify({...config("clean-duplicates"), dependencies})); +for (const [index, name] of dependencies.entries()) { + const directory = `node_modules/${name}`; + fs.mkdirSync(`${directory}/src`, {recursive: true}); + fs.writeFileSync(`${directory}/package.json`, JSON.stringify({name, version: "1.0.0"})); + fs.writeFileSync(`${directory}/rescript.json`, JSON.stringify(config(name))); + fs.writeFileSync(`${directory}/src/Foo.res`, `let value = ${index}\n`); +} +NODE + +# Build separately so both packages have real output before root-level cleanup. +for package in clean-duplicate-a clean-duplicate-b; do + if ! output=$(cd "node_modules/$package" && rewatch build 2>&1); then + error "Could not build $package" + printf '%s\n' "$output" >&2 + exit 1 + fi + if [ ! -f "node_modules/$package/src/Foo.mjs" ]; then + error "Test setup did not produce $package/src/Foo.mjs" + exit 1 + fi +done + +if ! output=$(rewatch clean 2>&1); then + error "Could not clean packages with same-named modules" + printf '%s\n' "$output" >&2 + exit 1 +fi + +for package in clean-duplicate-a clean-duplicate-b; do + if [ -f "node_modules/$package/src/Foo.mjs" ]; then + error "Clean left $package/src/Foo.mjs behind" + exit 1 + fi +done + +success "Same-named modules cleaned from both packages" diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 3209eae41d8..afd7a881b8a 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -163,6 +163,8 @@ fi ./clean/02-clean-dev-dependencies.sh && ./clean/03-clean-node-modules.sh && ./clean/04-clean-rebuild-no-compiler-update.sh && +./clean/05-clean-invalid-module-graph.sh && +./clean/06-clean-duplicate-names-across-packages.sh && # Experimental tests ./experimental/01-experimental-features-emit.sh &&