diff --git a/src/build.rs b/src/build.rs index 1586f9d..c03d845 100644 --- a/src/build.rs +++ b/src/build.rs @@ -1155,6 +1155,74 @@ fn compute_file_hash(path: &str) -> Option { } } +fn find_dependency_cycle(modules: &Vec<(&String, &Module)>) -> Vec { + let mut visited: AHashSet = AHashSet::new(); + let mut stack: Vec = vec![]; + + // we want to sort the module names so that we always return the same + // dependency cycle (there can be more than one) + let mut module_names = modules + .iter() + .map(|(name, _)| name.to_string()) + .collect::>(); + + module_names.sort_by(|a, b| a.cmp(b)); + for module_name in module_names { + if find_dependency_cycle_helper(&module_name, &modules, &mut visited, &mut stack) { + return stack; + } + visited.clear(); + stack.clear(); + } + stack +} + +fn find_dependency_cycle_helper( + module_name: &String, + modules: &Vec<(&String, &Module)>, + visited: &mut AHashSet, + stack: &mut Vec, +) -> bool { + if let Some(module) = modules + .iter() + .find(|(name, _)| *name == module_name) + .map(|(_, module)| module) + { + visited.insert(module_name.to_string()); + // if the module is a mlmap (namespace), we don't want to show this in the path + // because the namespace is not a module the user created, so only add source files + // to the stack + if let SourceType::SourceFile(_) = module.source_type { + stack.push(module_name.to_string()) + } + for dep in &module.deps { + if !visited.contains(dep) { + if find_dependency_cycle_helper(dep, modules, visited, stack) { + return true; + } + } else if stack.contains(dep) { + stack.push(dep.to_string()); + return true; + } + } + // because we only pushed source files to the stack, we also only need to + // pop these from the stack if we don't find a dependency cycle + if let SourceType::SourceFile(_) = module.source_type { + let _ = stack.pop(); + } + return false; + } + false +} + +fn format_dependency_cycle(cycle: &Vec) -> String { + cycle + .iter() + .map(|s| helpers::format_namespaced_module_name(s)) + .collect::>() + .join(" -> ") +} + pub fn build(filter: &Option, path: &str, no_timing: bool) -> Result { let timing_total = Instant::now(); let project_root = helpers::get_abs_path(path); @@ -1322,8 +1390,7 @@ pub fn build(filter: &Option, path: &str, no_timing: bool) -> Resu // we get this by traversing from the dirty modules to all the modules that // are dependent on them let mut compile_universe = dirty_modules.clone(); - - let mut current_step_modules = dirty_modules.clone(); + let mut current_step_modules = compile_universe.clone(); loop { let mut dependents: AHashSet = AHashSet::new(); for dirty_module in current_step_modules.iter() { @@ -1334,7 +1401,7 @@ pub fn build(filter: &Option, path: &str, no_timing: bool) -> Resu .map(|s| s.to_string()) .collect::>(); - compile_universe.extend(current_step_modules.clone()); + compile_universe.extend(current_step_modules.to_owned()); if current_step_modules.is_empty() { break; } @@ -1371,8 +1438,9 @@ pub fn build(filter: &Option, path: &str, no_timing: bool) -> Resu loop_count, ); - in_progress_modules - .clone() + let current_in_progres_modules = in_progress_modules.clone(); + + current_in_progres_modules .par_iter() .map(|module_name| { let module = build_state.get_module(module_name).unwrap(); @@ -1602,8 +1670,18 @@ pub fn build(filter: &Option, path: &str, no_timing: bool) -> Resu break; } if in_progress_modules.len() == 0 { - // we probably want to find the cycle(s), and give a helpful error message here - compile_errors.push_str("Can't continue... Dependency cycle\n") + // find the dependency cycle + let cycle = find_dependency_cycle( + &compile_universe + .iter() + .map(|s| (s, build_state.get_module(s).unwrap())) + .collect::>(), + ); + compile_errors.push_str(&format!( + "\n{}\n{}\n", + style("Can't continue... Found a circular dependency in your code:").red(), + format_dependency_cycle(&cycle) + )) } if compile_errors.len() > 0 { break; diff --git a/src/helpers.rs b/src/helpers.rs index dc0d8a1..851e7d6 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -314,3 +314,16 @@ pub fn get_extension(path: &str) -> String { .expect("Could not get extension 2") .to_string(); } + +pub fn format_namespaced_module_name(module_name: &str) -> String { + // from ModuleName-Namespace to Namespace.ModuleName + // also format ModuleName-@Namespace to Namespace.ModuleName + let mut split = module_name.split("-"); + let module_name = split.next().unwrap(); + let namespace = split.next(); + let namespace = namespace.map(|ns| ns.trim_start_matches("@")); + return match namespace { + None => module_name.to_string(), + Some(ns) => ns.to_string() + "." + module_name, + }; +} diff --git a/testrepo/packages/dep01/bsconfig.json b/testrepo/packages/dep01/bsconfig.json index 7f305a0..106a46f 100644 --- a/testrepo/packages/dep01/bsconfig.json +++ b/testrepo/packages/dep01/bsconfig.json @@ -9,7 +9,5 @@ "in-source": true }, "suffix": ".bs.js", - "bs-dependencies": [ - "@testrepo/dep02" - ] + "bs-dependencies": ["@testrepo/dep02"] } diff --git a/testrepo/packages/dep01/src/Dep01.mjs b/testrepo/packages/dep01/src/Dep01.mjs index 3fac616..ea2a8df 100644 --- a/testrepo/packages/dep01/src/Dep01.mjs +++ b/testrepo/packages/dep01/src/Dep01.mjs @@ -10,4 +10,4 @@ function log(param) { export { log , } -/* No side effect */ +/* Dep02 Not a pure module */ diff --git a/testrepo/packages/dep02/bsconfig.json b/testrepo/packages/dep02/bsconfig.json index fc120a1..0355169 100644 --- a/testrepo/packages/dep02/bsconfig.json +++ b/testrepo/packages/dep02/bsconfig.json @@ -9,5 +9,5 @@ "in-source": true }, "suffix": ".bs.js", - "bs-dependencies": [] + "bs-dependencies": ["@testrepo/new-namespace"] } diff --git a/testrepo/packages/dep02/src/Dep02.mjs b/testrepo/packages/dep02/src/Dep02.mjs index fed28ac..856730e 100644 --- a/testrepo/packages/dep02/src/Dep02.mjs +++ b/testrepo/packages/dep02/src/Dep02.mjs @@ -1,6 +1,7 @@ // Generated by ReScript, PLEASE EDIT WITH CARE import * as $$Array from "./Array.mjs"; +import * as NS_alias$$atNewNamespace from "@testrepo/new-namespace/src/NS_alias.mjs"; function log(param) { $$Array.forEach([ @@ -11,7 +12,9 @@ function log(param) { })); } +console.log(NS_alias$$atNewNamespace.hello_world(undefined)); + export { log , } -/* No side effect */ +/* Not a pure module */ diff --git a/testrepo/packages/dep02/src/Dep02.res b/testrepo/packages/dep02/src/Dep02.res index 8e2c651..e832615 100644 --- a/testrepo/packages/dep02/src/Dep02.res +++ b/testrepo/packages/dep02/src/Dep02.res @@ -1,2 +1,3 @@ -open Array; +open Array let log = () => ["a", "b"]->forEach(Js.log) +Js.log(NS.Alias.hello_world()) diff --git a/testrepo/packages/main/src/output.txt b/testrepo/packages/main/src/output.txt index 054b666..999ee65 100644 --- a/testrepo/packages/main/src/output.txt +++ b/testrepo/packages/main/src/output.txt @@ -1,3 +1,4 @@ +Hello world 01 02 a diff --git a/testrepo/packages/new-namespace/bsconfig.json b/testrepo/packages/new-namespace/bsconfig.json index 075d7fa..3b796b9 100644 --- a/testrepo/packages/new-namespace/bsconfig.json +++ b/testrepo/packages/new-namespace/bsconfig.json @@ -10,5 +10,6 @@ "module": "es6", "in-source": true }, + "bs-dependencies": ["@testrepo/dep01"], "suffix": ".bs.js" } diff --git a/tests/compile.sh b/tests/compile.sh index 955e1a4..b14d1a0 100755 --- a/tests/compile.sh +++ b/tests/compile.sh @@ -1,3 +1,4 @@ +#!/bin/bash cd $(dirname $0) source "./utils.sh" cd ../testrepo @@ -52,6 +53,12 @@ replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/remove-file.txt git checkout -- packages/dep02/src/Dep02.res rewatch build &> /dev/null +# it should show an error when we have a dependency cycle +echo 'Dep01.log()' >> packages/new-namespace/src/NS_alias.res +rewatch build --no-timing=true &> ../tests/snapshots/dependency-cycle.txt +git checkout -- packages/new-namespace/src/NS_alias.res +rewatch build &> /dev/null + # make sure we don't have changes in the test repo if git diff --exit-code ./; then diff --git a/tests/snapshots/dependency-cycle.txt b/tests/snapshots/dependency-cycle.txt new file mode 100644 index 0000000..651352b --- /dev/null +++ b/tests/snapshots/dependency-cycle.txt @@ -0,0 +1,9 @@ +[1/7] ๐ŸŒด Building package tree... [1/7] ๏ธโœ… Built package tree in 0.00s +[2/7] ๐Ÿ” Finding source files... [2/7] ๏ธโœ… Found source files in 0.00s +[3/7] ๐Ÿงน Cleaning up previous build... [3/7] ๏ธโœ… Cleaned 0/9 0.00s + [4/7] ๏ธโœ… Parsed 1 source files in 0.00s + [5/7] ๏ธโœ… Collected deps in 0.00s + [6/7] ๏ธ๐Ÿ›‘ Compiled 0 modules in 0.00s + +Can't continue... Found a circular dependency in your code: +Dep01 -> Dep02 -> NS -> NewNamespace.NS_alias -> Dep01