From 776801c06d690b570abf0e141e1b689c152cfa03 Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 2 Aug 2025 21:43:54 +0200 Subject: [PATCH 01/47] Be more aware of the workspace during clean --- rewatch/src/build/clean.rs | 81 +++++++++++++++++++++++++---------- rewatch/src/build/packages.rs | 10 ++++- 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index cb7a7db73f..028f5cd129 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -1,5 +1,6 @@ use super::build_types::*; use super::packages; +use crate::build::packages::Package; use crate::helpers; use crate::helpers::emojis::*; use ahash::AHashSet; @@ -329,6 +330,9 @@ pub fn cleanup_after_build(build_state: &BuildState) { pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_deps: bool) -> Result<()> { let project_root = helpers::get_abs_path(path); let workspace_root = helpers::get_workspace_root(&project_root); + let workspace_config_name = &workspace_root + .as_ref() + .and_then(|wr| packages::read_package_name(wr).ok()); let packages = packages::make( &None, &project_root, @@ -348,30 +352,21 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ ); let _ = std::io::stdout().flush(); }; - packages.iter().for_each(|(_, package)| { - if show_progress { - if snapshot_output { - println!("Cleaning {}", package.name) - } else { - print!( - "{}{} {}Cleaning {}...", - LINE_CLEAR, - style("[1/2]").bold().dim(), - SWEEP, - package.name - ); - } - let _ = std::io::stdout().flush(); + match &workspace_root { + Some(_) => { + // There is a workspace detected, so will only clean the current package + let package = packages + .get(&root_config_name) + .expect("Could not find package during clean"); + clean_package(show_progress, snapshot_output, package); } + None => { + packages.iter().for_each(|(_, package)| { + clean_package(show_progress, snapshot_output, package); + }); + } + } - let path_str = package.get_build_path(); - let path = std::path::Path::new(&path_str); - let _ = std::fs::remove_dir_all(path); - - let path_str = package.get_ocaml_build_path(); - let path = std::path::Path::new(&path_str); - let _ = std::fs::remove_dir_all(path); - }); let timing_clean_compiler_assets_elapsed = timing_clean_compiler_assets.elapsed(); if !snapshot_output && show_progress { @@ -399,7 +394,22 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ .get(&build_state.root_config_name) .expect("Could not find root package"); - let suffix = root_package.config.suffix.as_deref().unwrap_or(".res.mjs"); + // Use the current package suffix if present. + // Otherwise, use the parent suffix if present. + // Fall back to .res.mjs + let suffix = match root_package.config.suffix.as_deref() { + Some(suffix) => suffix, + None => match &workspace_config_name { + None => ".res.mjs", + Some(workspace_config_name) => { + if let Some(package) = build_state.packages.get(workspace_config_name) { + package.config.suffix.as_deref().unwrap_or(".res.mjs") + } else { + ".res.mjs" + } + } + }, + }; if !snapshot_output && show_progress { println!( @@ -428,3 +438,28 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ Ok(()) } + +fn clean_package(show_progress: bool, snapshot_output: bool, package: &Package) { + if show_progress { + if snapshot_output { + println!("Cleaning {}", package.name) + } else { + print!( + "{}{} {}Cleaning {}...", + LINE_CLEAR, + style("[1/2]").bold().dim(), + SWEEP, + package.name + ); + } + let _ = std::io::stdout().flush(); + } + + let path_str = package.get_build_path(); + let path = std::path::Path::new(&path_str); + let _ = std::fs::remove_dir_all(path); + + let path_str = package.get_ocaml_build_path(); + let path = std::path::Path::new(&path_str); + let _ = std::fs::remove_dir_all(path); +} diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 24eca3d82e..78996ffa8d 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -611,7 +611,15 @@ pub fn make( show_progress: bool, build_dev_deps: bool, ) -> Result> { - let map = read_packages(root_folder, workspace_root, show_progress, build_dev_deps)?; + let map = match &workspace_root { + Some(wr) => { + let root_folder_str = root_folder.to_string_lossy(); + let workspace_root_str = wr.to_string_lossy(); + log::debug!("Building workspace: {workspace_root_str} for {root_folder_str}",); + read_packages(wr, workspace_root, show_progress, build_dev_deps)? + } + None => read_packages(root_folder, workspace_root, show_progress, build_dev_deps)?, + }; /* Once we have the deduplicated packages, we can add the source files for each - to minimize * the IO */ From dff32e78a76e7112acf03afec8b881738105948a Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 2 Aug 2025 22:24:38 +0200 Subject: [PATCH 02/47] Only clean current package --- rewatch/src/build/clean.rs | 72 ++++++++++++++++++++++------------- rewatch/src/build/packages.rs | 30 +++++++++++++-- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 028f5cd129..6aa4b53da6 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -29,7 +29,7 @@ fn remove_iast(package: &packages::Package, source_file: &Path) { )); } -fn remove_mjs_file(source_file: &Path, suffix: &String) { +fn remove_mjs_file(source_file: &Path, suffix: &str) { let _ = std::fs::remove_file(source_file.with_extension( // suffix.to_string includes the ., so we need to remove it &suffix.to_string()[1..], @@ -59,36 +59,51 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files(build_state: &BuildState, root_package: &packages::Package) { +fn clean_source_files( + build_state: &BuildState, + // If the root_package is part of a workspace, we only want to clean that package, + // not the entire build_state modules. + workspace_has_root_config: bool, + root_package: &Package, + suffix: &str, +) { // get all rescript file locations let rescript_file_locations = build_state .modules .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - let package = build_state.packages.get(&module.package_name).unwrap(); - Some( - root_package - .config - .get_package_specs() - .iter() - .filter_map(|spec| { - if spec.in_source { - Some(( - package.path.join(&source_file.implementation.path), - root_package.config.get_suffix(spec), - )) - } else { - None - } - }) - .collect::>(), - ) + if !workspace_has_root_config || module.package_name == root_package.name + { + let package = build_state.packages.get(&module.package_name).unwrap(); + Some( + root_package + .config + .get_package_specs() + .iter() + .filter_map(|spec| { + if spec.in_source { + Some(( + package.path.join(&source_file.implementation.path), + match &root_package.config.suffix { + None => suffix, + Some(sfx) => sfx, + }, + )) + } else { + None + } + }) + .collect::>(), + ) + } else { + None + } } _ => None, }) .flatten() - .collect::>(); + .collect::>(); rescript_file_locations .par_iter() @@ -352,15 +367,20 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ ); let _ = std::io::stdout().flush(); }; - match &workspace_root { - Some(_) => { - // There is a workspace detected, so will only clean the current package + + let mut workspace_has_root_config = false; + match &workspace_config_name { + Some(workspace_config_name) if packages.contains_key(workspace_config_name) => { + // The workspace package was found in the packages. + // This means that the root_config and workspace_config have a parent/child relationship. + // So we only want to clean the root package in this case. let package = packages .get(&root_config_name) .expect("Could not find package during clean"); clean_package(show_progress, snapshot_output, package); + workspace_has_root_config = true; } - None => { + _ => { packages.iter().for_each(|(_, package)| { clean_package(show_progress, snapshot_output, package); }); @@ -421,7 +441,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, root_package); + clean_source_files(&build_state, workspace_has_root_config, root_package, suffix); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 78996ffa8d..3a58887e67 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -613,10 +613,32 @@ pub fn make( ) -> Result> { let map = match &workspace_root { Some(wr) => { - let root_folder_str = root_folder.to_string_lossy(); - let workspace_root_str = wr.to_string_lossy(); - log::debug!("Building workspace: {workspace_root_str} for {root_folder_str}",); - read_packages(wr, workspace_root, show_progress, build_dev_deps)? + // If the workspace root contains the root_folder as a dependency, + // it should be considered as related, and we read the workspace. + let root_name = read_package_name(root_folder)?; + let workspace_config = read_config(wr)?; + + let is_child = workspace_config + .dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == &root_name) + || workspace_config + .dev_dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == &root_name); + + if is_child { + let root_folder_str = root_folder.to_string_lossy(); + let workspace_root_str = wr.to_string_lossy(); + log::info!("Building workspace: {workspace_root_str} for {root_folder_str}",); + read_packages(wr, workspace_root, show_progress, build_dev_deps)? + } else { + read_packages(root_folder, workspace_root, show_progress, build_dev_deps)? + } } None => read_packages(root_folder, workspace_root, show_progress, build_dev_deps)?, }; From 119d553a3b6dd88359edba35fcad6e6be034d70d Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 2 Aug 2025 22:35:17 +0200 Subject: [PATCH 03/47] fmt --- rewatch/src/build/clean.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 6aa4b53da6..31a4d523b2 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -73,8 +73,7 @@ fn clean_source_files( .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - if !workspace_has_root_config || module.package_name == root_package.name - { + if !workspace_has_root_config || module.package_name == root_package.name { let package = build_state.packages.get(&module.package_name).unwrap(); Some( root_package From 78bfd88a5cf6c75fadb09262ab2c11dd9ad149ca Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Aug 2025 09:30:06 +0200 Subject: [PATCH 04/47] Return is_child from packages make --- rewatch/src/build.rs | 2 +- rewatch/src/build/clean.rs | 29 ++++++++++++----------------- rewatch/src/build/packages.rs | 10 ++++++---- rewatch/src/format.rs | 3 ++- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 990333ca05..88e462453b 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -131,7 +131,7 @@ pub fn initialize_build( } let timing_package_tree = Instant::now(); - let packages = packages::make( + let (packages, _is_child) = packages::make( filter, &project_root, &workspace_root, diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 31a4d523b2..51115cc4de 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -347,7 +347,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let workspace_config_name = &workspace_root .as_ref() .and_then(|wr| packages::read_package_name(wr).ok()); - let packages = packages::make( + let (packages, workspace_has_root_config) = packages::make( &None, &project_root, &workspace_root, @@ -367,23 +367,18 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let _ = std::io::stdout().flush(); }; - let mut workspace_has_root_config = false; - match &workspace_config_name { - Some(workspace_config_name) if packages.contains_key(workspace_config_name) => { - // The workspace package was found in the packages. - // This means that the root_config and workspace_config have a parent/child relationship. - // So we only want to clean the root package in this case. - let package = packages - .get(&root_config_name) - .expect("Could not find package during clean"); + if workspace_has_root_config { + // The workspace package was found in the packages. + // This means that the root_config and workspace_config have a parent/child relationship. + // So we only want to clean the root package in this case. + let package = packages + .get(&root_config_name) + .expect("Could not find package during clean"); + clean_package(show_progress, snapshot_output, package); + } else { + packages.iter().for_each(|(_, package)| { clean_package(show_progress, snapshot_output, package); - workspace_has_root_config = true; - } - _ => { - packages.iter().for_each(|(_, package)| { - clean_package(show_progress, snapshot_output, package); - }); - } + }); } let timing_clean_compiler_assets_elapsed = timing_clean_compiler_assets.elapsed(); diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 3a58887e67..10e8253855 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -603,14 +603,16 @@ fn extend_with_children( /// 2. Take the (by then deduplicated) packages, and find all the '.res' and /// interface files. /// -/// The two step process is there to reduce IO overhead +/// The two step process is there to reduce IO overhead. +/// `is_child` is true when the workspace_root rescript.json has the name of the root_folder rescript.json pub fn make( filter: &Option, root_folder: &Path, workspace_root: &Option, show_progress: bool, build_dev_deps: bool, -) -> Result> { +) -> Result<(AHashMap, bool)> { + let mut is_child = false; let map = match &workspace_root { Some(wr) => { // If the workspace root contains the root_folder as a dependency, @@ -618,7 +620,7 @@ pub fn make( let root_name = read_package_name(root_folder)?; let workspace_config = read_config(wr)?; - let is_child = workspace_config + is_child = workspace_config .dependencies .to_owned() .unwrap_or_default() @@ -647,7 +649,7 @@ pub fn make( * the IO */ let result = extend_with_children(filter, map, build_dev_deps); - Ok(result) + Ok((result, is_child)) } pub fn parse_packages(build_state: &mut BuildState) { diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index 742a7a1511..01b7e3e9ba 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -38,7 +38,8 @@ fn get_all_files() -> Result> { let project_root = helpers::get_abs_path(¤t_dir); let workspace_root_option = helpers::get_workspace_root(&project_root); - let build_state = packages::make(&None, &project_root, &workspace_root_option, false, false)?; + let (build_state, _is_child) = + packages::make(&None, &project_root, &workspace_root_option, false, false)?; let mut files: Vec = Vec::new(); for (_package_name, package) in build_state { From 81966e0259df3026540d72a1274718e8134c0361 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Aug 2025 09:57:23 +0200 Subject: [PATCH 05/47] Invert if --- rewatch/src/build/clean.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 51115cc4de..3563a9e2f3 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -73,7 +73,11 @@ fn clean_source_files( .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - if !workspace_has_root_config || module.package_name == root_package.name { + if workspace_has_root_config && module.package_name != root_package.name { + // Skip this package as we only want to clean the root_package + // if is the child of the workspace. + None + } else { let package = build_state.packages.get(&module.package_name).unwrap(); Some( root_package @@ -95,8 +99,6 @@ fn clean_source_files( }) .collect::>(), ) - } else { - None } } _ => None, From 7b84cb4a4b397269924c7212f64ae211d854b102 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Aug 2025 19:15:17 +0200 Subject: [PATCH 06/47] Introduce PackageMap to make things a bit more clear. --- rewatch/src/build.rs | 3 ++- rewatch/src/build/clean.rs | 7 +++++-- rewatch/src/build/packages.rs | 21 ++++++++++++++++----- rewatch/src/format.rs | 4 ++-- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 88e462453b..2f0bc7030a 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -11,6 +11,7 @@ pub mod read_compile_state; use self::compile::compiler_args; use self::parse::parser_args; use crate::build::compile::{mark_modules_with_deleted_deps_dirty, mark_modules_with_expired_deps_dirty}; +use crate::build::packages::PackageMap; use crate::helpers::emojis::*; use crate::helpers::{self, get_workspace_root}; use crate::{config, sourcedirs}; @@ -131,7 +132,7 @@ pub fn initialize_build( } let timing_package_tree = Instant::now(); - let (packages, _is_child) = packages::make( + let PackageMap { packages, .. } = packages::make( filter, &project_root, &workspace_root, diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 3563a9e2f3..667531f56f 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -1,6 +1,6 @@ use super::build_types::*; use super::packages; -use crate::build::packages::Package; +use crate::build::packages::{Package, PackageMap}; use crate::helpers; use crate::helpers::emojis::*; use ahash::AHashSet; @@ -349,7 +349,10 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let workspace_config_name = &workspace_root .as_ref() .and_then(|wr| packages::read_package_name(wr).ok()); - let (packages, workspace_has_root_config) = packages::make( + let PackageMap { + packages, + root_is_child_of_workspace: workspace_has_root_config, + } = packages::make( &None, &project_root, &workspace_root, diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 10e8253855..2607f4772c 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -597,6 +597,14 @@ fn extend_with_children( build } +pub struct PackageMap { + pub packages: AHashMap, + /// When packages::make is invoked with a workspace_root, + /// this flag determines if there is a parent/child relation + /// between root_folder and workspace_root. + pub root_is_child_of_workspace: bool, +} + /// Make turns a folder, that should contain a config, into a tree of Packages. /// It does so in two steps: /// 1. Get all the packages parsed, and take all the source folders from the config @@ -611,8 +619,8 @@ pub fn make( workspace_root: &Option, show_progress: bool, build_dev_deps: bool, -) -> Result<(AHashMap, bool)> { - let mut is_child = false; +) -> Result { + let mut root_is_child_of_workspace = false; let map = match &workspace_root { Some(wr) => { // If the workspace root contains the root_folder as a dependency, @@ -620,7 +628,7 @@ pub fn make( let root_name = read_package_name(root_folder)?; let workspace_config = read_config(wr)?; - is_child = workspace_config + root_is_child_of_workspace = workspace_config .dependencies .to_owned() .unwrap_or_default() @@ -633,7 +641,7 @@ pub fn make( .iter() .any(|dep| dep == &root_name); - if is_child { + if root_is_child_of_workspace { let root_folder_str = root_folder.to_string_lossy(); let workspace_root_str = wr.to_string_lossy(); log::info!("Building workspace: {workspace_root_str} for {root_folder_str}",); @@ -649,7 +657,10 @@ pub fn make( * the IO */ let result = extend_with_children(filter, map, build_dev_deps); - Ok((result, is_child)) + Ok(PackageMap { + packages: result, + root_is_child_of_workspace, + }) } pub fn parse_packages(build_state: &mut BuildState) { diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index 01b7e3e9ba..b5078231ff 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -38,11 +38,11 @@ fn get_all_files() -> Result> { let project_root = helpers::get_abs_path(¤t_dir); let workspace_root_option = helpers::get_workspace_root(&project_root); - let (build_state, _is_child) = + let packages::PackageMap { packages, .. } = packages::make(&None, &project_root, &workspace_root_option, false, false)?; let mut files: Vec = Vec::new(); - for (_package_name, package) in build_state { + for (_package_name, package) in packages { if package.is_local_dep && let Some(source_files) = package.source_files { From 178a0ec4478597a68e478a723264cb4cb9347bb7 Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 7 Aug 2025 13:52:19 +0200 Subject: [PATCH 07/47] Use workspace extension if there is a link, otherwise root_package. --- rewatch/src/build/clean.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 667531f56f..6f53c3a477 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -413,21 +413,25 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ .get(&build_state.root_config_name) .expect("Could not find root package"); - // Use the current package suffix if present. - // Otherwise, use the parent suffix if present. + // Use the workspace extension if the link is present. + // Otherwise, use the current suffix if present. // Fall back to .res.mjs - let suffix = match root_package.config.suffix.as_deref() { - Some(suffix) => suffix, - None => match &workspace_config_name { - None => ".res.mjs", - Some(workspace_config_name) => { - if let Some(package) = build_state.packages.get(workspace_config_name) { - package.config.suffix.as_deref().unwrap_or(".res.mjs") - } else { - ".res.mjs" - } - } - }, + let fallback_extension = ".res.mjs"; + let suffix = if workspace_has_root_config { + match &workspace_config_name { + None => fallback_extension, + Some(workspace_config_name) => build_state + .packages + .get(workspace_config_name) + .and_then(|workspace_package| workspace_package.config.suffix.as_deref()) + .unwrap_or(fallback_extension), + } + } else { + root_package + .config + .suffix + .as_deref() + .unwrap_or(fallback_extension) }; if !snapshot_output && show_progress { From 5b2e4274a7b2c8b12731cb18b384ec5cf14935df Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 7 Aug 2025 13:52:49 +0200 Subject: [PATCH 08/47] Add a TODO that we need to clean up some confusing bits. --- rewatch/src/build.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 2f0bc7030a..38b3fcfc06 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -61,6 +61,8 @@ pub fn get_compiler_args(path: &Path) -> Result { let package_root = helpers::get_abs_path(&helpers::get_nearest_config(path).expect("Couldn't find package root")); let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p)); + // TODO: we load the workspace_root (if present) and call it root config + // This is all very confusing and we should introduce an Enum to represent the top level thing let root_rescript_config = packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?; let rescript_config = packages::read_config(&package_root)?; From 08a146d9eebd16e37531d1837b7d39de7caae13b Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 9 Aug 2025 19:07:59 +0200 Subject: [PATCH 09/47] Refactor using project_context --- rewatch/src/build.rs | 68 +++---- rewatch/src/build/build_types.rs | 33 +++- rewatch/src/build/clean.rs | 138 ++++++------- rewatch/src/build/compile.rs | 57 +++--- rewatch/src/build/packages.rs | 189 ++++++++---------- rewatch/src/build/parse.rs | 54 ++--- rewatch/src/build/read_compile_state.rs | 5 +- rewatch/src/cli.rs | 8 +- rewatch/src/format.rs | 28 ++- rewatch/src/helpers.rs | 47 +++-- rewatch/src/lib.rs | 1 + rewatch/src/main.rs | 7 +- rewatch/src/project_context.rs | 251 ++++++++++++++++++++++++ 13 files changed, 524 insertions(+), 362 deletions(-) create mode 100644 rewatch/src/project_context.rs diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 38b3fcfc06..9833cea25f 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -8,12 +8,11 @@ pub mod packages; pub mod parse; pub mod read_compile_state; -use self::compile::compiler_args; use self::parse::parser_args; use crate::build::compile::{mark_modules_with_deleted_deps_dirty, mark_modules_with_expired_deps_dirty}; -use crate::build::packages::PackageMap; use crate::helpers::emojis::*; -use crate::helpers::{self, get_workspace_root}; +use crate::helpers::{self}; +use crate::project_context::ProjectContext; use crate::{config, sourcedirs}; use anyhow::{Result, anyhow}; use build_types::*; @@ -56,35 +55,27 @@ pub struct CompilerArgs { pub parser_args: Vec, } -pub fn get_compiler_args(path: &Path) -> Result { - let filename = &helpers::get_abs_path(path); - let package_root = - helpers::get_abs_path(&helpers::get_nearest_config(path).expect("Couldn't find package root")); - let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p)); - // TODO: we load the workspace_root (if present) and call it root config - // This is all very confusing and we should introduce an Enum to represent the top level thing - let root_rescript_config = - packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?; - let rescript_config = packages::read_config(&package_root)?; - let is_type_dev = match filename.strip_prefix(&package_root) { +pub fn get_compiler_args(rescript_file_path: &Path) -> Result { + let filename = &helpers::get_abs_path(rescript_file_path); + let current_package = helpers::get_abs_path( + &helpers::get_nearest_config(rescript_file_path).expect("Couldn't find package root"), + ); + let project_context = ProjectContext::new(¤t_package)?; + + let is_type_dev = match filename.strip_prefix(¤t_package) { Err(_) => false, - Ok(relative_path) => root_rescript_config.find_is_type_dev_for_path(relative_path), + Ok(relative_path) => project_context + .get_current_rescript_config() + .find_is_type_dev_for_path(relative_path), }; // make PathBuf from package root and get the relative path for filename - let relative_filename = filename.strip_prefix(PathBuf::from(&package_root)).unwrap(); + let relative_filename = filename.strip_prefix(PathBuf::from(¤t_package)).unwrap(); - let file_path = PathBuf::from(&package_root).join(filename); + let file_path = PathBuf::from(¤t_package).join(filename); let contents = helpers::read_file(&file_path).expect("Error reading file"); - let (ast_path, parser_args) = parser_args( - &rescript_config, - &root_rescript_config, - relative_filename, - &workspace_root, - workspace_root.as_ref().unwrap_or(&package_root), - &contents, - ); + let (ast_path, parser_args) = parser_args(&project_context, relative_filename, &contents); let is_interface = filename.to_string_lossy().ends_with('i'); let has_interface = if is_interface { true @@ -93,15 +84,13 @@ pub fn get_compiler_args(path: &Path) -> Result { interface_filename.push('i'); PathBuf::from(&interface_filename).exists() }; - let compiler_args = compiler_args( - &rescript_config, - &root_rescript_config, + let compiler_args = compile::compiler_args( + project_context.get_current_rescript_config(), &ast_path, relative_filename, is_interface, has_interface, - &package_root, - &workspace_root, + &project_context, &None, is_type_dev, true, @@ -123,10 +112,8 @@ pub fn initialize_build( build_dev_deps: bool, snapshot_output: bool, ) -> Result { - let project_root = helpers::get_abs_path(path); - let workspace_root = helpers::get_workspace_root(&project_root); let bsc_path = helpers::get_bsc(); - let root_config_name = packages::read_package_name(&project_root)?; + let project_context = ProjectContext::new(path)?; if !snapshot_output && show_progress { print!("{} {}Building package tree...", style("[1/7]").bold().dim(), TREE); @@ -134,13 +121,7 @@ pub fn initialize_build( } let timing_package_tree = Instant::now(); - let PackageMap { packages, .. } = packages::make( - filter, - &project_root, - &workspace_root, - show_progress, - build_dev_deps, - )?; + let packages = packages::make(filter, &project_context, show_progress, build_dev_deps)?; let timing_package_tree_elapsed = timing_package_tree.elapsed(); if !snapshot_output && show_progress { @@ -170,7 +151,7 @@ pub fn initialize_build( let _ = stdout().flush(); } - let mut build_state = BuildState::new(project_root, root_config_name, packages, workspace_root, bsc_path); + let mut build_state = BuildState::new(project_context, packages, bsc_path); packages::parse_packages(&mut build_state); let timing_source_files_elapsed = timing_source_files.elapsed(); @@ -566,9 +547,8 @@ pub fn build( pub fn pass_through_legacy(mut args: Vec) -> i32 { let project_root = helpers::get_abs_path(Path::new(".")); - let workspace_root = helpers::get_workspace_root(&project_root); - - let rescript_legacy_path = helpers::get_rescript_legacy(&project_root, workspace_root); + let project_context = ProjectContext::new(&project_root).unwrap(); + let rescript_legacy_path = helpers::get_rescript_legacy(&project_context); args.insert(0, rescript_legacy_path.into()); let status = std::process::Command::new("node") diff --git a/rewatch/src/build/build_types.rs b/rewatch/src/build/build_types.rs index e8c1a28ee4..1d32bfae38 100644 --- a/rewatch/src/build/build_types.rs +++ b/rewatch/src/build/build_types.rs @@ -1,4 +1,5 @@ use crate::build::packages::{Namespace, Package}; +use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; use std::{fmt::Display, path::PathBuf, time::SystemTime}; @@ -89,14 +90,12 @@ impl Module { #[derive(Debug)] pub struct BuildState { + pub project_context: ProjectContext, pub modules: AHashMap, pub packages: AHashMap, pub module_names: AHashSet, - pub project_root: PathBuf, - pub root_config_name: String, pub deleted_modules: AHashSet, pub bsc_path: PathBuf, - pub workspace_root: Option, pub deps_initialized: bool, } @@ -109,20 +108,16 @@ impl BuildState { self.modules.get(module_name) } pub fn new( - project_root: PathBuf, - root_config_name: String, + project_context: ProjectContext, packages: AHashMap, - workspace_root: Option, bsc_path: PathBuf, ) -> Self { Self { + project_context, module_names: AHashSet::new(), modules: AHashMap::new(), packages, - project_root, - root_config_name, deleted_modules: AHashSet::new(), - workspace_root, bsc_path, deps_initialized: false, } @@ -132,6 +127,26 @@ impl BuildState { self.modules.insert(module_name.to_owned(), module); self.module_names.insert(module_name.to_owned()); } + + // TODO: Consider returing a result? + // At the same time, the packages have been processed in packages::make + // It would have already failed if the root is absent + pub fn get_root_package(&self) -> &Package { + match &self.project_context { + ProjectContext::SingleProject { config, .. } + | ProjectContext::MonorepoRoot { config, .. } + | ProjectContext::MonorepoPackage { + parent_config: config, + .. + } => self + .packages + .get(&config.name) + // You will be tempted to point this out as "don't panic" during code review. + // Please don't and see https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call + // cargo clippy pointed this out. + .unwrap_or_else(|| panic!("Root package \"{}\" not found", &config.name)), + } + } } #[derive(Debug)] diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 6f53c3a477..db2448ed8a 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -1,10 +1,11 @@ use super::build_types::*; use super::packages; -use crate::build::packages::{Package, PackageMap}; +use crate::build::packages::Package; use crate::helpers; use crate::helpers::emojis::*; -use ahash::AHashSet; -use anyhow::Result; +use crate::project_context::ProjectContext; +use ahash::{AHashMap, AHashSet}; +use anyhow::{Result, anyhow}; use console::style; use rayon::prelude::*; use std::io::Write; @@ -59,23 +60,16 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files( - build_state: &BuildState, - // If the root_package is part of a workspace, we only want to clean that package, - // not the entire build_state modules. - workspace_has_root_config: bool, - root_package: &Package, - suffix: &str, -) { +fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &String) { + let packages_to_clean = build_state.project_context.get_scoped_local_packages(); + // get all rescript file locations let rescript_file_locations = build_state .modules .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - if workspace_has_root_config && module.package_name != root_package.name { - // Skip this package as we only want to clean the root_package - // if is the child of the workspace. + if !packages_to_clean.contains(&module.package_name) { None } else { let package = build_state.packages.get(&module.package_name).unwrap(); @@ -88,23 +82,23 @@ fn clean_source_files( if spec.in_source { Some(( package.path.join(&source_file.implementation.path), - match &root_package.config.suffix { - None => suffix, - Some(sfx) => sfx, + match &spec.suffix { + None => suffix.to_owned(), + Some(suffix) => suffix.clone(), }, )) } else { None } }) - .collect::>(), + .collect::>(), ) } } _ => None, }) .flatten() - .collect::>(); + .collect::>(); rescript_file_locations .par_iter() @@ -343,23 +337,25 @@ pub fn cleanup_after_build(build_state: &BuildState) { }); } +fn find_and_clean_package( + packages: &AHashMap, + name: &String, + show_progress: bool, + snapshot_output: bool, +) -> Result<()> { + if let Some(package) = packages.get(name) { + clean_package(show_progress, snapshot_output, package); + Ok(()) + } else { + Err(anyhow!("Could not find package \"{}\" during clean", &name)) + } +} + pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_deps: bool) -> Result<()> { - let project_root = helpers::get_abs_path(path); - let workspace_root = helpers::get_workspace_root(&project_root); - let workspace_config_name = &workspace_root - .as_ref() - .and_then(|wr| packages::read_package_name(wr).ok()); - let PackageMap { - packages, - root_is_child_of_workspace: workspace_has_root_config, - } = packages::make( - &None, - &project_root, - &workspace_root, - show_progress, - build_dev_deps, - )?; - let root_config_name = packages::read_package_name(&project_root)?; + let project_context = ProjectContext::new(path)?; + + let packages = packages::make(&None, &project_context, show_progress, build_dev_deps)?; + // let root_config_name = packages::read_package_name(&project_root)?; let bsc_path = helpers::get_bsc(); let timing_clean_compiler_assets = Instant::now(); @@ -372,19 +368,30 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let _ = std::io::stdout().flush(); }; - if workspace_has_root_config { - // The workspace package was found in the packages. - // This means that the root_config and workspace_config have a parent/child relationship. - // So we only want to clean the root package in this case. - let package = packages - .get(&root_config_name) - .expect("Could not find package during clean"); - clean_package(show_progress, snapshot_output, package); - } else { - packages.iter().for_each(|(_, package)| { + match &project_context { + ProjectContext::SingleProject { config, .. } => { + find_and_clean_package(&packages, &config.name, show_progress, snapshot_output)?; + } + ProjectContext::MonorepoRoot { + config, + local_dependencies, + .. + } => { + find_and_clean_package(&packages, &config.name, show_progress, snapshot_output)?; + // TODO: this does the local dependencies and dev-dependencies. + // I guess the dev deps should be cleaned if flag is set? + for dep in local_dependencies { + find_and_clean_package(&packages, dep, show_progress, snapshot_output)?; + } + } + ProjectContext::MonorepoPackage { config, .. } => { + // We know we are in a monorepo, but we only clean the package that is being targeted. + let package = packages + .get(&config.name) + .expect("Could not find package during clean"); clean_package(show_progress, snapshot_output, package); - }); - } + } + }; let timing_clean_compiler_assets_elapsed = timing_clean_compiler_assets.elapsed(); @@ -400,39 +407,10 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ } let timing_clean_mjs = Instant::now(); - let mut build_state = BuildState::new( - project_root.to_owned(), - root_config_name, - packages, - workspace_root, - bsc_path, - ); + let mut build_state = BuildState::new(project_context, packages, bsc_path); packages::parse_packages(&mut build_state); - let root_package = build_state - .packages - .get(&build_state.root_config_name) - .expect("Could not find root package"); - - // Use the workspace extension if the link is present. - // Otherwise, use the current suffix if present. - // Fall back to .res.mjs - let fallback_extension = ".res.mjs"; - let suffix = if workspace_has_root_config { - match &workspace_config_name { - None => fallback_extension, - Some(workspace_config_name) => build_state - .packages - .get(workspace_config_name) - .and_then(|workspace_package| workspace_package.config.suffix.as_deref()) - .unwrap_or(fallback_extension), - } - } else { - root_package - .config - .suffix - .as_deref() - .unwrap_or(fallback_extension) - }; + let root_package = build_state.get_root_package(); + let suffix = build_state.project_context.get_suffix(); if !snapshot_output && show_progress { println!( @@ -444,7 +422,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, workspace_has_root_config, root_package, suffix); + clean_source_files(&build_state, root_package, &suffix); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index b8b4873c48..0a4d43680b 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -8,12 +8,13 @@ use super::packages; use crate::config; use crate::helpers; use crate::helpers::StrippedVerbatimPath; +use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; use anyhow::anyhow; use console::style; use log::{debug, trace}; use rayon::prelude::*; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; use std::time::SystemTime; @@ -154,21 +155,14 @@ pub fn compile( .get_package(&module.package_name) .expect("Package not found"); - let root_package = - build_state.get_package(&build_state.root_config_name).unwrap(); - let interface_result = match source_file.interface.to_owned() { Some(Interface { path, .. }) => { let result = compile_file( package, - root_package, &helpers::get_ast_path(&path), module, true, - &build_state.bsc_path, - &build_state.packages, - &build_state.project_root, - &build_state.workspace_root, + build_state, ); Some(result) } @@ -176,14 +170,10 @@ pub fn compile( }; let result = compile_file( package, - root_package, &helpers::get_ast_path(&source_file.implementation.path), module, false, - &build_state.bsc_path, - &build_state.packages, - &build_state.project_root, - &build_state.workspace_root, + build_state, ); let cmi_digest_after = helpers::compute_file_hash(Path::new(&cmi_path)); @@ -347,24 +337,21 @@ pub fn compile( pub fn compiler_args( config: &config::Config, - root_config: &config::Config, ast_path: &Path, file_path: &Path, is_interface: bool, has_interface: bool, - project_root: &Path, - workspace_root: &Option, + project_context: &ProjectContext, // if packages are known, we pass a reference here - // this saves us a scan to find their paths + // this saves us a scan to find their paths. + // This is None when called by build::get_compiler_args packages: &Option<&AHashMap>, // Is the file listed as "type":"dev"? is_type_dev: bool, is_local_dep: bool, ) -> Vec { let bsc_flags = config::flatten_flags(&config.compiler_flags); - - let dependency_paths = get_dependency_paths(config, project_root, workspace_root, packages, is_type_dev); - + let dependency_paths = get_dependency_paths(config, project_context, packages, is_type_dev); let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace()); let namespace_args = match &config.get_namespace() { @@ -388,6 +375,7 @@ pub fn compiler_args( packages::Namespace::NoNamespace => vec![], }; + let root_config = project_context.get_root_config(); let jsx_args = root_config.get_jsx_args(); let jsx_module_args = root_config.get_jsx_module_args(); let jsx_mode_args = root_config.get_jsx_mode_args(); @@ -449,7 +437,7 @@ pub fn compiler_args( "-I".to_string(), Path::new("..").join("ocaml").to_string_lossy().to_string(), ], - dependency_paths.concat(), + dependency_paths, jsx_args, jsx_module_args, jsx_mode_args, @@ -497,11 +485,10 @@ impl DependentPackage { fn get_dependency_paths( config: &config::Config, - project_root: &Path, - workspace_root: &Option, + project_context: &ProjectContext, packages: &Option<&AHashMap>, is_file_type_dev: bool, -) -> Vec> { +) -> Vec { let normal_deps = config .dependencies .clone() @@ -534,7 +521,7 @@ fn get_dependency_paths( .as_ref() .map(|package| package.path.clone()) } else { - packages::read_dependency(package_name, project_root, project_root, workspace_root).ok() + packages::read_dependency(package_name, project_context).ok() } .map(|canonicalized_path| { vec![ @@ -555,19 +542,23 @@ fn get_dependency_paths( dependency_path }) .collect::>>() + .concat() } fn compile_file( package: &packages::Package, - root_package: &packages::Package, ast_path: &Path, module: &Module, is_interface: bool, - bsc_path: &Path, - packages: &AHashMap, - project_root: &Path, - workspace_root: &Option, + build_state: &BuildState, ) -> Result, String> { + let BuildState { + packages, + project_context, + bsc_path, + .. + } = build_state; + let root_package = build_state.get_root_package(); let ocaml_build_path_abs = package.get_ocaml_build_path(); let build_path_abs = package.get_build_path(); let implementation_file_path = match &module.source_type { @@ -584,13 +575,11 @@ fn compile_file( let is_type_dev = module.is_type_dev; let to_mjs_args = compiler_args( &package.config, - &root_package.config, ast_path, implementation_file_path, is_interface, has_interface, - project_root, - workspace_root, + project_context, &Some(packages), is_type_dev, package.is_local_dep, diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 2607f4772c..d624a322e1 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -2,9 +2,11 @@ use super::build_types::*; use super::namespaces; use super::packages; use crate::config; +use crate::config::Config; use crate::helpers; use crate::helpers::StrippedVerbatimPath; use crate::helpers::emojis::*; +use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; use anyhow::{Result, anyhow}; use console::style; @@ -237,43 +239,48 @@ fn get_source_dirs(source: config::Source, sub_path: Option) -> AHashSe source_folders } -pub fn read_config(package_dir: &Path) -> Result { +pub fn read_config(package_dir: &Path) -> Result<(Config, PathBuf)> { let rescript_json_path = package_dir.join("rescript.json"); let bsconfig_json_path = package_dir.join("bsconfig.json"); if Path::new(&rescript_json_path).exists() { - config::Config::new(&rescript_json_path) + Config::new(&rescript_json_path).map(|c| (c, rescript_json_path)) } else { - config::Config::new(&bsconfig_json_path) + Config::new(&bsconfig_json_path).map(|c| (c, bsconfig_json_path)) } } -pub fn read_dependency( - package_name: &str, - parent_path: &Path, - project_root: &Path, - workspace_root: &Option, -) -> Result { - let path_from_parent = helpers::package_path(parent_path, package_name); - let path_from_project_root = helpers::package_path(project_root, package_name); - let maybe_path_from_workspace_root = workspace_root - .as_ref() - .map(|workspace_root| helpers::package_path(workspace_root, package_name)); - - let path = match ( - path_from_parent, - path_from_project_root, - maybe_path_from_workspace_root, - ) { - (path_from_parent, _, _) if path_from_parent.exists() => Ok(path_from_parent), - (_, path_from_project_root, _) if path_from_project_root.exists() => Ok(path_from_project_root), - (_, _, Some(path_from_workspace_root)) if path_from_workspace_root.exists() => { - Ok(path_from_workspace_root) - } - _ => Err(format!( +pub fn read_dependency(package_name: &str, project_context: &ProjectContext) -> Result { + // root folder + node_modules + package_name + let path_from_root = helpers::package_path(project_context.get_root_path(), package_name); + + // let path_from_project_root = helpers::package_path(project_root, package_name); + // let maybe_path_from_workspace_root = workspace_root + // .as_ref() + // .map(|workspace_root| helpers::package_path(workspace_root, package_name)); + + // let path = match ( + // path_from_parent, + // path_from_project_root, + // maybe_path_from_workspace_root, + // ) { + // (path_from_parent, _, _) if path_from_parent.exists() => Ok(path_from_parent), + // (_, path_from_project_root, _) if path_from_project_root.exists() => Ok(path_from_project_root), + // (_, _, Some(path_from_workspace_root)) if path_from_workspace_root.exists() => { + // Ok(path_from_workspace_root) + // } + // _ => Err(format!( + // "The package \"{package_name}\" is not found (are node_modules up-to-date?)..." + // )), + // }?; + + let path = (if path_from_root.exists() { + Ok(path_from_root) + } else { + Err(format!( "The package \"{package_name}\" is not found (are node_modules up-to-date?)..." - )), - }?; + )) + })?; let canonical_path = match path .canonicalize() @@ -299,17 +306,15 @@ pub fn read_dependency( /// recursively continues operation for their dependencies as well. fn read_dependencies( registered_dependencies_set: &mut AHashSet, - parent_config: &config::Config, - parent_path: &Path, - project_root: &Path, - workspace_root: &Option, + project_context: &ProjectContext, + package_config: &Config, show_progress: bool, build_dev_deps: bool, ) -> Vec { - let mut dependencies = parent_config.dependencies.to_owned().unwrap_or_default(); + let mut dependencies = package_config.dependencies.to_owned().unwrap_or_default(); // Concatenate dev dependencies if build_dev_deps is true - if build_dev_deps && let Some(dev_deps) = parent_config.dev_dependencies.to_owned() { + if build_dev_deps && let Some(dev_deps) = package_config.dev_dependencies.to_owned() { dependencies.extend(dev_deps); } @@ -327,8 +332,8 @@ fn read_dependencies( // Read all config files in parallel instead of blocking .par_iter() .map(|package_name| { - let (config, canonical_path) = - match read_dependency(package_name, parent_path, project_root, workspace_root) { + let ((config, _), canonical_path) = + match read_dependency(package_name, project_context) { Err(error) => { if show_progress { println!( @@ -339,7 +344,7 @@ fn read_dependencies( ); } - let parent_path_str = parent_path.to_string_lossy(); + let parent_path_str = project_context.get_root_path().to_string_lossy(); log::error!( "We could not build package tree reading dependency '{package_name}', at path '{parent_path_str}'. Error: {error}", ); @@ -350,7 +355,7 @@ fn read_dependencies( match read_config(&canonical_path) { Ok(config) => (config, canonical_path), Err(error) => { - let parent_path_str = parent_path.to_string_lossy(); + let parent_path_str = project_context.get_root_path().to_string_lossy(); log::error!( "We could not build package tree '{package_name}', at path '{parent_path_str}'. Error: {error}", ); @@ -361,16 +366,31 @@ fn read_dependencies( }; let is_local_dep = { - canonical_path.starts_with(project_root) - && !canonical_path.components().any(|c| c.as_os_str() == "node_modules") + match project_context { + ProjectContext::SingleProject { + config, + .. + } => config.name.as_str() == package_name + , + ProjectContext::MonorepoRoot { + local_dependencies, + .. + } => { + local_dependencies.contains(package_name) + } + ProjectContext::MonorepoPackage { + parent_path, + .. + } => { + helpers::is_local_package(parent_path, &canonical_path) + } + } }; let dependencies = read_dependencies( &mut registered_dependencies_set.to_owned(), + project_context, &config, - &canonical_path, - project_root, - workspace_root, show_progress, is_local_dep && build_dev_deps, ); @@ -380,7 +400,7 @@ fn read_dependencies( config, path: canonical_path, dependencies, - is_local_dep + is_local_dep, } }) .collect() @@ -469,25 +489,34 @@ This inconsistency will cause issues with package resolution.\n", } fn read_packages( - project_root: &Path, - workspace_root: &Option, + project_context: &ProjectContext, show_progress: bool, build_dev_deps: bool, ) -> Result> { - let root_config = read_config(project_root)?; - // Store all packages and completely deduplicate them let mut map: AHashMap = AHashMap::new(); - let root_package = make_package(root_config.to_owned(), project_root, true, true); + let root_package = match project_context { + ProjectContext::SingleProject { config, path } + | ProjectContext::MonorepoRoot { config, path, .. } + | ProjectContext::MonorepoPackage { + parent_config: config, + parent_path: path, + .. + } => { + let folder = path + .parent() + .ok_or_else(|| anyhow!("Could not the read parent folder or a rescript.json file"))?; + make_package(config.to_owned(), folder, true, true) + } + }; + map.insert(root_package.name.to_string(), root_package); let mut registered_dependencies_set: AHashSet = AHashSet::new(); let dependencies = flatten_dependencies(read_dependencies( &mut registered_dependencies_set, - &root_config, - project_root, - project_root, - workspace_root, + project_context, + project_context.get_root_config(), show_progress, build_dev_deps, )); @@ -597,14 +626,6 @@ fn extend_with_children( build } -pub struct PackageMap { - pub packages: AHashMap, - /// When packages::make is invoked with a workspace_root, - /// this flag determines if there is a parent/child relation - /// between root_folder and workspace_root. - pub root_is_child_of_workspace: bool, -} - /// Make turns a folder, that should contain a config, into a tree of Packages. /// It does so in two steps: /// 1. Get all the packages parsed, and take all the source folders from the config @@ -612,55 +633,19 @@ pub struct PackageMap { /// interface files. /// /// The two step process is there to reduce IO overhead. -/// `is_child` is true when the workspace_root rescript.json has the name of the root_folder rescript.json pub fn make( filter: &Option, - root_folder: &Path, - workspace_root: &Option, + project_context: &ProjectContext, show_progress: bool, build_dev_deps: bool, -) -> Result { - let mut root_is_child_of_workspace = false; - let map = match &workspace_root { - Some(wr) => { - // If the workspace root contains the root_folder as a dependency, - // it should be considered as related, and we read the workspace. - let root_name = read_package_name(root_folder)?; - let workspace_config = read_config(wr)?; - - root_is_child_of_workspace = workspace_config - .dependencies - .to_owned() - .unwrap_or_default() - .iter() - .any(|dep| dep == &root_name) - || workspace_config - .dev_dependencies - .to_owned() - .unwrap_or_default() - .iter() - .any(|dep| dep == &root_name); - - if root_is_child_of_workspace { - let root_folder_str = root_folder.to_string_lossy(); - let workspace_root_str = wr.to_string_lossy(); - log::info!("Building workspace: {workspace_root_str} for {root_folder_str}",); - read_packages(wr, workspace_root, show_progress, build_dev_deps)? - } else { - read_packages(root_folder, workspace_root, show_progress, build_dev_deps)? - } - } - None => read_packages(root_folder, workspace_root, show_progress, build_dev_deps)?, - }; +) -> Result> { + let map = read_packages(project_context, show_progress, build_dev_deps)?; /* Once we have the deduplicated packages, we can add the source files for each - to minimize * the IO */ let result = extend_with_children(filter, map, build_dev_deps); - Ok(PackageMap { - packages: result, - root_is_child_of_workspace, - }) + Ok(result) } pub fn parse_packages(build_state: &mut BuildState) { @@ -678,7 +663,7 @@ pub fn parse_packages(build_state: &mut BuildState) { helpers::create_path(&build_path_abs); helpers::create_path(&bs_build_path); let root_config = build_state - .get_package(&build_state.root_config_name) + .get_package(&build_state.get_root_package().name) .expect("cannot find root config"); root_config.config.get_package_specs().iter().for_each(|spec| { diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 67908e772e..933916309a 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -5,6 +5,7 @@ use super::packages; use crate::config; use crate::config::OneOrMore; use crate::helpers; +use crate::project_context::ProjectContext; use ahash::AHashSet; use log::debug; use rayon::prelude::*; @@ -38,8 +39,6 @@ pub fn generate_asts( } SourceType::SourceFile(source_file) => { - let root_package = build_state.get_package(&build_state.root_config_name).unwrap(); - let (ast_result, iast_result, dirty) = if source_file.implementation.parse_dirty || source_file .interface @@ -50,21 +49,15 @@ pub fn generate_asts( inc(); let ast_result = generate_ast( package.to_owned(), - root_package.to_owned(), &source_file.implementation.path.to_owned(), - &build_state.bsc_path, - &build_state.workspace_root, + build_state, ); let iast_result = match source_file.interface.as_ref().map(|i| i.path.to_owned()) { - Some(interface_file_path) => generate_ast( - package.to_owned(), - root_package.to_owned(), - &interface_file_path.to_owned(), - &build_state.bsc_path, - &build_state.workspace_root, - ) - .map(Some), + Some(interface_file_path) => { + generate_ast(package.to_owned(), &interface_file_path.to_owned(), build_state) + .map(Some) + } _ => Ok(None), }; @@ -240,29 +233,25 @@ pub fn generate_asts( } pub fn parser_args( - config: &config::Config, - root_config: &config::Config, + project_context: &ProjectContext, filename: &Path, - workspace_root: &Option, - root_path: &Path, contents: &str, ) -> (PathBuf, Vec) { + let current_config = project_context.get_current_rescript_config(); + let root_config = project_context.get_root_config(); let file = &filename; let ast_path = helpers::get_ast_path(file); + let node_modules_path = project_context.get_root_path().join("node_modules"); let ppx_flags = config::flatten_ppx_flags( - &if let Some(workspace_root) = workspace_root { - workspace_root.join("node_modules") - } else { - root_path.join("node_modules") - }, - &filter_ppx_flags(&config.ppx_flags, contents), - &config.name, + node_modules_path.as_path(), + &filter_ppx_flags(¤t_config.ppx_flags, contents), + ¤t_config.name, ); let jsx_args = root_config.get_jsx_args(); let jsx_module_args = root_config.get_jsx_module_args(); let jsx_mode_args = root_config.get_jsx_mode_args(); let jsx_preserve_args = root_config.get_jsx_preserve_args(); - let bsc_flags = config::flatten_flags(&config.compiler_flags); + let bsc_flags = config::flatten_flags(¤t_config.compiler_flags); let file = PathBuf::from("..").join("..").join(file); @@ -289,23 +278,14 @@ pub fn parser_args( fn generate_ast( package: packages::Package, - root_package: packages::Package, filename: &Path, - bsc_path: &PathBuf, - workspace_root: &Option, + build_state: &BuildState, ) -> Result<(PathBuf, Option), String> { let file_path = PathBuf::from(&package.path).join(filename); let contents = helpers::read_file(&file_path).expect("Error reading file"); let build_path_abs = package.get_build_path(); - let (ast_path, parser_args) = parser_args( - &package.config, - &root_package.config, - filename, - workspace_root, - &root_package.path, - &contents, - ); + let (ast_path, parser_args) = parser_args(&build_state.project_context, filename, &contents); // generate the dir of the ast_path (it mirrors the source file dir) let ast_parent_path = package.get_build_path().join(ast_path.parent().unwrap()); @@ -313,7 +293,7 @@ fn generate_ast( /* Create .ast */ let result = match Some( - Command::new(bsc_path) + Command::new(&build_state.bsc_path) .current_dir(&build_path_abs) .args(parser_args) .output() diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index 0f516fc6d0..2a56167c33 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -79,10 +79,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { "iast" | "ast" => { let module_name = helpers::file_path_to_module_name(path, package_namespace); - let root_package = build_state - .packages - .get(&build_state.root_config_name) - .expect("Could not find root package"); + let root_package = build_state.get_root_package(); if let Some(res_file_path_buf) = get_res_path_from_ast(path) { let _ = ast_modules.insert( res_file_path_buf.clone(), diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 0870f603ea..9d9002988e 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -181,10 +181,6 @@ pub enum Command { }, /// Formats ReScript files. Format { - /// Format the whole project. - #[arg(short, long, group = "format_input_mode")] - all: bool, - /// Check formatting status without applying changes. #[arg(short, long)] check: bool, @@ -199,8 +195,8 @@ pub enum Command { )] stdin: Option, - /// Files to format. - #[arg(group = "format_input_mode", required_unless_present_any = ["format_input_mode"])] + /// Files to format. If no files are provided, all files are formatted. + #[arg(group = "format_input_mode")] files: Vec, }, /// This prints the compiler arguments. It expects the path to a rescript file (.res or .resi). diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index b5078231ff..195daa1d17 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -1,4 +1,4 @@ -use crate::helpers; +use crate::{helpers, project_context}; use anyhow::{Result, bail}; use num_cpus; use rayon::prelude::*; @@ -12,12 +12,7 @@ use crate::build::packages; use crate::cli::FileExtension; use clap::ValueEnum; -pub fn format( - stdin_extension: Option, - all: bool, - check: bool, - files: Vec, -) -> Result<()> { +pub fn format(stdin_extension: Option, check: bool, files: Vec) -> Result<()> { let bsc_path = helpers::get_bsc(); match stdin_extension { @@ -25,7 +20,11 @@ pub fn format( format_stdin(&bsc_path, extension)?; } None => { - let files = if all { get_all_files()? } else { files }; + let files = if files.is_empty() { + get_files_in_scope()? + } else { + files + }; format_files(&bsc_path, files, check)?; } } @@ -33,18 +32,17 @@ pub fn format( Ok(()) } -fn get_all_files() -> Result> { +fn get_files_in_scope() -> Result> { let current_dir = std::env::current_dir()?; - let project_root = helpers::get_abs_path(¤t_dir); - let workspace_root_option = helpers::get_workspace_root(&project_root); + let project_context = project_context::ProjectContext::new(¤t_dir)?; - let packages::PackageMap { packages, .. } = - packages::make(&None, &project_root, &workspace_root_option, false, false)?; + let packages = packages::make(&None, &project_context, false, false)?; let mut files: Vec = Vec::new(); + let packages_to_format = project_context.get_scoped_local_packages(); for (_package_name, package) in packages { - if package.is_local_dep - && let Some(source_files) = package.source_files + if packages_to_format.contains(&package.name) + && let Some(source_files) = &package.source_files { for (path, _metadata) in source_files { if let Some(extension) = path.extension() { diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index 456a43cdae..c3d7a8e1c1 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -1,4 +1,5 @@ use crate::build::packages; +use crate::project_context::ProjectContext; use std::ffi::OsString; use std::fs; use std::fs::File; @@ -191,27 +192,17 @@ pub fn get_bsc() -> PathBuf { .to_stripped_verbatim_path() } -pub fn get_rescript_legacy(root_path: &Path, workspace_root: Option) -> PathBuf { +pub fn get_rescript_legacy(project_context: &ProjectContext) -> PathBuf { let bin_dir = Path::new("node_modules").join("rescript").join("cli"); - match ( - root_path - .join(&bin_dir) - .join("rescript-legacy.js") - .canonicalize() - .map(StrippedVerbatimPath::to_stripped_verbatim_path), - workspace_root.map(|workspace_root| { - workspace_root - .join(&bin_dir) - .join("rescript-legacy.js") - .canonicalize() - .map(StrippedVerbatimPath::to_stripped_verbatim_path) - }), - ) { - (Ok(path), _) => path, - (_, Some(Ok(path))) => path, - _ => panic!("Could not find rescript-legacy.exe"), - } + let root_path = project_context.get_root_path(); + let rescript_legacy_path = root_path + .join(&bin_dir) + .join("rescript-legacy.js") + .canonicalize() + .map(StrippedVerbatimPath::to_stripped_verbatim_path); + + rescript_legacy_path.unwrap_or_else(|_| panic!("Could not find rescript-legacy.exe")) } pub fn string_ends_with_any(s: &Path, suffixes: &[&str]) -> bool { @@ -357,12 +348,6 @@ fn has_rescript_config(path: &Path) -> bool { path.join("bsconfig.json").exists() || path.join("rescript.json").exists() } -pub fn get_workspace_root(package_root: &Path) -> Option { - std::path::PathBuf::from(&package_root) - .parent() - .and_then(get_nearest_config) -} - // traverse up the directory tree until we find a config.json, if not return None pub fn get_nearest_config(path_buf: &Path) -> Option { let mut current_dir = path_buf.to_owned(); @@ -390,3 +375,15 @@ pub fn get_source_file_from_rescript_file(path: &Path, suffix: &str) -> PathBuf &suffix.to_string()[1..], ) } + +/// Accepts two absolute paths and returns true if the parent is a subpath of the child. +/// Excluding node_modules in the child path. +pub fn is_subpath(parent: &Path, child: &Path) -> bool { + parent.starts_with(child) && parent.components().count() == child.components().count() + 1 +} +pub fn is_local_package(workspace_path: &Path, canonical_package_path: &Path) -> bool { + canonical_package_path.starts_with(workspace_path) + && !canonical_package_path + .components() + .any(|c| c.as_os_str() == "node_modules") +} diff --git a/rewatch/src/lib.rs b/rewatch/src/lib.rs index 5b24c38886..a389e8172e 100644 --- a/rewatch/src/lib.rs +++ b/rewatch/src/lib.rs @@ -5,6 +5,7 @@ pub mod config; pub mod format; pub mod helpers; pub mod lock; +pub mod project_context; pub mod queue; pub mod sourcedirs; pub mod watcher; diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 7ae9ae0884..8b5a85b687 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -91,12 +91,7 @@ fn main() -> Result<()> { let code = build::pass_through_legacy(legacy_args); std::process::exit(code); } - cli::Command::Format { - stdin, - all, - check, - files, - } => format::format(stdin, all, check, files), + cli::Command::Format { stdin, check, files } => format::format(stdin, check, files), } } diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs new file mode 100644 index 0000000000..f01214a12d --- /dev/null +++ b/rewatch/src/project_context.rs @@ -0,0 +1,251 @@ +use crate::build::packages; +use crate::config::Config; +use crate::helpers; +use ahash::AHashSet; +use anyhow::Result; +use anyhow::anyhow; +use log::debug; +use std::fmt; +use std::path::{Path, PathBuf}; + +type RescriptJsonPath = PathBuf; + +/// Represents the context of a command, categorizing in various ways: +/// - SingleProject: one rescript.json with no parent or children. +/// - MonorepoRoot: a rescript.json with (dev-)dependencies found in subfolders. +/// - MonorepoPackage: a rescript.json with a parent rescript.json. +/// The current package name is listed in the parent (dev-)dependencies. +// I don't think this linting rule matters very much as we only create a single instance of this enum. +// See https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant +#[allow(clippy::large_enum_variant)] +pub enum ProjectContext { + /// Single project - no workspace relationship + SingleProject { config: Config, path: RescriptJsonPath }, + /// Monorepo root - contains local dependencies (symlinked in node_modules) + MonorepoRoot { + config: Config, + path: RescriptJsonPath, + local_dependencies: AHashSet, // names of local deps + }, + /// Package within a monorepo - has a parent workspace + MonorepoPackage { + config: Config, + // This field is only used for debugging purposes, maybe it could be removed in the future. + path: RescriptJsonPath, + parent_config: Config, + parent_path: RescriptJsonPath, + }, +} + +impl fmt::Debug for ProjectContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self { + ProjectContext::SingleProject { config, path } => { + write!( + f, + "SingleProject: \"{}\" at \"{}\"", + config.name, + path.to_string_lossy() + ) + } + ProjectContext::MonorepoRoot { + config, + path, + local_dependencies, + } => { + let deps = format!( + "[{}]", + local_dependencies + .iter() + .map(|dep| format!("\"{dep}\"")) + .collect::>() + .join(", ") + ); + write!( + f, + "MonorepoRoot: \"{}\" at \"{}\" with {}", + config.name, + path.to_string_lossy(), + deps + ) + } + ProjectContext::MonorepoPackage { + config, + path, + parent_config, + parent_path, + } => { + write!( + f, + "MonorepoPackage:\n \"{}\" at \"{}\"\n with parent \"{}\" at \"{}\"", + config.name, + path.to_string_lossy(), + parent_config.name, + parent_path.to_string_lossy() + ) + } + } + } +} +fn read_local_packages(folder_path: &Path, config: &Config) -> Result> { + let mut local_dependencies: AHashSet = AHashSet::::new(); + let mut dependencies = AHashSet::::new(); + for dep in config.dependencies.as_ref().unwrap_or(&vec![]) { + dependencies.insert(dep.clone()); + } + for dev_dep in config.dev_dependencies.as_ref().unwrap_or(&vec![]) { + dependencies.insert(dev_dep.clone()); + } + + for dep in dependencies { + // Monorepo packages are expected to be symlinked in node_modules. + if let Ok(dep_path) = folder_path.join("node_modules").join(&dep).canonicalize() + && helpers::is_local_package(folder_path, &dep_path) + { + local_dependencies.insert(dep.to_string()); + } + } + + Ok(local_dependencies) +} + +impl ProjectContext { + pub fn new(path: &Path) -> Result { + let path = helpers::get_abs_path(path); + let (current_config, current_rescript_json) = packages::read_config(&path) + .map_err(|_| anyhow!("Could not read rescript.json at {}", path.to_string_lossy()))?; + let nearest_parent_config_path = match path.parent() { + None => Err(anyhow!( + "The current path \"{}\" does not have a parent folder", + path.to_string_lossy() + )), + Some(parent) => Ok(helpers::get_nearest_config(parent)), + }?; + let context = match nearest_parent_config_path { + None => { + let local_dependencies = read_local_packages(&path, ¤t_config)?; + if local_dependencies.is_empty() { + Ok(ProjectContext::SingleProject { + config: current_config, + path: current_rescript_json.clone(), + }) + } else { + Ok(ProjectContext::MonorepoRoot { + config: current_config, + path: current_rescript_json.clone(), + local_dependencies, + }) + } + } + Some(parent_config_path) => { + match packages::read_config(parent_config_path.as_path()) { + Err(e) => Err(anyhow!( + "Could not read the parent config at {}: {}", + parent_config_path.to_string_lossy(), + e + )), + Ok((workspace_config, workspace_rescript_json)) => { + let is_current_config_listed_in_workspace = workspace_config + .dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == ¤t_config.name) + || workspace_config + .dev_dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == ¤t_config.name); + + if is_current_config_listed_in_workspace { + // There is a parent rescript.json, and it has a reference to the current package. + Ok(ProjectContext::MonorepoPackage { + config: current_config, + path: path.join("rescript.json"), + parent_config: workspace_config, + parent_path: workspace_rescript_json.clone(), + }) + } else { + // There is a parent rescript.json, but it has no reference to the current package. + Ok(ProjectContext::SingleProject { + config: current_config, + path: current_rescript_json.clone(), + }) + } + } + } + } + }; + context.iter().for_each(|pc| { + debug!("Created project context {:#?} for \"{}\"", pc, path.display()); + }); + context + } + + pub fn get_root_config(&self) -> &Config { + match self { + ProjectContext::SingleProject { config, .. } => config, + ProjectContext::MonorepoRoot { config, .. } => config, + ProjectContext::MonorepoPackage { + parent_config: config, + .. + } => config, + } + } + + pub fn get_root_path(&self) -> &Path { + let rescript_json = match self { + ProjectContext::SingleProject { path, .. } => path, + ProjectContext::MonorepoRoot { path, .. } => path, + ProjectContext::MonorepoPackage { + parent_path: path, .. + } => path, + }; + rescript_json.parent().unwrap() + } + + pub fn get_suffix(&self) -> String { + match self { + ProjectContext::SingleProject { config, .. } + | ProjectContext::MonorepoRoot { config, .. } + | ProjectContext::MonorepoPackage { + parent_config: config, + .. + } => config.suffix.clone().unwrap_or(String::from(".res.mjs")), + } + } + + /// Returns the local packages relevant for the current context. + /// Either a single project, all projects from a monorepo or a single package inside a monorepo. + pub fn get_scoped_local_packages(&self) -> AHashSet { + let mut local_packages = AHashSet::::new(); + match &self { + ProjectContext::SingleProject { config, .. } => { + local_packages.insert(config.name.clone()); + } + ProjectContext::MonorepoRoot { + config, + local_dependencies, + .. + } => { + local_packages.insert(config.name.clone()); + for dep in local_dependencies { + local_packages.insert(dep.clone()); + } + } + ProjectContext::MonorepoPackage { config, .. } => { + local_packages.insert(config.name.clone()); + } + }; + local_packages + } + + pub fn get_current_rescript_config(&self) -> &Config { + match &self { + ProjectContext::SingleProject { config, .. } + | ProjectContext::MonorepoRoot { config, .. } + | ProjectContext::MonorepoPackage { config, .. } => config, + } + } +} From 860e6e2c1f86b737035ef4c9d1f5d21746b2092e Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 9 Aug 2025 20:48:26 +0200 Subject: [PATCH 10/47] A rescript.json with an unrelated parent can still be a monorepo. --- rewatch/src/project_context.rs | 63 +++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index f01214a12d..72e126f841 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -53,14 +53,19 @@ impl fmt::Debug for ProjectContext { path, local_dependencies, } => { - let deps = format!( - "[{}]", - local_dependencies - .iter() - .map(|dep| format!("\"{dep}\"")) - .collect::>() - .join(", ") - ); + let deps = if local_dependencies.is_empty() { + String::from("[]") + } else { + format!( + "[\n{}\n]", + local_dependencies + .iter() + .map(|dep| format!(" \"{dep}\"")) + .collect::>() + .join(",\n") + ) + }; + write!( f, "MonorepoRoot: \"{}\" at \"{}\" with {}", @@ -109,6 +114,26 @@ fn read_local_packages(folder_path: &Path, config: &Config) -> Result Result { + let local_dependencies = read_local_packages(path, ¤t_config)?; + if local_dependencies.is_empty() { + Ok(ProjectContext::SingleProject { + config: current_config, + path: current_rescript_json.clone(), + }) + } else { + Ok(ProjectContext::MonorepoRoot { + config: current_config, + path: current_rescript_json.clone(), + local_dependencies, + }) + } +} + impl ProjectContext { pub fn new(path: &Path) -> Result { let path = helpers::get_abs_path(path); @@ -122,21 +147,7 @@ impl ProjectContext { Some(parent) => Ok(helpers::get_nearest_config(parent)), }?; let context = match nearest_parent_config_path { - None => { - let local_dependencies = read_local_packages(&path, ¤t_config)?; - if local_dependencies.is_empty() { - Ok(ProjectContext::SingleProject { - config: current_config, - path: current_rescript_json.clone(), - }) - } else { - Ok(ProjectContext::MonorepoRoot { - config: current_config, - path: current_rescript_json.clone(), - local_dependencies, - }) - } - } + None => monorepo_or_single_project(&path, current_rescript_json, current_config), Some(parent_config_path) => { match packages::read_config(parent_config_path.as_path()) { Err(e) => Err(anyhow!( @@ -168,10 +179,8 @@ impl ProjectContext { }) } else { // There is a parent rescript.json, but it has no reference to the current package. - Ok(ProjectContext::SingleProject { - config: current_config, - path: current_rescript_json.clone(), - }) + // However, the current package could still be a monorepo root! + monorepo_or_single_project(&path, current_rescript_json, current_config) } } } From 820231597c7a271f115c20750daefb526de8e219 Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 9 Aug 2025 20:54:40 +0200 Subject: [PATCH 11/47] Remove all from format command. --- rewatch/tests/format.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/tests/format.sh b/rewatch/tests/format.sh index ccfef0f4e4..ef77351ba1 100755 --- a/rewatch/tests/format.sh +++ b/rewatch/tests/format.sh @@ -4,7 +4,7 @@ cd ../testrepo bold "Test: It should format all files" git diff --name-only ./ -error_output=$("$REWATCH_EXECUTABLE" format --all) +error_output=$("$REWATCH_EXECUTABLE" format) git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) if [ $? -eq 0 ] && [ $git_diff_file_count -eq 8 ]; then From c2c918392b8f7d032f336364ecbad8d47e4f3827 Mon Sep 17 00:00:00 2001 From: nojaf Date: Sat, 9 Aug 2025 21:21:51 +0200 Subject: [PATCH 12/47] get_rescript_legacy inside rescript repository. --- rewatch/src/helpers.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index c3d7a8e1c1..b3115f528d 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -193,14 +193,23 @@ pub fn get_bsc() -> PathBuf { } pub fn get_rescript_legacy(project_context: &ProjectContext) -> PathBuf { - let bin_dir = Path::new("node_modules").join("rescript").join("cli"); - let root_path = project_context.get_root_path(); - let rescript_legacy_path = root_path - .join(&bin_dir) - .join("rescript-legacy.js") - .canonicalize() - .map(StrippedVerbatimPath::to_stripped_verbatim_path); + let node_modules_rescript = root_path.join("node_modules").join("rescript"); + let rescript_legacy_path = if node_modules_rescript.exists() { + node_modules_rescript + .join("cli") + .join("rescript-legacy.js") + .canonicalize() + .map(StrippedVerbatimPath::to_stripped_verbatim_path) + } else { + // If the root folder / node_modules doesn't exist, something is wrong. + // The only way this can happen is if we are inside the rescript repository. + root_path + .join("cli") + .join("rescript-legacy.js") + .canonicalize() + .map(StrippedVerbatimPath::to_stripped_verbatim_path) + }; rescript_legacy_path.unwrap_or_else(|_| panic!("Could not find rescript-legacy.exe")) } @@ -376,11 +385,6 @@ pub fn get_source_file_from_rescript_file(path: &Path, suffix: &str) -> PathBuf ) } -/// Accepts two absolute paths and returns true if the parent is a subpath of the child. -/// Excluding node_modules in the child path. -pub fn is_subpath(parent: &Path, child: &Path) -> bool { - parent.starts_with(child) && parent.components().count() == child.components().count() + 1 -} pub fn is_local_package(workspace_path: &Path, canonical_package_path: &Path) -> bool { canonical_package_path.starts_with(workspace_path) && !canonical_package_path From 50baa0b1f29c023a8c43386510deb1217e42a553 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Sat, 9 Aug 2025 19:46:41 +0000 Subject: [PATCH 13/47] sigh From c19670e6a2ef9791464bc86ffbc4353a6e5bd7f0 Mon Sep 17 00:00:00 2001 From: nojaf Date: Sun, 10 Aug 2025 11:28:12 +0200 Subject: [PATCH 14/47] StrippedVerbatimPath in before local dep check --- rewatch/src/project_context.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 72e126f841..f6dcd4ff57 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -104,10 +104,20 @@ fn read_local_packages(folder_path: &Path, config: &Config) -> Result Date: Sun, 10 Aug 2025 10:07:47 +0000 Subject: [PATCH 15/47] Use proper --version --- rewatch/tests/suite-ci.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rewatch/tests/suite-ci.sh b/rewatch/tests/suite-ci.sh index 76cc50b21c..5b15a1b34b 100755 --- a/rewatch/tests/suite-ci.sh +++ b/rewatch/tests/suite-ci.sh @@ -18,8 +18,11 @@ export RESCRIPT_BSC_EXE source ./utils.sh +bold "Yarn install" +(cd ../testrepo && yarn) + bold "Rescript version" -(cd ../testrepo && ./node_modules/.bin/rescript -v) +(cd ../testrepo && ./node_modules/.bin/rescript --version) # we need to reset the yarn.lock and package.json to the original state # so there is not diff in git. The CI will install new ReScript package From 43097c8ea312df0de21b4f140bb692e0db747f27 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Sun, 10 Aug 2025 13:20:58 +0000 Subject: [PATCH 16/47] Add playground to monorepo --- rescript.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rescript.json b/rescript.json index 1f566dcf3d..2aa506b7b6 100644 --- a/rescript.json +++ b/rescript.json @@ -1,4 +1,4 @@ { "name": "rescript", - "dependencies": ["@tests/gentype-react-example"] + "dependencies": ["@tests/gentype-react-example", "playground"] } From c740f4ea095ac825542174199ba746b59007385c Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 12:57:22 +0200 Subject: [PATCH 17/47] Copilot nitpicks --- rewatch/src/build/clean.rs | 4 ++-- rewatch/src/build/packages.rs | 23 +---------------------- rewatch/src/project_context.rs | 2 +- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index db2448ed8a..f3572ce15d 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -33,7 +33,7 @@ fn remove_iast(package: &packages::Package, source_file: &Path) { fn remove_mjs_file(source_file: &Path, suffix: &str) { let _ = std::fs::remove_file(source_file.with_extension( // suffix.to_string includes the ., so we need to remove it - &suffix.to_string()[1..], + &suffix[1..], )); } @@ -60,7 +60,7 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &String) { +fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &str) { let packages_to_clean = build_state.project_context.get_scoped_local_packages(); // get all rescript file locations diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 44dc6dafe9..2524172337 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -253,27 +253,6 @@ pub fn read_config(package_dir: &Path) -> Result<(Config, PathBuf)> { pub fn read_dependency(package_name: &str, project_context: &ProjectContext) -> Result { // root folder + node_modules + package_name let path_from_root = helpers::package_path(project_context.get_root_path(), package_name); - - // let path_from_project_root = helpers::package_path(project_root, package_name); - // let maybe_path_from_workspace_root = workspace_root - // .as_ref() - // .map(|workspace_root| helpers::package_path(workspace_root, package_name)); - - // let path = match ( - // path_from_parent, - // path_from_project_root, - // maybe_path_from_workspace_root, - // ) { - // (path_from_parent, _, _) if path_from_parent.exists() => Ok(path_from_parent), - // (_, path_from_project_root, _) if path_from_project_root.exists() => Ok(path_from_project_root), - // (_, _, Some(path_from_workspace_root)) if path_from_workspace_root.exists() => { - // Ok(path_from_workspace_root) - // } - // _ => Err(format!( - // "The package \"{package_name}\" is not found (are node_modules up-to-date?)..." - // )), - // }?; - let path = (if path_from_root.exists() { Ok(path_from_root) } else { @@ -332,7 +311,7 @@ fn read_dependencies( // Read all config files in parallel instead of blocking .par_iter() .map(|package_name| { - let ((config, _), canonical_path) = + let ((config, _config_path), canonical_path) = match read_dependency(package_name, project_context) { Err(error) => { if show_progress { diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index f6dcd4ff57..4a9bab5bcf 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -93,7 +93,7 @@ impl fmt::Debug for ProjectContext { } } fn read_local_packages(folder_path: &Path, config: &Config) -> Result> { - let mut local_dependencies: AHashSet = AHashSet::::new(); + let mut local_dependencies = AHashSet::::new(); let mut dependencies = AHashSet::::new(); for dep in config.dependencies.as_ref().unwrap_or(&vec![]) { dependencies.insert(dep.clone()); From e5b723cba1e940a2c4a273f12fdc8f1cf7de605b Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 13:12:53 +0200 Subject: [PATCH 18/47] Add test for only formatting the current project. --- rewatch/tests/format.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/rewatch/tests/format.sh b/rewatch/tests/format.sh index ef77351ba1..4beb240bf5 100755 --- a/rewatch/tests/format.sh +++ b/rewatch/tests/format.sh @@ -42,3 +42,18 @@ else echo $error_output exit 1 fi + +bold "Test: It should format only the current project" + +error_output=$(cd packages/file-casing && "../../$REWATCH_EXECUTABLE" format) +git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) +if [ $? -eq 0 ] && [ $git_diff_file_count -eq 2 ]; +then + success "file-casing formatted" + git restore . +else + error "Error formatting current project file-casing" + echo "Expected 2 files to be changed, got $git_diff_file_count" + echo $error_output + exit 1 +fi \ No newline at end of file From 7b9b230085c1d91e730f33c3a81672556ae6829e Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 14:05:14 +0200 Subject: [PATCH 19/47] Add clean single project test --- rewatch/tests/clean.sh | 49 +++++++++++++++++++++++++++++++++++++++ rewatch/tests/suite-ci.sh | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100755 rewatch/tests/clean.sh diff --git a/rewatch/tests/clean.sh b/rewatch/tests/clean.sh new file mode 100755 index 0000000000..bf3b27bd5d --- /dev/null +++ b/rewatch/tests/clean.sh @@ -0,0 +1,49 @@ +#!/bin/bash +cd $(dirname $0) +source "./utils.sh" +cd ../testrepo + +bold "Test: It should clean a single project" + +# First we build the entire monorepo +error_output=$(rewatch build 2>&1) +if [ $? -eq 0 ]; +then + success "Built monorepo" +else + error "Error building monorepo" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Then we clean a single project +error_output=$(cd packages/file-casing && "../../$REWATCH_EXECUTABLE" clean 2>&1) +clean_status=$? +if [ $clean_status -ne 0 ]; +then + error "Error cleaning current project file-casing" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Count compiled files in the cleaned project +project_compiled_files=$(find packages/file-casing -type f -name '*.mjs' | wc -l | tr -d '[:space:]') +if [ "$project_compiled_files" -eq 0 ]; +then + success "file-casing cleaned" +else + error "Expected 0 .mjs files in file-casing after clean, got $project_compiled_files" + printf "%s\n" "$error_output" + exit 1 +fi + +# Ensure other project files were not cleaned +other_project_compiled_files=$(find packages/new-namespace -type f -name '*.mjs' | wc -l | tr -d '[:space:]') +if [ "$other_project_compiled_files" -gt 0 ]; +then + success "Didn't clean other project files" + git restore . +else + error "Expected files from new-namespace not to be cleaned" + exit 1 +fi \ No newline at end of file diff --git a/rewatch/tests/suite-ci.sh b/rewatch/tests/suite-ci.sh index 5b15a1b34b..a16457d0ce 100755 --- a/rewatch/tests/suite-ci.sh +++ b/rewatch/tests/suite-ci.sh @@ -43,4 +43,4 @@ else exit 1 fi -./compile.sh && ./watch.sh && ./lock.sh && ./suffix.sh && ./legacy.sh && ./format.sh +./compile.sh && ./watch.sh && ./lock.sh && ./suffix.sh && ./legacy.sh && ./format.sh && ./clean.sh \ No newline at end of file From f76b55b9ddc8b5c19c224ca969409705a664d253 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 15:19:53 +0200 Subject: [PATCH 20/47] Add test for compiler-args --- rewatch/tests/compiler-args.sh | 36 ++++++++++++++++++++++++++++++++++ rewatch/tests/suite-ci.sh | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100755 rewatch/tests/compiler-args.sh diff --git a/rewatch/tests/compiler-args.sh b/rewatch/tests/compiler-args.sh new file mode 100755 index 0000000000..bbe455bfc0 --- /dev/null +++ b/rewatch/tests/compiler-args.sh @@ -0,0 +1,36 @@ +#!/bin/bash +cd $(dirname $0) +source "./utils.sh" +cd ../testrepo + +bold "Test: It should not matter where we grab the compiler-args for a file" +# Capture stdout for both invocations +stdout_root=$(rewatch compiler-args packages/file-casing/src/Consume.res 2>/dev/null) +stdout_pkg=$(cd packages/file-casing && "../../$REWATCH_EXECUTABLE" compiler-args src/Consume.res 2>/dev/null) + +error_output=$(rewatch compiler-args packages/file-casing/src/Consume.res 2>&1) +if [ $? -ne 0 ]; then + error "Error grabbing compiler args for packages/file-casing/src/Consume.res" + printf "%s\n" "$error_output" >&2 + exit 1 +fi +error_output=$(cd packages/file-casing && "../../$REWATCH_EXECUTABLE" compiler-args src/Consume.res 2>&1) +if [ $? -ne 0 ]; then + error "Error grabbing compiler args for src/Consume.res in packages/file-casing" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Compare the stdout of both runs; must be exactly identical +tmp1=$(mktemp); tmp2=$(mktemp) +trap 'rm -f "$tmp1" "$tmp2"' EXIT +printf "%s" "$stdout_root" > "$tmp1" +printf "%s" "$stdout_pkg" > "$tmp2" +if git diff --no-index --exit-code "$tmp1" "$tmp2" > /dev/null; then + success "compiler-args stdout is identical regardless of cwd" +else + error "compiler-args stdout differs depending on cwd" + echo "---- diff ----" + git diff --no-index "$tmp1" "$tmp2" || true + exit 1 +fi diff --git a/rewatch/tests/suite-ci.sh b/rewatch/tests/suite-ci.sh index a16457d0ce..fc1d615643 100755 --- a/rewatch/tests/suite-ci.sh +++ b/rewatch/tests/suite-ci.sh @@ -43,4 +43,4 @@ else exit 1 fi -./compile.sh && ./watch.sh && ./lock.sh && ./suffix.sh && ./legacy.sh && ./format.sh && ./clean.sh \ No newline at end of file +./compile.sh && ./watch.sh && ./lock.sh && ./suffix.sh && ./legacy.sh && ./format.sh && ./clean.sh && ./compiler-args.sh \ No newline at end of file From f993a713668d369eb967b8040f3626a17f98f2a7 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 15:35:43 +0200 Subject: [PATCH 21/47] Get root config from project_context --- rewatch/src/build/packages.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 2524172337..50bedb15b9 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -655,11 +655,9 @@ pub fn parse_packages(build_state: &mut BuildState) { let bs_build_path = package.get_ocaml_build_path(); helpers::create_path(&build_path_abs); helpers::create_path(&bs_build_path); - let root_config = build_state - .get_package(&build_state.get_root_package().name) - .expect("cannot find root config"); + let root_config = build_state.project_context.get_root_config(); - root_config.config.get_package_specs().iter().for_each(|spec| { + root_config.get_package_specs().iter().for_each(|spec| { if !spec.in_source { // we don't want to calculate this if we don't have out of source specs // we do this twice, but we almost never have multiple package specs From 21b27586a3f0c4b132eba0a4cd08c36afa059719 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 15:49:12 +0200 Subject: [PATCH 22/47] Return Result for get_root_package --- rewatch/src/build.rs | 2 +- rewatch/src/build/build_types.rs | 11 +++-------- rewatch/src/build/clean.rs | 2 +- rewatch/src/build/compile.rs | 15 ++++++++------- rewatch/src/build/read_compile_state.rs | 9 +++++---- 5 files changed, 18 insertions(+), 21 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 9833cea25f..ca49214a57 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -174,7 +174,7 @@ pub fn initialize_build( let _ = stdout().flush(); } let timing_compile_state = Instant::now(); - let compile_assets_state = read_compile_state::read(&mut build_state); + let compile_assets_state = read_compile_state::read(&mut build_state)?; let timing_compile_state_elapsed = timing_compile_state.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/build_types.rs b/rewatch/src/build/build_types.rs index 1d32bfae38..39f39c0354 100644 --- a/rewatch/src/build/build_types.rs +++ b/rewatch/src/build/build_types.rs @@ -1,6 +1,7 @@ use crate::build::packages::{Namespace, Package}; use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; +use anyhow::{Result, anyhow}; use std::{fmt::Display, path::PathBuf, time::SystemTime}; #[derive(Debug, Clone, PartialEq)] @@ -128,10 +129,7 @@ impl BuildState { self.module_names.insert(module_name.to_owned()); } - // TODO: Consider returing a result? - // At the same time, the packages have been processed in packages::make - // It would have already failed if the root is absent - pub fn get_root_package(&self) -> &Package { + pub fn get_root_package(&self) -> Result<&Package> { match &self.project_context { ProjectContext::SingleProject { config, .. } | ProjectContext::MonorepoRoot { config, .. } @@ -141,10 +139,7 @@ impl BuildState { } => self .packages .get(&config.name) - // You will be tempted to point this out as "don't panic" during code review. - // Please don't and see https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call - // cargo clippy pointed this out. - .unwrap_or_else(|| panic!("Root package \"{}\" not found", &config.name)), + .ok_or_else(|| anyhow!("Root package \"{}\" not found", &config.name)), } } } diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index f3572ce15d..5037e7967c 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -409,7 +409,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let timing_clean_mjs = Instant::now(); let mut build_state = BuildState::new(project_context, packages, bsc_path); packages::parse_packages(&mut build_state); - let root_package = build_state.get_root_package(); + let root_package = build_state.get_root_package()?; let suffix = build_state.project_context.get_suffix(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index 0a4d43680b..f7b9341948 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -10,7 +10,7 @@ use crate::helpers; use crate::helpers::StrippedVerbatimPath; use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; -use anyhow::anyhow; +use anyhow::{Result, anyhow}; use console::style; use log::{debug, trace}; use rayon::prelude::*; @@ -551,14 +551,14 @@ fn compile_file( module: &Module, is_interface: bool, build_state: &BuildState, -) -> Result, String> { +) -> Result> { let BuildState { packages, project_context, bsc_path, .. } = build_state; - let root_package = build_state.get_root_package(); + let root_package = build_state.get_root_package()?; let ocaml_build_path_abs = package.get_ocaml_build_path(); let build_path_abs = package.get_build_path(); let implementation_file_path = match &module.source_type { @@ -568,7 +568,8 @@ fn compile_file( sourcetype, ast_path.to_string_lossy() )), - }?; + } + .map_err(|e| anyhow!(e))?; let basename = helpers::file_path_to_compiler_asset_basename(implementation_file_path, &package.namespace); let has_interface = module.get_interface().is_some(); @@ -600,9 +601,9 @@ fn compile_file( Ok(x) if !x.status.success() => { let stderr = String::from_utf8_lossy(&x.stderr); let stdout = String::from_utf8_lossy(&x.stdout); - Err(stderr.to_string() + &stdout) + Err(anyhow!(stderr.to_string() + &stdout)) } - Err(e) => Err(format!( + Err(e) => Err(anyhow!( "Could not compile file. Error: {e}. Path to AST: {ast_path:?}" )), Ok(x) => { @@ -610,7 +611,7 @@ fn compile_file( .expect("stdout should be non-null") .to_string(); - let dir = std::path::Path::new(implementation_file_path).parent().unwrap(); + let dir = Path::new(implementation_file_path).parent().unwrap(); // perhaps we can do this copying somewhere else if !is_interface { diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index 2a56167c33..0b5d58a9bb 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -7,7 +7,7 @@ use std::fs; use std::path::{Path, PathBuf}; use std::time::SystemTime; -pub fn read(build_state: &mut BuildState) -> CompileAssetsState { +pub fn read(build_state: &mut BuildState) -> anyhow::Result { let mut ast_modules: AHashMap = AHashMap::new(); let mut cmi_modules: AHashMap = AHashMap::new(); let mut cmt_modules: AHashMap = AHashMap::new(); @@ -73,13 +73,14 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { .flatten() .collect::>(); + let root_package = build_state.get_root_package()?; + compile_assets.iter().for_each( |(path, last_modified, extension, package_name, package_namespace, package_is_root)| { match extension.as_str() { "iast" | "ast" => { let module_name = helpers::file_path_to_module_name(path, package_namespace); - let root_package = build_state.get_root_package(); if let Some(res_file_path_buf) = get_res_path_from_ast(path) { let _ = ast_modules.insert( res_file_path_buf.clone(), @@ -123,13 +124,13 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { }, ); - CompileAssetsState { + Ok(CompileAssetsState { ast_modules, cmi_modules, cmt_modules, ast_rescript_file_locations, rescript_file_locations, - } + }) } fn get_res_path_from_ast(ast_file: &Path) -> Option { From 7892d7b64a107e4531db162fba442fe75e349c93 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:11:41 +0200 Subject: [PATCH 23/47] Make a conscious split between dev and non-dev local dependencies for MonorepoRoot. --- rewatch/src/build/clean.rs | 13 +++--- rewatch/src/build/packages.rs | 3 +- rewatch/src/cli.rs | 3 ++ rewatch/src/format.rs | 13 ++++-- rewatch/src/main.rs | 7 +++- rewatch/src/project_context.rs | 72 +++++++++++++++++++++------------- 6 files changed, 72 insertions(+), 39 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 5037e7967c..fbb09d27a5 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -60,8 +60,10 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &str) { - let packages_to_clean = build_state.project_context.get_scoped_local_packages(); +fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &str, clean_dev_deps: bool) { + let packages_to_clean = build_state + .project_context + .get_scoped_local_packages(clean_dev_deps); // get all rescript file locations let rescript_file_locations = build_state @@ -351,11 +353,10 @@ fn find_and_clean_package( } } -pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_deps: bool) -> Result<()> { +pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_deps: bool) -> Result<()> { let project_context = ProjectContext::new(path)?; - let packages = packages::make(&None, &project_context, show_progress, build_dev_deps)?; - // let root_config_name = packages::read_package_name(&project_root)?; + let packages = packages::make(&None, &project_context, show_progress, clean_dev_deps)?; let bsc_path = helpers::get_bsc(); let timing_clean_compiler_assets = Instant::now(); @@ -422,7 +423,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, build_dev_ let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, root_package, &suffix); + clean_source_files(&build_state, root_package, &suffix, clean_dev_deps); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 50bedb15b9..7fce2e9367 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -353,9 +353,10 @@ fn read_dependencies( , ProjectContext::MonorepoRoot { local_dependencies, + local_dev_dependencies, .. } => { - local_dependencies.contains(package_name) + local_dependencies.contains(package_name) || local_dev_dependencies.contains(package_name) } ProjectContext::MonorepoPackage { parent_path, diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 9d9002988e..5a66fe0df1 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -198,6 +198,9 @@ pub enum Command { /// Files to format. If no files are provided, all files are formatted. #[arg(group = "format_input_mode")] files: Vec, + + #[command(flatten)] + dev: DevArg, }, /// This prints the compiler arguments. It expects the path to a rescript file (.res or .resi). CompilerArgs { diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index 195daa1d17..5b33b77e02 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -12,7 +12,12 @@ use crate::build::packages; use crate::cli::FileExtension; use clap::ValueEnum; -pub fn format(stdin_extension: Option, check: bool, files: Vec) -> Result<()> { +pub fn format( + stdin_extension: Option, + check: bool, + files: Vec, + format_dev_deps: bool, +) -> Result<()> { let bsc_path = helpers::get_bsc(); match stdin_extension { @@ -21,7 +26,7 @@ pub fn format(stdin_extension: Option, check: bool, files: Vec { let files = if files.is_empty() { - get_files_in_scope()? + get_files_in_scope(format_dev_deps)? } else { files }; @@ -32,13 +37,13 @@ pub fn format(stdin_extension: Option, check: bool, files: Vec Result> { +fn get_files_in_scope(format_dev_deps: bool) -> Result> { let current_dir = std::env::current_dir()?; let project_context = project_context::ProjectContext::new(¤t_dir)?; let packages = packages::make(&None, &project_context, false, false)?; let mut files: Vec = Vec::new(); - let packages_to_format = project_context.get_scoped_local_packages(); + let packages_to_format = project_context.get_scoped_local_packages(format_dev_deps); for (_package_name, package) in packages { if packages_to_format.contains(&package.name) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 8b5a85b687..5fc08d4250 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -91,7 +91,12 @@ fn main() -> Result<()> { let code = build::pass_through_legacy(legacy_args); std::process::exit(code); } - cli::Command::Format { stdin, check, files } => format::format(stdin, check, files), + cli::Command::Format { + stdin, + check, + files, + dev, + } => format::format(stdin, check, files, dev.dev), } } diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 4a9bab5bcf..cabaa23988 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -26,6 +26,7 @@ pub enum ProjectContext { config: Config, path: RescriptJsonPath, local_dependencies: AHashSet, // names of local deps + local_dev_dependencies: AHashSet, }, /// Package within a monorepo - has a parent workspace MonorepoPackage { @@ -37,6 +38,21 @@ pub enum ProjectContext { }, } +fn format_dependencies(dependencies: &AHashSet) -> String { + if dependencies.is_empty() { + String::from("[]") + } else { + format!( + "[\n{}\n]", + dependencies + .iter() + .map(|dep| format!(" \"{dep}\"")) + .collect::>() + .join(",\n") + ) + } +} + impl fmt::Debug for ProjectContext { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self { @@ -52,26 +68,18 @@ impl fmt::Debug for ProjectContext { config, path, local_dependencies, + local_dev_dependencies, } => { - let deps = if local_dependencies.is_empty() { - String::from("[]") - } else { - format!( - "[\n{}\n]", - local_dependencies - .iter() - .map(|dep| format!(" \"{dep}\"")) - .collect::>() - .join(",\n") - ) - }; + let deps = format_dependencies(local_dependencies); + let dev_deps = format_dependencies(local_dev_dependencies); write!( f, - "MonorepoRoot: \"{}\" at \"{}\" with {}", + "MonorepoRoot: \"{}\" at \"{}\" with dependencies:\n {}\n and devDependencies:\n {}", config.name, path.to_string_lossy(), - deps + deps, + dev_deps ) } ProjectContext::MonorepoPackage { @@ -92,21 +100,17 @@ impl fmt::Debug for ProjectContext { } } } -fn read_local_packages(folder_path: &Path, config: &Config) -> Result> { +fn read_local_packages( + folder_path: &Path, + dependencies_from_config: &Vec, +) -> Result> { let mut local_dependencies = AHashSet::::new(); - let mut dependencies = AHashSet::::new(); - for dep in config.dependencies.as_ref().unwrap_or(&vec![]) { - dependencies.insert(dep.clone()); - } - for dev_dep in config.dev_dependencies.as_ref().unwrap_or(&vec![]) { - dependencies.insert(dev_dep.clone()); - } - for dep in dependencies { + for dep in dependencies_from_config { // Monorepo packages are expected to be symlinked in node_modules. if let Ok(dep_path) = folder_path .join("node_modules") - .join(&dep) + .join(dep) .canonicalize() .map(helpers::StrippedVerbatimPath::to_stripped_verbatim_path) { @@ -129,8 +133,15 @@ fn monorepo_or_single_project( current_rescript_json: RescriptJsonPath, current_config: Config, ) -> Result { - let local_dependencies = read_local_packages(path, ¤t_config)?; - if local_dependencies.is_empty() { + let local_dependencies = match ¤t_config.dependencies { + None => AHashSet::::new(), + Some(deps) => read_local_packages(path, deps)?, + }; + let local_dev_dependencies = match ¤t_config.dev_dependencies { + None => AHashSet::::new(), + Some(deps) => read_local_packages(path, deps)?, + }; + if local_dependencies.is_empty() && local_dev_dependencies.is_empty() { Ok(ProjectContext::SingleProject { config: current_config, path: current_rescript_json.clone(), @@ -140,6 +151,7 @@ fn monorepo_or_single_project( config: current_config, path: current_rescript_json.clone(), local_dependencies, + local_dev_dependencies, }) } } @@ -237,7 +249,7 @@ impl ProjectContext { /// Returns the local packages relevant for the current context. /// Either a single project, all projects from a monorepo or a single package inside a monorepo. - pub fn get_scoped_local_packages(&self) -> AHashSet { + pub fn get_scoped_local_packages(&self, include_dev_deps: bool) -> AHashSet { let mut local_packages = AHashSet::::new(); match &self { ProjectContext::SingleProject { config, .. } => { @@ -246,12 +258,18 @@ impl ProjectContext { ProjectContext::MonorepoRoot { config, local_dependencies, + local_dev_dependencies, .. } => { local_packages.insert(config.name.clone()); for dep in local_dependencies { local_packages.insert(dep.clone()); } + if include_dev_deps { + for dep in local_dev_dependencies { + local_packages.insert(dep.clone()); + } + } } ProjectContext::MonorepoPackage { config, .. } => { local_packages.insert(config.name.clone()); From 958fac3f516217c5929ebf66deb62be16303d574 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:25:08 +0200 Subject: [PATCH 24/47] Add dev project to root of test-repo --- rewatch/testrepo/package.json | 3 ++- rewatch/testrepo/packages/pure-dev/dev/RealDev.res | 1 + rewatch/testrepo/packages/pure-dev/package.json | 9 +++++++++ rewatch/testrepo/packages/pure-dev/rescript.json | 8 ++++++++ rewatch/testrepo/rescript.json | 3 +++ rewatch/testrepo/yarn.lock | 6 ++++++ 6 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 rewatch/testrepo/packages/pure-dev/dev/RealDev.res create mode 100644 rewatch/testrepo/packages/pure-dev/package.json create mode 100644 rewatch/testrepo/packages/pure-dev/rescript.json diff --git a/rewatch/testrepo/package.json b/rewatch/testrepo/package.json index f42be392d8..55c8e5d425 100644 --- a/rewatch/testrepo/package.json +++ b/rewatch/testrepo/package.json @@ -13,7 +13,8 @@ "packages/nonexisting-dev-files", "packages/deprecated-config", "packages/file-casing", - "packages/file-casing-no-namespace" + "packages/file-casing-no-namespace", + "packages/pure-dev" ] }, "dependencies": { diff --git a/rewatch/testrepo/packages/pure-dev/dev/RealDev.res b/rewatch/testrepo/packages/pure-dev/dev/RealDev.res new file mode 100644 index 0000000000..4c298a701f --- /dev/null +++ b/rewatch/testrepo/packages/pure-dev/dev/RealDev.res @@ -0,0 +1 @@ +let dev = true \ No newline at end of file diff --git a/rewatch/testrepo/packages/pure-dev/package.json b/rewatch/testrepo/packages/pure-dev/package.json new file mode 100644 index 0000000000..0276294558 --- /dev/null +++ b/rewatch/testrepo/packages/pure-dev/package.json @@ -0,0 +1,9 @@ +{ + "name": "@testrepo/pure-dev", + "version": "0.0.1", + "keywords": [ + "rescript" + ], + "author": "", + "license": "MIT" +} diff --git a/rewatch/testrepo/packages/pure-dev/rescript.json b/rewatch/testrepo/packages/pure-dev/rescript.json new file mode 100644 index 0000000000..7eb6700ad8 --- /dev/null +++ b/rewatch/testrepo/packages/pure-dev/rescript.json @@ -0,0 +1,8 @@ +{ + "name": "@testrepo/nonexisting-dev-files", + "sources": { + "dir": "dev", + "subdirs": true, + "type": "dev" + } +} diff --git a/rewatch/testrepo/rescript.json b/rewatch/testrepo/rescript.json index ae437e38dd..7f47e9e3a6 100644 --- a/rewatch/testrepo/rescript.json +++ b/rewatch/testrepo/rescript.json @@ -26,5 +26,8 @@ "@testrepo/deprecated-config", "@testrepo/file-casing", "@testrepo/file-casing-no-namespace" + ], + "dev-dependencies": [ + "@testrepo/pure-dev" ] } diff --git a/rewatch/testrepo/yarn.lock b/rewatch/testrepo/yarn.lock index 808197041f..bc7da49d6a 100644 --- a/rewatch/testrepo/yarn.lock +++ b/rewatch/testrepo/yarn.lock @@ -114,6 +114,12 @@ __metadata: languageName: unknown linkType: soft +"@testrepo/pure-dev@workspace:packages/pure-dev": + version: 0.0.0-use.local + resolution: "@testrepo/pure-dev@workspace:packages/pure-dev" + languageName: unknown + linkType: soft + "@testrepo/with-dev-deps@workspace:packages/with-dev-deps": version: 0.0.0-use.local resolution: "@testrepo/with-dev-deps@workspace:packages/with-dev-deps" From 66d62eca0f0b4204015c231ab1bf36b67e02f9d9 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:30:25 +0200 Subject: [PATCH 25/47] Try and add test for format --dev --- .../testrepo/packages/pure-dev/dev/RealDev.res | 2 +- rewatch/tests/format.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/rewatch/testrepo/packages/pure-dev/dev/RealDev.res b/rewatch/testrepo/packages/pure-dev/dev/RealDev.res index 4c298a701f..cd25d227f7 100644 --- a/rewatch/testrepo/packages/pure-dev/dev/RealDev.res +++ b/rewatch/testrepo/packages/pure-dev/dev/RealDev.res @@ -1 +1 @@ -let dev = true \ No newline at end of file +let dev = true \ No newline at end of file diff --git a/rewatch/tests/format.sh b/rewatch/tests/format.sh index 4beb240bf5..8db6d1f5a1 100755 --- a/rewatch/tests/format.sh +++ b/rewatch/tests/format.sh @@ -56,4 +56,19 @@ else echo "Expected 2 files to be changed, got $git_diff_file_count" echo $error_output exit 1 +fi + +bold "Test: it should format dev package as well" + +error_output=$("$REWATCH_EXECUTABLE" format --dev) +git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) +if [ $? -eq 0 ] && [ $git_diff_file_count -eq 9 ]; +then + success "Test package formatted. Got $git_diff_file_count changed files." + git restore . +else + error "Error formatting test package" + echo "Expected 9 files to be changed, got $git_diff_file_count" + echo $error_output + exit 1 fi \ No newline at end of file From 83483429a35f62ed9f49edc508783b0a808a241d Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:33:41 +0200 Subject: [PATCH 26/47] Respect --dev for packages::make in format --- rewatch/src/format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index 5b33b77e02..2fa918c930 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -41,7 +41,7 @@ fn get_files_in_scope(format_dev_deps: bool) -> Result> { let current_dir = std::env::current_dir()?; let project_context = project_context::ProjectContext::new(¤t_dir)?; - let packages = packages::make(&None, &project_context, false, false)?; + let packages = packages::make(&None, &project_context, false, format_dev_deps)?; let mut files: Vec = Vec::new(); let packages_to_format = project_context.get_scoped_local_packages(format_dev_deps); From c87fa26629e1dc5c43ccb966ad94b689e6629ae8 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:35:42 +0200 Subject: [PATCH 27/47] Improve success message --- rewatch/tests/format.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/tests/format.sh b/rewatch/tests/format.sh index 8db6d1f5a1..f96403c8dd 100755 --- a/rewatch/tests/format.sh +++ b/rewatch/tests/format.sh @@ -64,7 +64,7 @@ error_output=$("$REWATCH_EXECUTABLE" format --dev) git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) if [ $? -eq 0 ] && [ $git_diff_file_count -eq 9 ]; then - success "Test package formatted. Got $git_diff_file_count changed files." + success "All packages (including dev) were formatted. Got $git_diff_file_count changed files." git restore . else error "Error formatting test package" From 98cdcc29ac40619c4300f20373d31b9f27869bef Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 17:47:28 +0200 Subject: [PATCH 28/47] Add test to ensure we clean dev-dependency with --dev. --- rewatch/src/build/clean.rs | 41 ++++++++++---------------------------- rewatch/tests/clean.sh | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index fbb09d27a5..61f6a8e83a 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -60,11 +60,12 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files(build_state: &BuildState, root_package: &Package, suffix: &str, clean_dev_deps: bool) { - let packages_to_clean = build_state - .project_context - .get_scoped_local_packages(clean_dev_deps); - +fn clean_source_files( + build_state: &BuildState, + root_package: &Package, + suffix: &str, + packages_to_clean: &AHashSet, +) { // get all rescript file locations let rescript_file_locations = build_state .modules @@ -369,30 +370,10 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_ let _ = std::io::stdout().flush(); }; - match &project_context { - ProjectContext::SingleProject { config, .. } => { - find_and_clean_package(&packages, &config.name, show_progress, snapshot_output)?; - } - ProjectContext::MonorepoRoot { - config, - local_dependencies, - .. - } => { - find_and_clean_package(&packages, &config.name, show_progress, snapshot_output)?; - // TODO: this does the local dependencies and dev-dependencies. - // I guess the dev deps should be cleaned if flag is set? - for dep in local_dependencies { - find_and_clean_package(&packages, dep, show_progress, snapshot_output)?; - } - } - ProjectContext::MonorepoPackage { config, .. } => { - // We know we are in a monorepo, but we only clean the package that is being targeted. - let package = packages - .get(&config.name) - .expect("Could not find package during clean"); - clean_package(show_progress, snapshot_output, package); - } - }; + let packages_to_clean = project_context.get_scoped_local_packages(clean_dev_deps); + for package in &packages_to_clean { + find_and_clean_package(&packages, package, show_progress, snapshot_output)?; + } let timing_clean_compiler_assets_elapsed = timing_clean_compiler_assets.elapsed(); @@ -423,7 +404,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_ let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, root_package, &suffix, clean_dev_deps); + clean_source_files(&build_state, root_package, &suffix, &packages_to_clean); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/tests/clean.sh b/rewatch/tests/clean.sh index bf3b27bd5d..b6539d5a3f 100755 --- a/rewatch/tests/clean.sh +++ b/rewatch/tests/clean.sh @@ -46,4 +46,39 @@ then else error "Expected files from new-namespace not to be cleaned" exit 1 +fi + +bold "--dev should clean dev-dependencies of monorepo" + +# First we build the entire monorepo (including --dev) +error_output=$(rewatch build --dev 2>&1) +if [ $? -eq 0 ]; +then + success "Built monorepo" +else + error "Error building monorepo" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Clean entire monorepo (including --dev) +error_output=$(rewatch clean --dev 2>&1) +clean_status=$? +if [ $clean_status -ne 0 ]; +then + error "Error cleaning current project file-casing" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Count compiled files in dev-dependency project "pure-dev" +project_compiled_files=$(find packages/pure-dev -type f -name '*.mjs' | wc -l | tr -d '[:space:]') +if [ "$project_compiled_files" -eq 0 ]; +then + success "pure-dev cleaned" + git restore . +else + error "Expected 0 .mjs files in pure-dev after clean, got $project_compiled_files" + printf "%s\n" "$error_output" + exit 1 fi \ No newline at end of file From 1ccc44c31fff5b3dddef09f883448a81ce7a99fa Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 11 Aug 2025 18:00:19 +0200 Subject: [PATCH 29/47] Pass in actual rescript.json --- rewatch/src/project_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index cabaa23988..460e67d660 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -195,7 +195,7 @@ impl ProjectContext { // There is a parent rescript.json, and it has a reference to the current package. Ok(ProjectContext::MonorepoPackage { config: current_config, - path: path.join("rescript.json"), + path: current_rescript_json, parent_config: workspace_config, parent_path: workspace_rescript_json.clone(), }) From f83fb385f812fac0f60b1d0a758ee8446a3c3336 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 08:58:26 +0200 Subject: [PATCH 30/47] Ensure dependencies are cleaned as well. --- rewatch/src/build/build_types.rs | 16 ++-------- rewatch/src/build/clean.rs | 40 +++++++------------------ rewatch/src/build/compile.rs | 8 ++--- rewatch/src/build/packages.rs | 14 ++++----- rewatch/src/build/read_compile_state.rs | 7 ++--- rewatch/src/config.rs | 2 -- rewatch/tests/clean.sh | 35 ++++++++++++++++++++++ 7 files changed, 60 insertions(+), 62 deletions(-) diff --git a/rewatch/src/build/build_types.rs b/rewatch/src/build/build_types.rs index 39f39c0354..76035ab27f 100644 --- a/rewatch/src/build/build_types.rs +++ b/rewatch/src/build/build_types.rs @@ -1,7 +1,7 @@ use crate::build::packages::{Namespace, Package}; +use crate::config::Config; use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; -use anyhow::{Result, anyhow}; use std::{fmt::Display, path::PathBuf, time::SystemTime}; #[derive(Debug, Clone, PartialEq)] @@ -129,18 +129,8 @@ impl BuildState { self.module_names.insert(module_name.to_owned()); } - pub fn get_root_package(&self) -> Result<&Package> { - match &self.project_context { - ProjectContext::SingleProject { config, .. } - | ProjectContext::MonorepoRoot { config, .. } - | ProjectContext::MonorepoPackage { - parent_config: config, - .. - } => self - .packages - .get(&config.name) - .ok_or_else(|| anyhow!("Root package \"{}\" not found", &config.name)), - } + pub fn get_root_config(&self) -> &Config { + self.project_context.get_root_config() } } diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 61f6a8e83a..96bb51140d 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -1,11 +1,12 @@ use super::build_types::*; use super::packages; use crate::build::packages::Package; +use crate::config::Config; use crate::helpers; use crate::helpers::emojis::*; use crate::project_context::ProjectContext; -use ahash::{AHashMap, AHashSet}; -use anyhow::{Result, anyhow}; +use ahash::AHashSet; +use anyhow::Result; use console::style; use rayon::prelude::*; use std::io::Write; @@ -60,25 +61,19 @@ pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) { } } -fn clean_source_files( - build_state: &BuildState, - root_package: &Package, - suffix: &str, - packages_to_clean: &AHashSet, -) { +fn clean_source_files(build_state: &BuildState, root_config: &Config, suffix: &str) { // get all rescript file locations let rescript_file_locations = build_state .modules .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - if !packages_to_clean.contains(&module.package_name) { + if !build_state.packages.contains_key(&module.package_name) { None } else { let package = build_state.packages.get(&module.package_name).unwrap(); Some( - root_package - .config + root_config .get_package_specs() .iter() .filter_map(|spec| { @@ -340,20 +335,6 @@ pub fn cleanup_after_build(build_state: &BuildState) { }); } -fn find_and_clean_package( - packages: &AHashMap, - name: &String, - show_progress: bool, - snapshot_output: bool, -) -> Result<()> { - if let Some(package) = packages.get(name) { - clean_package(show_progress, snapshot_output, package); - Ok(()) - } else { - Err(anyhow!("Could not find package \"{}\" during clean", &name)) - } -} - pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_deps: bool) -> Result<()> { let project_context = ProjectContext::new(path)?; @@ -370,9 +351,8 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_ let _ = std::io::stdout().flush(); }; - let packages_to_clean = project_context.get_scoped_local_packages(clean_dev_deps); - for package in &packages_to_clean { - find_and_clean_package(&packages, package, show_progress, snapshot_output)?; + for (_, package) in &packages { + clean_package(show_progress, snapshot_output, package) } let timing_clean_compiler_assets_elapsed = timing_clean_compiler_assets.elapsed(); @@ -391,7 +371,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_ let timing_clean_mjs = Instant::now(); let mut build_state = BuildState::new(project_context, packages, bsc_path); packages::parse_packages(&mut build_state); - let root_package = build_state.get_root_package()?; + let root_config = build_state.get_root_config(); let suffix = build_state.project_context.get_suffix(); if !snapshot_output && show_progress { @@ -404,7 +384,7 @@ pub fn clean(path: &Path, show_progress: bool, snapshot_output: bool, clean_dev_ let _ = std::io::stdout().flush(); } - clean_source_files(&build_state, root_package, &suffix, &packages_to_clean); + clean_source_files(&build_state, root_config, &suffix); let timing_clean_mjs_elapsed = timing_clean_mjs.elapsed(); if !snapshot_output && show_progress { diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index f7b9341948..2e13b92563 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -558,7 +558,7 @@ fn compile_file( bsc_path, .. } = build_state; - let root_package = build_state.get_root_package()?; + let root_config = build_state.get_root_config(); let ocaml_build_path_abs = package.get_ocaml_build_path(); let build_path_abs = package.get_build_path(); let implementation_file_path = match &module.source_type { @@ -699,7 +699,7 @@ fn compile_file( } // copy js file - root_package.config.get_package_specs().iter().for_each(|spec| { + root_config.get_package_specs().iter().for_each(|spec| { if spec.in_source { if let SourceType::SourceFile(SourceFile { implementation: Implementation { path, .. }, @@ -708,11 +708,11 @@ fn compile_file( { let source = helpers::get_source_file_from_rescript_file( &Path::new(&package.path).join(path), - &root_package.config.get_suffix(spec), + &root_config.get_suffix(spec), ); let destination = helpers::get_source_file_from_rescript_file( &package.get_build_path().join(path), - &root_package.config.get_suffix(spec), + &root_config.get_suffix(spec), ); if source.exists() { diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 7fce2e9367..a2879c8c00 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -489,14 +489,10 @@ fn read_packages( ) -> Result> { // Store all packages and completely deduplicate them let mut map: AHashMap = AHashMap::new(); - let root_package = match project_context { + let current_package = match project_context { ProjectContext::SingleProject { config, path } | ProjectContext::MonorepoRoot { config, path, .. } - | ProjectContext::MonorepoPackage { - parent_config: config, - parent_path: path, - .. - } => { + | ProjectContext::MonorepoPackage { config, path, .. } => { let folder = path .parent() .ok_or_else(|| anyhow!("Could not the read parent folder or a rescript.json file"))?; @@ -504,13 +500,13 @@ fn read_packages( } }; - map.insert(root_package.name.to_string(), root_package); + map.insert(current_package.name.to_string(), current_package); let mut registered_dependencies_set: AHashSet = AHashSet::new(); let dependencies = flatten_dependencies(read_dependencies( &mut registered_dependencies_set, project_context, - project_context.get_root_config(), + project_context.get_current_rescript_config(), show_progress, build_dev_deps, )); @@ -656,7 +652,7 @@ pub fn parse_packages(build_state: &mut BuildState) { let bs_build_path = package.get_ocaml_build_path(); helpers::create_path(&build_path_abs); helpers::create_path(&bs_build_path); - let root_config = build_state.project_context.get_root_config(); + let root_config = build_state.get_root_config(); root_config.get_package_specs().iter().for_each(|spec| { if !spec.in_source { diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index 0b5d58a9bb..3a3b8adc59 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -73,7 +73,7 @@ pub fn read(build_state: &mut BuildState) -> anyhow::Result .flatten() .collect::>(); - let root_package = build_state.get_root_package()?; + let root_config = build_state.get_root_config(); compile_assets.iter().for_each( |(path, last_modified, extension, package_name, package_namespace, package_is_root)| { @@ -91,9 +91,8 @@ pub fn read(build_state: &mut BuildState) -> anyhow::Result last_modified: last_modified.to_owned(), ast_file_path: path.to_path_buf(), is_root: *package_is_root, - suffix: root_package - .config - .get_suffix(root_package.config.get_package_specs().first().unwrap()), + suffix: root_config + .get_suffix(root_config.get_package_specs().first().unwrap()), }, ); let _ = ast_rescript_file_locations.insert(res_file_path_buf); diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 6d2878c790..f5846e8eef 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -533,8 +533,6 @@ impl Config { .unwrap_or(".js".to_string()) } - // TODO: needs improving! - pub fn find_is_type_dev_for_path(&self, relative_path: &Path) -> bool { let relative_parent = match relative_path.parent() { None => return false, diff --git a/rewatch/tests/clean.sh b/rewatch/tests/clean.sh index b6539d5a3f..37626b0afb 100755 --- a/rewatch/tests/clean.sh +++ b/rewatch/tests/clean.sh @@ -81,4 +81,39 @@ else error "Expected 0 .mjs files in pure-dev after clean, got $project_compiled_files" printf "%s\n" "$error_output" exit 1 +fi + +bold "Test: It should clean dependencies from node_modules" + +# Build a package with external dependencies +error_output=$(cd packages/with-dev-deps && "../../$REWATCH_EXECUTABLE" build 2>&1) +if [ $? -eq 0 ]; +then + success "Built with-dev-deps" +else + error "Error building with-dev-deps" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Then we clean a single project +error_output=$(cd packages/with-dev-deps && "../../$REWATCH_EXECUTABLE" clean 2>&1) +clean_status=$? +if [ $clean_status -ne 0 ]; +then + error "Error cleaning current project file-casing" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +# Count compiled files in the cleaned project +compiler_assets=$(find node_modules/rescript-nodejs/lib/ocaml -type f -name '*.*' | wc -l | tr -d '[:space:]') +if [ $compiler_assets -eq 0 ]; +then + success "compiler assets from node_modules cleaned" + git checkout . +else + error "Expected 0 files in node_modules/rescript-nodejs/lib/ocaml after clean, got $compiler_assets" + printf "%s\n" "$error_output" + exit 1 fi \ No newline at end of file From dc05c767f6ba2534c77e6f091b665cf943be5583 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 09:40:53 +0200 Subject: [PATCH 31/47] restore instead of clean --- rewatch/tests/clean.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/tests/clean.sh b/rewatch/tests/clean.sh index 37626b0afb..f04aa14387 100755 --- a/rewatch/tests/clean.sh +++ b/rewatch/tests/clean.sh @@ -111,7 +111,7 @@ compiler_assets=$(find node_modules/rescript-nodejs/lib/ocaml -type f -name '*.* if [ $compiler_assets -eq 0 ]; then success "compiler assets from node_modules cleaned" - git checkout . + git restore . else error "Expected 0 files in node_modules/rescript-nodejs/lib/ocaml after clean, got $compiler_assets" printf "%s\n" "$error_output" From c8ebab8c9dd5bb717499209c73bf164f227eebb5 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 14:50:34 +0200 Subject: [PATCH 32/47] Address code review remarks of clean.rs --- rewatch/src/build/clean.rs | 43 +++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 96bb51140d..e4a4786a8c 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -68,30 +68,25 @@ fn clean_source_files(build_state: &BuildState, root_config: &Config, suffix: &s .values() .filter_map(|module| match &module.source_type { SourceType::SourceFile(source_file) => { - if !build_state.packages.contains_key(&module.package_name) { - None - } else { - let package = build_state.packages.get(&module.package_name).unwrap(); - Some( - root_config - .get_package_specs() - .iter() - .filter_map(|spec| { - if spec.in_source { - Some(( - package.path.join(&source_file.implementation.path), - match &spec.suffix { - None => suffix.to_owned(), - Some(suffix) => suffix.clone(), - }, - )) - } else { - None - } - }) - .collect::>(), - ) - } + 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 => suffix.to_owned(), + Some(suffix) => suffix, + }, + )) + } else { + None + } + }) + .collect::>() + }) } _ => None, }) From 067e983d4cb8a53d7285509cb99e7d194413e5a3 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 15:27:53 +0200 Subject: [PATCH 33/47] Store path to config in Config struct --- rewatch/src/build/packages.rs | 22 +++++++------ rewatch/src/config.rs | 18 ++++++++++- rewatch/src/project_context.rs | 57 ++++++++++++---------------------- 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index a2879c8c00..e0298c42c5 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -239,14 +239,14 @@ fn get_source_dirs(source: config::Source, sub_path: Option) -> AHashSe source_folders } -pub fn read_config(package_dir: &Path) -> Result<(Config, PathBuf)> { +pub fn read_config(package_dir: &Path) -> Result { let rescript_json_path = package_dir.join("rescript.json"); let bsconfig_json_path = package_dir.join("bsconfig.json"); if Path::new(&rescript_json_path).exists() { - Config::new(&rescript_json_path).map(|c| (c, rescript_json_path)) + Config::new(&rescript_json_path) } else { - Config::new(&bsconfig_json_path).map(|c| (c, bsconfig_json_path)) + Config::new(&bsconfig_json_path) } } @@ -311,7 +311,7 @@ fn read_dependencies( // Read all config files in parallel instead of blocking .par_iter() .map(|package_name| { - let ((config, _config_path), canonical_path) = + let (config, canonical_path) = match read_dependency(package_name, project_context) { Err(error) => { if show_progress { @@ -359,10 +359,10 @@ fn read_dependencies( local_dependencies.contains(package_name) || local_dev_dependencies.contains(package_name) } ProjectContext::MonorepoPackage { - parent_path, + parent_config, .. } => { - helpers::is_local_package(parent_path, &canonical_path) + helpers::is_local_package(&parent_config.path, &canonical_path) } } }; @@ -490,10 +490,11 @@ fn read_packages( // Store all packages and completely deduplicate them let mut map: AHashMap = AHashMap::new(); let current_package = match project_context { - ProjectContext::SingleProject { config, path } - | ProjectContext::MonorepoRoot { config, path, .. } - | ProjectContext::MonorepoPackage { config, path, .. } => { - let folder = path + ProjectContext::SingleProject { config } + | ProjectContext::MonorepoRoot { config, .. } + | ProjectContext::MonorepoPackage { config, .. } => { + let folder = config + .path .parent() .ok_or_else(|| anyhow!("Could not the read parent folder or a rescript.json file"))?; make_package(config.to_owned(), folder, true, true) @@ -998,6 +999,7 @@ mod test { bs_deps: args.bs_deps, build_dev_deps: args.build_dev_deps, allowed_dependents: args.allowed_dependents, + path: PathBuf::from("./something/rescript.json"), }), source_folders: AHashSet::new(), source_files: None, diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index f5846e8eef..3435859e4b 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -266,6 +266,13 @@ pub struct Config { // Holds all deprecation warnings for the config struct #[serde(skip)] deprecation_warnings: Vec, + + #[serde(default = "default_path")] + pub path: PathBuf, +} + +fn default_path() -> PathBuf { + PathBuf::from("./rescript.json") } /// This flattens string flags @@ -375,7 +382,9 @@ impl Config { /// Try to convert a bsconfig from a certain path to a bsconfig struct pub fn new(path: &Path) -> Result { let read = fs::read_to_string(path)?; - Config::new_from_json_string(&read) + let mut config = Config::new_from_json_string(&read)?; + config.set_path(path.to_path_buf())?; + Ok(config) } /// Try to convert a bsconfig from a string to a bsconfig struct @@ -387,6 +396,11 @@ impl Config { Ok(config) } + fn set_path(&mut self, path: PathBuf) -> Result<()> { + self.path = path; + Ok(()) + } + pub fn get_namespace(&self) -> packages::Namespace { let namespace_from_package = namespace_from_package_name(&self.name); match (self.namespace.as_ref(), self.namespace_entry.as_ref()) { @@ -618,6 +632,7 @@ pub mod tests { pub bs_deps: Vec, pub build_dev_deps: Vec, pub allowed_dependents: Option>, + pub path: PathBuf, } pub fn create_config(args: CreateConfigArgs) -> Config { @@ -642,6 +657,7 @@ pub mod tests { namespace_entry: None, deprecation_warnings: vec![], allowed_dependents: args.allowed_dependents, + path: args.path, } } diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 460e67d660..9b40585a39 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -6,9 +6,7 @@ use anyhow::Result; use anyhow::anyhow; use log::debug; use std::fmt; -use std::path::{Path, PathBuf}; - -type RescriptJsonPath = PathBuf; +use std::path::{Path}; /// Represents the context of a command, categorizing in various ways: /// - SingleProject: one rescript.json with no parent or children. @@ -20,22 +18,15 @@ type RescriptJsonPath = PathBuf; #[allow(clippy::large_enum_variant)] pub enum ProjectContext { /// Single project - no workspace relationship - SingleProject { config: Config, path: RescriptJsonPath }, + SingleProject { config: Config }, /// Monorepo root - contains local dependencies (symlinked in node_modules) MonorepoRoot { config: Config, - path: RescriptJsonPath, local_dependencies: AHashSet, // names of local deps local_dev_dependencies: AHashSet, }, /// Package within a monorepo - has a parent workspace - MonorepoPackage { - config: Config, - // This field is only used for debugging purposes, maybe it could be removed in the future. - path: RescriptJsonPath, - parent_config: Config, - parent_path: RescriptJsonPath, - }, + MonorepoPackage { config: Config, parent_config: Config }, } fn format_dependencies(dependencies: &AHashSet) -> String { @@ -56,17 +47,16 @@ fn format_dependencies(dependencies: &AHashSet) -> String { impl fmt::Debug for ProjectContext { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self { - ProjectContext::SingleProject { config, path } => { + ProjectContext::SingleProject { config } => { write!( f, "SingleProject: \"{}\" at \"{}\"", config.name, - path.to_string_lossy() + config.path.to_string_lossy() ) } ProjectContext::MonorepoRoot { config, - path, local_dependencies, local_dev_dependencies, } => { @@ -77,24 +67,22 @@ impl fmt::Debug for ProjectContext { f, "MonorepoRoot: \"{}\" at \"{}\" with dependencies:\n {}\n and devDependencies:\n {}", config.name, - path.to_string_lossy(), + config.path.to_string_lossy(), deps, dev_deps ) } ProjectContext::MonorepoPackage { config, - path, parent_config, - parent_path, } => { write!( f, "MonorepoPackage:\n \"{}\" at \"{}\"\n with parent \"{}\" at \"{}\"", config.name, - path.to_string_lossy(), + config.path.to_string_lossy(), parent_config.name, - parent_path.to_string_lossy() + parent_config.path.to_string_lossy() ) } } @@ -128,11 +116,7 @@ fn read_local_packages( Ok(local_dependencies) } -fn monorepo_or_single_project( - path: &Path, - current_rescript_json: RescriptJsonPath, - current_config: Config, -) -> Result { +fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result { let local_dependencies = match ¤t_config.dependencies { None => AHashSet::::new(), Some(deps) => read_local_packages(path, deps)?, @@ -144,12 +128,10 @@ fn monorepo_or_single_project( if local_dependencies.is_empty() && local_dev_dependencies.is_empty() { Ok(ProjectContext::SingleProject { config: current_config, - path: current_rescript_json.clone(), }) } else { Ok(ProjectContext::MonorepoRoot { config: current_config, - path: current_rescript_json.clone(), local_dependencies, local_dev_dependencies, }) @@ -159,7 +141,7 @@ fn monorepo_or_single_project( impl ProjectContext { pub fn new(path: &Path) -> Result { let path = helpers::get_abs_path(path); - let (current_config, current_rescript_json) = packages::read_config(&path) + let current_config = packages::read_config(&path) .map_err(|_| anyhow!("Could not read rescript.json at {}", path.to_string_lossy()))?; let nearest_parent_config_path = match path.parent() { None => Err(anyhow!( @@ -169,7 +151,7 @@ impl ProjectContext { Some(parent) => Ok(helpers::get_nearest_config(parent)), }?; let context = match nearest_parent_config_path { - None => monorepo_or_single_project(&path, current_rescript_json, current_config), + None => monorepo_or_single_project(&path, current_config), Some(parent_config_path) => { match packages::read_config(parent_config_path.as_path()) { Err(e) => Err(anyhow!( @@ -177,7 +159,7 @@ impl ProjectContext { parent_config_path.to_string_lossy(), e )), - Ok((workspace_config, workspace_rescript_json)) => { + Ok(workspace_config) => { let is_current_config_listed_in_workspace = workspace_config .dependencies .to_owned() @@ -195,14 +177,12 @@ impl ProjectContext { // There is a parent rescript.json, and it has a reference to the current package. Ok(ProjectContext::MonorepoPackage { config: current_config, - path: current_rescript_json, parent_config: workspace_config, - parent_path: workspace_rescript_json.clone(), }) } else { // There is a parent rescript.json, but it has no reference to the current package. // However, the current package could still be a monorepo root! - monorepo_or_single_project(&path, current_rescript_json, current_config) + monorepo_or_single_project(&path, current_config) } } } @@ -227,11 +207,12 @@ impl ProjectContext { pub fn get_root_path(&self) -> &Path { let rescript_json = match self { - ProjectContext::SingleProject { path, .. } => path, - ProjectContext::MonorepoRoot { path, .. } => path, - ProjectContext::MonorepoPackage { - parent_path: path, .. - } => path, + ProjectContext::SingleProject { config, .. } + | ProjectContext::MonorepoRoot { config, .. } + | ProjectContext::MonorepoPackage { + parent_config: config, + .. + } => &config.path, }; rescript_json.parent().unwrap() } From 2606104bfc69191ce1e5eca77428c6e0d5ec242b Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 15:56:58 +0200 Subject: [PATCH 34/47] Format --- rewatch/src/project_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 9b40585a39..1842d6cf1a 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -6,7 +6,7 @@ use anyhow::Result; use anyhow::anyhow; use log::debug; use std::fmt; -use std::path::{Path}; +use std::path::Path; /// Represents the context of a command, categorizing in various ways: /// - SingleProject: one rescript.json with no parent or children. From c463e9425252825e823df32b28e8f755c771ed3c Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:06:49 +0200 Subject: [PATCH 35/47] Add with-ppx example --- rewatch/testrepo/packages/with-ppx/package.json | 0 rewatch/testrepo/packages/with-ppx/rescript.json | 0 rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res | 0 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 rewatch/testrepo/packages/with-ppx/package.json create mode 100644 rewatch/testrepo/packages/with-ppx/rescript.json create mode 100644 rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res diff --git a/rewatch/testrepo/packages/with-ppx/package.json b/rewatch/testrepo/packages/with-ppx/package.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/rewatch/testrepo/packages/with-ppx/rescript.json b/rewatch/testrepo/packages/with-ppx/rescript.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res b/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res new file mode 100644 index 0000000000..e69de29bb2 From 607b412563fdc6851846ac99fb4fc501b30f579b Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:48:46 +0200 Subject: [PATCH 36/47] Ensure parse arguments of source package are used. --- rewatch/src/build.rs | 7 ++++- rewatch/src/build/parse.rs | 21 ++++++++----- rewatch/testrepo/package.json | 3 +- .../testrepo/packages/with-ppx/package.json | 14 +++++++++ .../testrepo/packages/with-ppx/rescript.json | 20 ++++++++++++ .../packages/with-ppx/src/FileWithPpx.res | 6 ++++ rewatch/testrepo/rescript.json | 3 +- rewatch/testrepo/yarn.lock | 31 +++++++++++++++++++ 8 files changed, 94 insertions(+), 11 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index ca49214a57..0f85a6cbef 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -75,7 +75,12 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { let file_path = PathBuf::from(¤t_package).join(filename); let contents = helpers::read_file(&file_path).expect("Error reading file"); - let (ast_path, parser_args) = parser_args(&project_context, relative_filename, &contents); + let (ast_path, parser_args) = parser_args( + &project_context, + project_context.get_current_rescript_config(), + relative_filename, + &contents, + ); let is_interface = filename.to_string_lossy().ends_with('i'); let has_interface = if is_interface { true diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 933916309a..301dd0c536 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -1,9 +1,9 @@ use super::build_types::*; use super::logs; use super::namespaces; -use super::packages; +use crate::build::packages::Package; use crate::config; -use crate::config::OneOrMore; +use crate::config::{Config, OneOrMore}; use crate::helpers; use crate::project_context::ProjectContext; use ahash::AHashSet; @@ -234,24 +234,24 @@ pub fn generate_asts( pub fn parser_args( project_context: &ProjectContext, + package_config: &Config, filename: &Path, contents: &str, ) -> (PathBuf, Vec) { - let current_config = project_context.get_current_rescript_config(); let root_config = project_context.get_root_config(); let file = &filename; let ast_path = helpers::get_ast_path(file); let node_modules_path = project_context.get_root_path().join("node_modules"); let ppx_flags = config::flatten_ppx_flags( node_modules_path.as_path(), - &filter_ppx_flags(¤t_config.ppx_flags, contents), - ¤t_config.name, + &filter_ppx_flags(&package_config.ppx_flags, contents), + &package_config.name, ); let jsx_args = root_config.get_jsx_args(); let jsx_module_args = root_config.get_jsx_module_args(); let jsx_mode_args = root_config.get_jsx_mode_args(); let jsx_preserve_args = root_config.get_jsx_preserve_args(); - let bsc_flags = config::flatten_flags(¤t_config.compiler_flags); + let bsc_flags = config::flatten_flags(&package_config.compiler_flags); let file = PathBuf::from("..").join("..").join(file); @@ -277,7 +277,7 @@ pub fn parser_args( } fn generate_ast( - package: packages::Package, + package: Package, filename: &Path, build_state: &BuildState, ) -> Result<(PathBuf, Option), String> { @@ -285,7 +285,12 @@ fn generate_ast( let contents = helpers::read_file(&file_path).expect("Error reading file"); let build_path_abs = package.get_build_path(); - let (ast_path, parser_args) = parser_args(&build_state.project_context, filename, &contents); + let (ast_path, parser_args) = + parser_args(&build_state.project_context, &package.config, filename, &contents); + + if file_path.ends_with("FileWithPpx.res") { + println!("\nFileWithPpx.res {}\n", parser_args.join(",")); + } // generate the dir of the ast_path (it mirrors the source file dir) let ast_parent_path = package.get_build_path().join(ast_path.parent().unwrap()); diff --git a/rewatch/testrepo/package.json b/rewatch/testrepo/package.json index 55c8e5d425..3bb995984a 100644 --- a/rewatch/testrepo/package.json +++ b/rewatch/testrepo/package.json @@ -14,7 +14,8 @@ "packages/deprecated-config", "packages/file-casing", "packages/file-casing-no-namespace", - "packages/pure-dev" + "packages/pure-dev", + "packages/with-ppx" ] }, "dependencies": { diff --git a/rewatch/testrepo/packages/with-ppx/package.json b/rewatch/testrepo/packages/with-ppx/package.json index e69de29bb2..8843d691cd 100644 --- a/rewatch/testrepo/packages/with-ppx/package.json +++ b/rewatch/testrepo/packages/with-ppx/package.json @@ -0,0 +1,14 @@ +{ + "name": "@testrepo/with-ppx", + "version": "0.0.1", + "keywords": [ + "rescript" + ], + "author": "", + "license": "MIT", + "dependencies": { + "rescript-nodejs": "16.1.0", + "sury": "^11.0.0-alpha.2", + "sury-ppx": "^11.0.0-alpha.2" + } +} diff --git a/rewatch/testrepo/packages/with-ppx/rescript.json b/rewatch/testrepo/packages/with-ppx/rescript.json index e69de29bb2..e8b4facce3 100644 --- a/rewatch/testrepo/packages/with-ppx/rescript.json +++ b/rewatch/testrepo/packages/with-ppx/rescript.json @@ -0,0 +1,20 @@ +{ + "name": "@testrepo/with-ppx", + "sources": [ + { + "dir": "src" + } + ], + "dependencies": [ + "rescript-nodejs", + "sury" + ], + "package-specs": { + "module": "es6", + "in-source": true + }, + "suffix": ".res.js", + "ppx-flags": [ + "sury-ppx/bin" + ] +} \ No newline at end of file diff --git a/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res b/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res index e69de29bb2..1d80e5b98a 100644 --- a/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res +++ b/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.res @@ -0,0 +1,6 @@ +@schema +type t = {foo: string} + +let foo = S.parseOrThrow(`{ "foo": "bar" }`, schema) + +Console.log(foo) \ No newline at end of file diff --git a/rewatch/testrepo/rescript.json b/rewatch/testrepo/rescript.json index 7f47e9e3a6..6085fed3a3 100644 --- a/rewatch/testrepo/rescript.json +++ b/rewatch/testrepo/rescript.json @@ -25,7 +25,8 @@ "@testrepo/nonexisting-dev-files", "@testrepo/deprecated-config", "@testrepo/file-casing", - "@testrepo/file-casing-no-namespace" + "@testrepo/file-casing-no-namespace", + "@testrepo/with-ppx" ], "dev-dependencies": [ "@testrepo/pure-dev" diff --git a/rewatch/testrepo/yarn.lock b/rewatch/testrepo/yarn.lock index bc7da49d6a..7712ac2c52 100644 --- a/rewatch/testrepo/yarn.lock +++ b/rewatch/testrepo/yarn.lock @@ -129,6 +129,16 @@ __metadata: languageName: unknown linkType: soft +"@testrepo/with-ppx@workspace:packages/with-ppx": + version: 0.0.0-use.local + resolution: "@testrepo/with-ppx@workspace:packages/with-ppx" + dependencies: + rescript-nodejs: "npm:16.1.0" + sury: "npm:^11.0.0-alpha.2" + sury-ppx: "npm:^11.0.0-alpha.2" + languageName: unknown + linkType: soft + "rescript-nodejs@npm:16.1.0": version: 16.1.0 resolution: "rescript-nodejs@npm:16.1.0" @@ -166,6 +176,27 @@ __metadata: languageName: node linkType: hard +"sury-ppx@npm:^11.0.0-alpha.2": + version: 11.0.0-alpha.2 + resolution: "sury-ppx@npm:11.0.0-alpha.2" + peerDependencies: + sury: ^11.0.0-alpha.2 + checksum: 10c0/ae9190fa4e406de46e88b67db233e757db36f5377301227cf5b084b5b81d360725d6fc4781e24c56cb87476cd3a42af5acc0cfc49f0c7ea17435caf065ba22ab + languageName: node + linkType: hard + +"sury@npm:^11.0.0-alpha.2": + version: 11.0.0-alpha.2 + resolution: "sury@npm:11.0.0-alpha.2" + peerDependencies: + rescript: 11.x + peerDependenciesMeta: + rescript: + optional: true + checksum: 10c0/254dd708608b125defc6b4be0f038df0f6704290df60504b70b7cd613f1e840d150ff65a3f23dbc7213f2b18b86fdc60400b4361ca46f5c86bbe7360eff9c84a + languageName: node + linkType: hard + "testrepo@workspace:.": version: 0.0.0-use.local resolution: "testrepo@workspace:." From fec8b5d1ed7d4a6d0dcd62632ed32a09f57a6abd Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:49:56 +0200 Subject: [PATCH 37/47] Add compiled file from with-ppx --- .../packages/with-ppx/src/FileWithPpx.mjs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 rewatch/testrepo/packages/with-ppx/src/FileWithPpx.mjs diff --git a/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.mjs b/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.mjs new file mode 100644 index 0000000000..23929387dd --- /dev/null +++ b/rewatch/testrepo/packages/with-ppx/src/FileWithPpx.mjs @@ -0,0 +1,17 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as S from "sury/src/S.mjs"; + +let schema = S.schema(s => ({ + foo: s.m(S.string) +})); + +let foo = S.parseOrThrow("{ \"foo\": \"bar\" }", schema); + +console.log(foo); + +export { + schema, + foo, +} +/* schema Not a pure module */ From ccbd69801323babbd3a242f319b56c5a6de45204 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:51:21 +0200 Subject: [PATCH 38/47] Update snapshot because new test project. --- .../snapshots/bs-dev-dependency-used-by-non-dev-source.txt | 2 +- rewatch/tests/snapshots/dependency-cycle.txt | 2 +- rewatch/tests/snapshots/remove-file.txt | 2 +- rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt | 2 +- rewatch/tests/snapshots/rename-file-internal-dep.txt | 2 +- rewatch/tests/snapshots/rename-file-with-interface.txt | 2 +- rewatch/tests/snapshots/rename-file.txt | 2 +- rewatch/tests/snapshots/rename-interface-file.txt | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt b/rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt index 04222b84a0..dcb7d2a84c 100644 --- a/rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt +++ b/rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt @@ -1,4 +1,4 @@ -Cleaned 0/60 +Cleaned 0/67 Parsed 2 source files Compiled 2 modules diff --git a/rewatch/tests/snapshots/dependency-cycle.txt b/rewatch/tests/snapshots/dependency-cycle.txt index cc25fb9c92..d188bae03b 100644 --- a/rewatch/tests/snapshots/dependency-cycle.txt +++ b/rewatch/tests/snapshots/dependency-cycle.txt @@ -1,4 +1,4 @@ -Cleaned 0/60 +Cleaned 0/67 Parsed 1 source files Compiled 0 modules diff --git a/rewatch/tests/snapshots/remove-file.txt b/rewatch/tests/snapshots/remove-file.txt index a19c44e596..f29d684156 100644 --- a/rewatch/tests/snapshots/remove-file.txt +++ b/rewatch/tests/snapshots/remove-file.txt @@ -1,4 +1,4 @@ -Cleaned 1/60 +Cleaned 1/67 Parsed 0 source files Compiled 1 modules diff --git a/rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt b/rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt index a0e20f68f0..dbfd47cfa9 100644 --- a/rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt +++ b/rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt @@ -1,4 +1,4 @@ -Cleaned 2/60 +Cleaned 2/67 Parsed 2 source files Compiled 3 modules diff --git a/rewatch/tests/snapshots/rename-file-internal-dep.txt b/rewatch/tests/snapshots/rename-file-internal-dep.txt index 34d9fc9c90..e2e6058d9d 100644 --- a/rewatch/tests/snapshots/rename-file-internal-dep.txt +++ b/rewatch/tests/snapshots/rename-file-internal-dep.txt @@ -1,4 +1,4 @@ -Cleaned 2/60 +Cleaned 2/67 Parsed 2 source files Compiled 2 modules diff --git a/rewatch/tests/snapshots/rename-file-with-interface.txt b/rewatch/tests/snapshots/rename-file-with-interface.txt index fa2b3dcd73..a5a1eba38b 100644 --- a/rewatch/tests/snapshots/rename-file-with-interface.txt +++ b/rewatch/tests/snapshots/rename-file-with-interface.txt @@ -1,5 +1,5 @@  No implementation file found for interface file (skipping): src/ModuleWithInterface.resi -Cleaned 2/60 +Cleaned 2/67 Parsed 1 source files Compiled 2 modules diff --git a/rewatch/tests/snapshots/rename-file.txt b/rewatch/tests/snapshots/rename-file.txt index 388a7fa123..12816226ad 100644 --- a/rewatch/tests/snapshots/rename-file.txt +++ b/rewatch/tests/snapshots/rename-file.txt @@ -1,4 +1,4 @@ -Cleaned 1/60 +Cleaned 1/67 Parsed 1 source files Compiled 1 modules diff --git a/rewatch/tests/snapshots/rename-interface-file.txt b/rewatch/tests/snapshots/rename-interface-file.txt index 7ab0b0c690..c422225d07 100644 --- a/rewatch/tests/snapshots/rename-interface-file.txt +++ b/rewatch/tests/snapshots/rename-interface-file.txt @@ -1,5 +1,5 @@  No implementation file found for interface file (skipping): src/ModuleWithInterface2.resi -Cleaned 1/60 +Cleaned 1/67 Parsed 1 source files Compiled 2 modules From 1db7bbea0653003f3dd3d650c1b1b7200a9fe5f8 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:52:37 +0200 Subject: [PATCH 39/47] Update suffix test. --- rewatch/tests/suffix.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/tests/suffix.sh b/rewatch/tests/suffix.sh index aa67fcfde9..32450befea 100755 --- a/rewatch/tests/suffix.sh +++ b/rewatch/tests/suffix.sh @@ -31,7 +31,7 @@ fi # Count files with new extension file_count=$(find ./packages -name *.res.js | wc -l) -if [ "$file_count" -eq 36 ]; +if [ "$file_count" -eq 38 ]; then success "Found files with correct suffix" else From ad06c8b6e78210b85288c3022578cfa80ffb3327 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 18:56:26 +0200 Subject: [PATCH 40/47] One more file to format --- rewatch/tests/format.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rewatch/tests/format.sh b/rewatch/tests/format.sh index f96403c8dd..2dd2df093a 100755 --- a/rewatch/tests/format.sh +++ b/rewatch/tests/format.sh @@ -6,13 +6,13 @@ bold "Test: It should format all files" git diff --name-only ./ error_output=$("$REWATCH_EXECUTABLE" format) git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) -if [ $? -eq 0 ] && [ $git_diff_file_count -eq 8 ]; +if [ $? -eq 0 ] && [ $git_diff_file_count -eq 9 ]; then success "Test package formatted. Got $git_diff_file_count changed files." git restore . else error "Error formatting test package" - echo "Expected 8 files to be changed, got $git_diff_file_count" + echo "Expected 9 files to be changed, got $git_diff_file_count" echo $error_output exit 1 fi @@ -62,7 +62,7 @@ bold "Test: it should format dev package as well" error_output=$("$REWATCH_EXECUTABLE" format --dev) git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs) -if [ $? -eq 0 ] && [ $git_diff_file_count -eq 9 ]; +if [ $? -eq 0 ] && [ $git_diff_file_count -eq 10 ]; then success "All packages (including dev) were formatted. Got $git_diff_file_count changed files." git restore . From db6883b1c5e29b7a0994fd070f12566e205b1f01 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 20:02:25 +0200 Subject: [PATCH 41/47] Moar code review suggestions. --- rewatch/src/project_context.rs | 87 ++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 1842d6cf1a..536266f765 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -31,16 +31,24 @@ pub enum ProjectContext { fn format_dependencies(dependencies: &AHashSet) -> String { if dependencies.is_empty() { - String::from("[]") + "[]".to_string() } else { - format!( - "[\n{}\n]", - dependencies - .iter() - .map(|dep| format!(" \"{dep}\"")) - .collect::>() - .join(",\n") - ) + let mut out = String::from("[\n"); + let mut first = true; + + for dep in dependencies { + if !first { + out.push_str(",\n"); + } else { + first = false; + } + out.push_str(" \""); + out.push_str(dep); + out.push('"'); + } + + out.push_str("\n]"); + out } } @@ -138,6 +146,21 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result bool { + workspace_config + .dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == ¤t_config.name) + || workspace_config + .dev_dependencies + .to_owned() + .unwrap_or_default() + .iter() + .any(|dep| dep == ¤t_config.name) +} + impl ProjectContext { pub fn new(path: &Path) -> Result { let path = helpers::get_abs_path(path); @@ -159,31 +182,19 @@ impl ProjectContext { parent_config_path.to_string_lossy(), e )), - Ok(workspace_config) => { - let is_current_config_listed_in_workspace = workspace_config - .dependencies - .to_owned() - .unwrap_or_default() - .iter() - .any(|dep| dep == ¤t_config.name) - || workspace_config - .dev_dependencies - .to_owned() - .unwrap_or_default() - .iter() - .any(|dep| dep == ¤t_config.name); - - if is_current_config_listed_in_workspace { - // There is a parent rescript.json, and it has a reference to the current package. - Ok(ProjectContext::MonorepoPackage { - config: current_config, - parent_config: workspace_config, - }) - } else { - // There is a parent rescript.json, but it has no reference to the current package. - // However, the current package could still be a monorepo root! - monorepo_or_single_project(&path, current_config) - } + Ok(workspace_config) + if is_config_listed_in_workspace(¤t_config, &workspace_config) => + { + // There is a parent rescript.json, and it has a reference to the current package. + Ok(ProjectContext::MonorepoPackage { + config: current_config, + parent_config: workspace_config, + }) + } + Ok(_) => { + // There is a parent rescript.json, but it has no reference to the current package. + // However, the current package could still be a monorepo root! + monorepo_or_single_project(&path, current_config) } } } @@ -243,13 +254,9 @@ impl ProjectContext { .. } => { local_packages.insert(config.name.clone()); - for dep in local_dependencies { - local_packages.insert(dep.clone()); - } + local_packages.extend(local_dependencies.iter().cloned()); if include_dev_deps { - for dep in local_dev_dependencies { - local_packages.insert(dep.clone()); - } + local_packages.extend(local_dev_dependencies.iter().cloned()); } } ProjectContext::MonorepoPackage { config, .. } => { From e01de31a830d20cff2120d692ea3e4c62ab70b90 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 20:36:38 +0200 Subject: [PATCH 42/47] Print if a package is local after checking. --- rewatch/src/project_context.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 536266f765..54c11077b9 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -110,14 +110,17 @@ fn read_local_packages( .canonicalize() .map(helpers::StrippedVerbatimPath::to_stripped_verbatim_path) { + let is_local = helpers::is_local_package(folder_path, &dep_path); + if is_local { + local_dependencies.insert(dep.to_string()); + } + debug!( - "Checking if dependency \"{}\" is a local package at \"{}\"", + "Dependency \"{}\" is a {}local package at \"{}\"", dep, + (if is_local { "" } else { "non-" }), dep_path.display() ); - if helpers::is_local_package(folder_path, &dep_path) { - local_dependencies.insert(dep.to_string()); - } } } From dbe85aa977a01cf1c49db5a468dc6f8c5e8ff805 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 21:04:08 +0200 Subject: [PATCH 43/47] Remove debug log --- rewatch/src/build/parse.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 301dd0c536..375c5e9abe 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -288,10 +288,6 @@ fn generate_ast( let (ast_path, parser_args) = parser_args(&build_state.project_context, &package.config, filename, &contents); - if file_path.ends_with("FileWithPpx.res") { - println!("\nFileWithPpx.res {}\n", parser_args.join(",")); - } - // generate the dir of the ast_path (it mirrors the source file dir) let ast_parent_path = package.get_build_path().join(ast_path.parent().unwrap()); helpers::create_path(&ast_parent_path); From ddb75e3a8296565a935f41de512b9cf917074249 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 12 Aug 2025 21:27:49 +0200 Subject: [PATCH 44/47] Windows fix of the day --- rewatch/src/build/deps.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewatch/src/build/deps.rs b/rewatch/src/build/deps.rs index 8159cdc5f3..c89f5898a0 100644 --- a/rewatch/src/build/deps.rs +++ b/rewatch/src/build/deps.rs @@ -18,11 +18,11 @@ fn get_dep_modules( Ok(lines) => { // we skip the first line with is some null characters // the following lines in the AST are the dependency modules - // we stop when we hit a line that starts with a "/", this is the path of the file. + // we stop when we hit a line that is an absolute path, this is the path of the file. // this is the point where the dependencies end and the actual AST starts for line in lines.skip(1).flatten() { let line = line.trim().to_string(); - if line.starts_with('/') { + if std::path::Path::new(&line).is_absolute() { break; } else if !line.is_empty() { deps.insert(line); From e59a7f4d21647bf80a95508575d2e038a52a5b8e Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 13 Aug 2025 10:30:31 +0200 Subject: [PATCH 45/47] Wait a bit more on watch file for Windows CI --- rewatch/tests/watch.sh | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/rewatch/tests/watch.sh b/rewatch/tests/watch.sh index 314d74dc83..229cd01f97 100755 --- a/rewatch/tests/watch.sh +++ b/rewatch/tests/watch.sh @@ -18,12 +18,33 @@ exit_watcher() { rm lib/rescript.lock } -rewatch_bg watch > /dev/null 2>&1 & +# Wait until a file exists (with timeout in seconds, default 30) +wait_for_file() { + local file="$1"; local timeout="${2:-30}" + while [ "$timeout" -gt 0 ]; do + [ -f "$file" ] && return 0 + sleep 1 + timeout=$((timeout - 1)) + done + return 1 +} + +# Start watcher and capture logs for debugging +rewatch_bg watch > rewatch.log 2>&1 & success "Watcher Started" +# Trigger a recompilation echo 'Js.log("added-by-test")' >> ./packages/main/src/Main.res -sleep 2 +# Wait for the compiled JS to show up (Windows CI can be slower) +target=./packages/main/src/Main.mjs +if ! wait_for_file "$target" 10; then + error "Expected output not found: $target" + ls -la ./packages/main/src || true + tail -n 200 rewatch.log || true + exit_watcher + exit 1 +fi if node ./packages/main/src/Main.mjs | grep 'added-by-test' &> /dev/null; then From b014cd1c48acf36c05bdbba92d388af1579efad4 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 13 Aug 2025 13:19:54 +0200 Subject: [PATCH 46/47] Add changelog entry --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7b81a564d..6164147cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ #### :boom: Breaking Change +- `rescript format` no longer accepts `--all`. All (non-dev) files of the current rescript.json are now formatted by default. https://github.com/rescript-lang/rescript/pull/7752 + #### :eyeglasses: Spec Compliance #### :rocket: New Feature @@ -21,12 +23,16 @@ - Add new Stdlib helpers: `String.capitalize`, `String.isEmpty`, `Dict.size`, `Dict.isEmpty`, `Array.isEmpty`, `Map.isEmpty`, `Set.isEmpty`. https://github.com/rescript-lang/rescript/pull/7516 #### :bug: Bug fix + - Fix issue with ast conversion (for ppx use) on functions with attributes on first argument. https://github.com/rescript-lang/rescript/pull/7761 #### :memo: Documentation #### :nail_care: Polish +- `rescript format` now has a `--dev` flag that works similar to `rescript clean`. https://github.com/rescript-lang/rescript/pull/7752 +- `rescript clean` now will clean an individual project (see [#7707](https://github.com/rescript-lang/rescript/issues/7707)). https://github.com/rescript-lang/rescript/pull/7752 + #### :house: Internal - AST: Use jsx_tag_name instead of Longindent.t to store jsx tag name. https://github.com/rescript-lang/rescript/pull/7760 From fc66c240a22d18e05d2e82a678e25aa387483d13 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 13 Aug 2025 14:27:25 +0200 Subject: [PATCH 47/47] Extract common data from enum. --- rewatch/src/build.rs | 6 +- rewatch/src/build/packages.rs | 46 +++++------ rewatch/src/project_context.rs | 134 +++++++++++++-------------------- 3 files changed, 74 insertions(+), 112 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 0f85a6cbef..c7c4cead3f 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -65,7 +65,7 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { let is_type_dev = match filename.strip_prefix(¤t_package) { Err(_) => false, Ok(relative_path) => project_context - .get_current_rescript_config() + .current_config .find_is_type_dev_for_path(relative_path), }; @@ -77,7 +77,7 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { let (ast_path, parser_args) = parser_args( &project_context, - project_context.get_current_rescript_config(), + &project_context.current_config, relative_filename, &contents, ); @@ -90,7 +90,7 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { PathBuf::from(&interface_filename).exists() }; let compiler_args = compile::compiler_args( - project_context.get_current_rescript_config(), + &project_context.current_config, &ast_path, relative_filename, is_interface, diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index e0298c42c5..c4d1b6e9b6 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -6,7 +6,7 @@ use crate::config::Config; use crate::helpers; use crate::helpers::StrippedVerbatimPath; use crate::helpers::emojis::*; -use crate::project_context::ProjectContext; +use crate::project_context::{MonoRepoContext, ProjectContext}; use ahash::{AHashMap, AHashSet}; use anyhow::{Result, anyhow}; use console::style; @@ -345,23 +345,18 @@ fn read_dependencies( }; let is_local_dep = { - match project_context { - ProjectContext::SingleProject { - config, - .. - } => config.name.as_str() == package_name + match &project_context.monorepo_context { + None => project_context.current_config.name.as_str() == package_name , - ProjectContext::MonorepoRoot { - local_dependencies, - local_dev_dependencies, - .. - } => { + Some(MonoRepoContext::MonorepoRoot { + local_dependencies, + local_dev_dependencies, + }) => { local_dependencies.contains(package_name) || local_dev_dependencies.contains(package_name) - } - ProjectContext::MonorepoPackage { - parent_config, - .. - } => { + }, + Some(MonoRepoContext::MonorepoPackage { + parent_config, + }) => { helpers::is_local_package(&parent_config.path, &canonical_path) } } @@ -489,16 +484,13 @@ fn read_packages( ) -> Result> { // Store all packages and completely deduplicate them let mut map: AHashMap = AHashMap::new(); - let current_package = match project_context { - ProjectContext::SingleProject { config } - | ProjectContext::MonorepoRoot { config, .. } - | ProjectContext::MonorepoPackage { config, .. } => { - let folder = config - .path - .parent() - .ok_or_else(|| anyhow!("Could not the read parent folder or a rescript.json file"))?; - make_package(config.to_owned(), folder, true, true) - } + let current_package = { + let config = &project_context.current_config; + let folder = config + .path + .parent() + .ok_or_else(|| anyhow!("Could not the read parent folder or a rescript.json file"))?; + make_package(config.to_owned(), folder, true, true) }; map.insert(current_package.name.to_string(), current_package); @@ -507,7 +499,7 @@ fn read_packages( let dependencies = flatten_dependencies(read_dependencies( &mut registered_dependencies_set, project_context, - project_context.get_current_rescript_config(), + &project_context.current_config, show_progress, build_dev_deps, )); diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index 54c11077b9..0293b7c809 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -8,25 +8,19 @@ use log::debug; use std::fmt; use std::path::Path; -/// Represents the context of a command, categorizing in various ways: -/// - SingleProject: one rescript.json with no parent or children. -/// - MonorepoRoot: a rescript.json with (dev-)dependencies found in subfolders. -/// - MonorepoPackage: a rescript.json with a parent rescript.json. -/// The current package name is listed in the parent (dev-)dependencies. -// I don't think this linting rule matters very much as we only create a single instance of this enum. -// See https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant -#[allow(clippy::large_enum_variant)] -pub enum ProjectContext { - /// Single project - no workspace relationship - SingleProject { config: Config }, +pub enum MonoRepoContext { /// Monorepo root - contains local dependencies (symlinked in node_modules) MonorepoRoot { - config: Config, local_dependencies: AHashSet, // names of local deps local_dev_dependencies: AHashSet, }, /// Package within a monorepo - has a parent workspace - MonorepoPackage { config: Config, parent_config: Config }, + MonorepoPackage { parent_config: Box }, +} + +pub struct ProjectContext { + pub current_config: Config, + pub monorepo_context: Option, } fn format_dependencies(dependencies: &AHashSet) -> String { @@ -54,41 +48,37 @@ fn format_dependencies(dependencies: &AHashSet) -> String { impl fmt::Debug for ProjectContext { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match &self { - ProjectContext::SingleProject { config } => { + match &self.monorepo_context { + None => { write!( f, - "SingleProject: \"{}\" at \"{}\"", - config.name, - config.path.to_string_lossy() + "Single project: \"{}\" at \"{}\"", + &self.current_config.name, + &self.current_config.path.to_string_lossy() ) } - ProjectContext::MonorepoRoot { - config, + Some(MonoRepoContext::MonorepoRoot { local_dependencies, local_dev_dependencies, - } => { + }) => { let deps = format_dependencies(local_dependencies); let dev_deps = format_dependencies(local_dev_dependencies); write!( f, - "MonorepoRoot: \"{}\" at \"{}\" with dependencies:\n {}\n and devDependencies:\n {}", - config.name, - config.path.to_string_lossy(), + "Monorepo root: \"{}\" at \"{}\" with dependencies:\n {}\n and devDependencies:\n {}", + &self.current_config.name, + &self.current_config.path.to_string_lossy(), deps, dev_deps ) } - ProjectContext::MonorepoPackage { - config, - parent_config, - } => { + Some(MonoRepoContext::MonorepoPackage { parent_config }) => { write!( f, "MonorepoPackage:\n \"{}\" at \"{}\"\n with parent \"{}\" at \"{}\"", - config.name, - config.path.to_string_lossy(), + &self.current_config.name, + &self.current_config.path.to_string_lossy(), parent_config.name, parent_config.path.to_string_lossy() ) @@ -118,7 +108,7 @@ fn read_local_packages( debug!( "Dependency \"{}\" is a {}local package at \"{}\"", dep, - (if is_local { "" } else { "non-" }), + if is_local { "" } else { "non-" }, dep_path.display() ); } @@ -137,14 +127,17 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result read_local_packages(path, deps)?, }; if local_dependencies.is_empty() && local_dev_dependencies.is_empty() { - Ok(ProjectContext::SingleProject { - config: current_config, + Ok(ProjectContext { + current_config, + monorepo_context: None, }) } else { - Ok(ProjectContext::MonorepoRoot { - config: current_config, - local_dependencies, - local_dev_dependencies, + Ok(ProjectContext { + current_config, + monorepo_context: Some(MonoRepoContext::MonorepoRoot { + local_dependencies, + local_dev_dependencies, + }), }) } } @@ -189,9 +182,11 @@ impl ProjectContext { if is_config_listed_in_workspace(¤t_config, &workspace_config) => { // There is a parent rescript.json, and it has a reference to the current package. - Ok(ProjectContext::MonorepoPackage { - config: current_config, - parent_config: workspace_config, + Ok(ProjectContext { + current_config, + monorepo_context: Some(MonoRepoContext::MonorepoPackage { + parent_config: Box::new(workspace_config), + }), }) } Ok(_) => { @@ -209,71 +204,46 @@ impl ProjectContext { } pub fn get_root_config(&self) -> &Config { - match self { - ProjectContext::SingleProject { config, .. } => config, - ProjectContext::MonorepoRoot { config, .. } => config, - ProjectContext::MonorepoPackage { - parent_config: config, - .. - } => config, + match &self.monorepo_context { + None => &self.current_config, + Some(MonoRepoContext::MonorepoRoot { .. }) => &self.current_config, + Some(MonoRepoContext::MonorepoPackage { parent_config }) => parent_config, } } pub fn get_root_path(&self) -> &Path { - let rescript_json = match self { - ProjectContext::SingleProject { config, .. } - | ProjectContext::MonorepoRoot { config, .. } - | ProjectContext::MonorepoPackage { - parent_config: config, - .. - } => &config.path, - }; - rescript_json.parent().unwrap() + self.get_root_config().path.parent().unwrap() } pub fn get_suffix(&self) -> String { - match self { - ProjectContext::SingleProject { config, .. } - | ProjectContext::MonorepoRoot { config, .. } - | ProjectContext::MonorepoPackage { - parent_config: config, - .. - } => config.suffix.clone().unwrap_or(String::from(".res.mjs")), - } + self.get_root_config() + .suffix + .clone() + .unwrap_or(String::from(".res.mjs")) } /// Returns the local packages relevant for the current context. /// Either a single project, all projects from a monorepo or a single package inside a monorepo. pub fn get_scoped_local_packages(&self, include_dev_deps: bool) -> AHashSet { let mut local_packages = AHashSet::::new(); - match &self { - ProjectContext::SingleProject { config, .. } => { - local_packages.insert(config.name.clone()); + match &self.monorepo_context { + None => { + local_packages.insert(self.current_config.name.clone()); } - ProjectContext::MonorepoRoot { - config, + Some(MonoRepoContext::MonorepoRoot { local_dependencies, local_dev_dependencies, - .. - } => { - local_packages.insert(config.name.clone()); + }) => { + local_packages.insert(self.current_config.name.clone()); local_packages.extend(local_dependencies.iter().cloned()); if include_dev_deps { local_packages.extend(local_dev_dependencies.iter().cloned()); } } - ProjectContext::MonorepoPackage { config, .. } => { - local_packages.insert(config.name.clone()); + Some(MonoRepoContext::MonorepoPackage { .. }) => { + local_packages.insert(self.current_config.name.clone()); } }; local_packages } - - pub fn get_current_rescript_config(&self) -> &Config { - match &self { - ProjectContext::SingleProject { config, .. } - | ProjectContext::MonorepoRoot { config, .. } - | ProjectContext::MonorepoPackage { config, .. } => config, - } - } }