diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed53baa56..deb232d09b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ #### :nail_care: Polish +- Improve circular dependency errors, and make sure they end up in the compiler log so the editor tooling can surface them. https://github.com/rescript-lang/rescript/pull/7940 + #### :house: Internal - Use AST nodes with locations for fn arguments in the typed tree. https://github.com/rescript-lang/rescript/pull/7873 diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index c48457a5e1..8048764f09 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -356,11 +356,28 @@ pub fn compile( .collect::>(), ); - compile_errors.push_str(&format!( - "\n{}\n{}\n", + let guidance = "Possible solutions:\n- Extract shared code into a new module both depend on.\n"; + let message = format!( + "\n{}\n{}\n{}", style("Can't continue... Found a circular dependency in your code:").red(), - dependency_cycle::format(&cycle) - )) + dependency_cycle::format(&cycle, build_state), + guidance + ); + + // Append the error to the logs of all packages involved in the cycle, + // so editor tooling can surface it from .compiler.log + let mut touched_packages = AHashSet::::new(); + for module_name in cycle.iter() { + if let Some(module) = build_state.get_module(module_name) { + if touched_packages.insert(module.package_name.clone()) { + if let Some(package) = build_state.get_package(&module.package_name) { + logs::append(package, &message); + } + } + } + } + + compile_errors.push_str(&message) } if !compile_errors.is_empty() { break; diff --git a/rewatch/src/build/compile/dependency_cycle.rs b/rewatch/src/build/compile/dependency_cycle.rs index a048d0ad8d..7fa763908b 100644 --- a/rewatch/src/build/compile/dependency_cycle.rs +++ b/rewatch/src/build/compile/dependency_cycle.rs @@ -1,7 +1,10 @@ use super::super::build_types::*; use crate::helpers; use ahash::AHashSet; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::{ + collections::{HashMap, HashSet, VecDeque}, + path::Path, +}; pub fn find(modules: &Vec<(&String, &Module)>) -> Vec { find_shortest_cycle(modules) @@ -139,15 +142,43 @@ fn find_cycle_bfs( None } -pub fn format(cycle: &[String]) -> String { - let mut cycle = cycle.to_vec(); - cycle.reverse(); - // add the first module to the end of the cycle - cycle.push(cycle[0].clone()); +pub fn format(cycle: &[String], build_state: &BuildCommandState) -> String { + let mut nodes = cycle.to_vec(); + if nodes.is_empty() { + return String::new(); + } + nodes.reverse(); + nodes.push(nodes[0].clone()); + + let root_path = &build_state.get_root_config().path; + let root = root_path.parent().unwrap_or(root_path.as_path()); - cycle + nodes .iter() - .map(|s| helpers::format_namespaced_module_name(s)) + .map(|name| { + let display_name = helpers::format_namespaced_module_name(name); + + match build_state.get_module(name) { + Some(Module { + source_type: SourceType::SourceFile(source_file), + package_name, + .. + }) => { + if let Some(package) = build_state.get_package(package_name) { + let abs_path = Path::new(&package.path).join(&source_file.implementation.path); + let rel_path = abs_path.strip_prefix(root).unwrap_or(&abs_path).to_string_lossy(); + format!("{display_name} ({rel_path})") + } else { + display_name + } + } + Some(Module { + source_type: SourceType::MlMap(_), + .. + }) + | None => display_name, + } + }) .collect::>() .join("\n → ") } diff --git a/rewatch/tests/snapshots/dependency-cycle.txt b/rewatch/tests/snapshots/dependency-cycle.txt index 7bf2339c9b..b5bf195403 100644 --- a/rewatch/tests/snapshots/dependency-cycle.txt +++ b/rewatch/tests/snapshots/dependency-cycle.txt @@ -12,10 +12,12 @@ The field 'bsc-flags' found in the package config of '@testrepo/deprecated-confi Use 'compiler-flags' instead. Can't continue... Found a circular dependency in your code: -Dep01 - → Dep02 - → NS - → NewNamespace.NS_alias - → Dep01 +Dep01 (packages/dep01/src/Dep01.res) + → Dep02 (packages/dep02/src/Dep02.res) + → NS (packages/new-namespace/src/NS.res) + → NewNamespace.NS_alias (packages/new-namespace/src/NS_alias.res) + → Dep01 (packages/dep01/src/Dep01.res) +Possible solutions: +- Extract shared code into a new module both depend on. Incremental build failed. Error:  Failed to Compile. See Errors Above