From 29a555119495c5a52589cf714958440838e1e8d0 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Fri, 9 Oct 2026 14:53:01 +0200 Subject: [PATCH 1/3] Allow cleaning projects with an invalid module graph Signed-off-by: Florian Hammerschmidt --- rewatch/src/build/clean.rs | 71 +++++++++---------- .../clean/05-clean-invalid-module-graph.sh | 58 +++++++++++++++ rewatch/tests/suite.sh | 1 + 3 files changed, 92 insertions(+), 38 deletions(-) create mode 100755 rewatch/tests/clean/05-clean-invalid-module-graph.sh diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 4fdc3e1faaa..e32dbc5b8bd 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,9 +373,7 @@ 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 root_config = project_context.get_root_config(); let suffix_for_print = match root_config.package_specs { None => match &root_config.suffix { None => String::from(".js"), @@ -408,7 +403,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/suite.sh b/rewatch/tests/suite.sh index 63b42d1f47d..b96c25583f6 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -122,6 +122,7 @@ 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 && # Experimental tests ./experimental/01-experimental-features-emit.sh && From 46500fa828cdeb49683532c19116e59710ca3a15 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Tue, 20 Oct 2026 15:42:56 +0200 Subject: [PATCH 2/3] Restore approved clean follow-ups after rebase Restore the changelog fragment, duplicate-package regression, and progress suffix list that were lost during the rebase. Cleanup behavior matches the approved revision while retaining master's remove_js_file rename and suite updates. Validation: Fresh Rust build, all 146 unit tests, all six cleanup integration tests, Clippy, Rust formatting, shell syntax, and changelog validation pass. Signed-off-by: Florian Hammerschmidt --- changes/clean-invalid-module-graphs.fix.md | 1 + rewatch/src/build/clean.rs | 24 ++------ ...6-clean-duplicate-names-across-packages.sh | 61 +++++++++++++++++++ rewatch/tests/suite.sh | 1 + 4 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 changes/clean-invalid-module-graphs.fix.md create mode 100755 rewatch/tests/clean/06-clean-duplicate-names-across-packages.sh 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 e32dbc5b8bd..a5e620b7d91 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -374,24 +374,12 @@ pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) - let timing_clean_mjs = Instant::now(); let root_config = project_context.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 suffix_for_print = root_config + .get_package_specs() + .iter() + .map(|spec| root_config.get_suffix(spec)) + .collect::>() + .join(", "); if !plain_output && show_progress { print!( 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 b96c25583f6..703304ac975 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -123,6 +123,7 @@ fi ./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 && From 1d9d3b229d213cc4793503a43a9b9e9eb837cbbf Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Tue, 20 Oct 2026 16:01:23 +0200 Subject: [PATCH 3/3] Align cleanup parity checks with successful Rust cleanup The parity suite still expected Rust cleanup to panic without a compiler, reject a missing runtime, and leave generated files after duplicate-module errors. Require successful cleanup in both implementations and removal of both duplicate outputs. Remove the obsolete out-of-source cleanup difference from the OCaml build-system guide. Validation: Fresh Rust build, five focused command-parity cases from the CI script, all six cleanup integration tests for each CLI, canonical suite coverage, shell syntax, changelog validation, and whitespace checks pass. Signed-off-by: Florian Hammerschmidt --- rewatch-ocaml/README.md | 1 - rewatch-ocaml/tests/check_command_validation.sh | 14 +++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/rewatch-ocaml/README.md b/rewatch-ocaml/README.md index d0d9b8f4daf..f79281cd0ec 100644 --- a/rewatch-ocaml/README.md +++ b/rewatch-ocaml/README.md @@ -91,7 +91,6 @@ These are intentional: - Configuration errors use the compiler-style form `path: message`, and JSON syntax errors use `path:line:column: message`. Messages name the field and the accepted values instead of the parser's internal types. -- `rescript clean` also removes out-of-source JavaScript outputs. - The CLI uses Cmdliner: - help is rendered as a man page; - `--no-timing` takes no value; diff --git a/rewatch-ocaml/tests/check_command_validation.sh b/rewatch-ocaml/tests/check_command_validation.sh index 6e09982bc5c..972d94c1ac3 100755 --- a/rewatch-ocaml/tests/check_command_validation.sh +++ b/rewatch-ocaml/tests/check_command_validation.sh @@ -747,17 +747,17 @@ run_case compiler-args-no-project panic reject compiler-args "$work/orphan/A.res run_missing_bsc_case build-missing-bsc build "$project" run_missing_bsc_case format-missing-bsc format "$project/src/A.res" RESCRIPT_BSC_EXE="$work/missing-bsc" run_both clean "$work/clean-missing-bsc" -if [ "$(classify "$rust_status")" != panic ] || [ "$ocaml_status" -ne 0 ] || \ +if [ "$rust_status" -ne 0 ] || [ "$ocaml_status" -ne 0 ] || \ [ -e "$work/clean-missing-bsc/lib/bs/marker" ]; then - echo "clean-missing-bsc: expected Rust panic and successful OCaml cleanup" >&2 + echo "clean-missing-bsc: expected both implementations to clean without a compiler" >&2 dump_both exit 1 fi checked=$((checked + 1)) run_both --unset RESCRIPT_RUNTIME clean "$work/clean-missing-runtime" -if [ "$(classify "$rust_status")" != reject ] || [ "$ocaml_status" -ne 0 ] || \ +if [ "$rust_status" -ne 0 ] || [ "$ocaml_status" -ne 0 ] || \ [ -e "$work/clean-missing-runtime/lib/bs/marker" ]; then - echo "clean-missing-runtime: expected Rust rejection and successful OCaml cleanup" >&2 + echo "clean-missing-runtime: expected both implementations to clean without a runtime" >&2 dump_both exit 1 fi @@ -1541,9 +1541,9 @@ run_case clean-missing-dependency exit2 exit2 clean "$work/missing-dependency" run_case clean-configless-dependency exit2 exit2 clean "$work/configless-dependency" run_case clean-malformed-dependency exit2 exit2 clean "$work/malformed-dependency" run_both clean "$work/clean-duplicate-{impl}" -if [ "$(classify "$rust_status")" != reject ] || [ "$ocaml_status" -ne 0 ] || \ - [ ! -e "$work/clean-duplicate-rust/src/one/A.js" ] || \ - [ ! -e "$work/clean-duplicate-rust/src/two/A.js" ] || \ +if [ "$rust_status" -ne 0 ] || [ "$ocaml_status" -ne 0 ] || \ + [ -e "$work/clean-duplicate-rust/src/one/A.js" ] || \ + [ -e "$work/clean-duplicate-rust/src/two/A.js" ] || \ [ -e "$work/clean-duplicate-rust/lib/bs/marker" ] || \ [ -e "$work/clean-duplicate-ocaml/src/one/A.js" ] || \ [ -e "$work/clean-duplicate-ocaml/src/two/A.js" ] || \