From 4414053ce6cc6684bd0d8027e0811f8ad2ed9a14 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 4 May 2025 15:58:26 +0200 Subject: [PATCH 1/7] test: add @rescript/core (package with dev deps) --- testrepo/packages/with-dev-deps/package.json | 3 ++- testrepo/packages/with-dev-deps/rescript.json | 1 + testrepo/yarn.lock | 11 ++++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/testrepo/packages/with-dev-deps/package.json b/testrepo/packages/with-dev-deps/package.json index d096adcb..19700f55 100644 --- a/testrepo/packages/with-dev-deps/package.json +++ b/testrepo/packages/with-dev-deps/package.json @@ -7,6 +7,7 @@ "author": "", "license": "MIT", "dependencies": { - "rescript": "*" + "rescript": "*", + "@rescript/core": "*" } } diff --git a/testrepo/packages/with-dev-deps/rescript.json b/testrepo/packages/with-dev-deps/rescript.json index 8cae5b6d..6d67d606 100644 --- a/testrepo/packages/with-dev-deps/rescript.json +++ b/testrepo/packages/with-dev-deps/rescript.json @@ -13,5 +13,6 @@ "module": "es6", "in-source": true }, + "bs-dependencies": ["@rescript/core"], "suffix": ".res.js" } diff --git a/testrepo/yarn.lock b/testrepo/yarn.lock index 2a3f684a..cc5d0224 100644 --- a/testrepo/yarn.lock +++ b/testrepo/yarn.lock @@ -2,7 +2,12 @@ # yarn lockfile v1 +"@rescript/core@*": + version "1.6.1" + resolved "https://registry.yarnpkg.com/@rescript/core/-/core-1.6.1.tgz#159670c94d64a2b8236f46be2bf09a007b1ece08" + integrity sha512-vyb5k90ck+65Fgui+5vCja/mUfzKaK3kOPT4Z6aAJdHLH1eljEi1zKhXroCiCtpNLSWp8k4ulh1bdB5WS0hvqA== + rescript@*: - version "11.0.0" - resolved "https://registry.yarnpkg.com/rescript/-/rescript-11.0.0.tgz#9a0b6fc998c360543c459aba49b77a572a0306cd" - integrity sha512-uIUwDZZmDUb7ymGkBiiGioxMg8hXh1mze/2k/qhYQcZGgi7PrLHQIW9AksM7gb9WnpjCAvFsA8U2VgC0nA468w== + version "11.1.4" + resolved "https://registry.yarnpkg.com/rescript/-/rescript-11.1.4.tgz#9a42ebc4fc5363707e39cef5b3188160b63bee42" + integrity sha512-0bGU0bocihjSC6MsE3TMjHjY0EUpchyrREquLS8VsZ3ohSMD+VHUEwimEfB3kpBI1vYkw3UFZ3WD8R28guz/Vw== From 7f3a4628c5d626d51c77deb706246211623f9eaa Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 4 May 2025 16:50:53 +0200 Subject: [PATCH 2/7] test: improve testsuite and dev deps test case --- tests/compile.sh | 28 +++++++++++++++++----------- tests/utils.sh | 2 +- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/tests/compile.sh b/tests/compile.sh index 2b47d720..65d599bb 100755 --- a/tests/compile.sh +++ b/tests/compile.sh @@ -33,36 +33,36 @@ fi node ./packages/main/src/Main.mjs > ./packages/main/src/output.txt mv ./packages/main/src/Main.res ./packages/main/src/Main2.res -rewatch build --no-timing=true &> ../tests/snapshots/rename-file.txt +rewatch build &> ../tests/snapshots/rename-file.txt mv ./packages/main/src/Main2.res ./packages/main/src/Main.res # Rename a file with a dependent - this should trigger an error mv ./packages/main/src/InternalDep.res ./packages/main/src/InternalDep2.res -rewatch build --no-timing=true &> ../tests/snapshots/rename-file-internal-dep.txt +rewatch build &> ../tests/snapshots/rename-file-internal-dep.txt # replace the absolute path so the snapshot is the same on all machines replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/rename-file-internal-dep.txt mv ./packages/main/src/InternalDep2.res ./packages/main/src/InternalDep.res # Rename a file with a dependent in a namespaced package - this should trigger an error (regression) mv ./packages/new-namespace/src/Other_module.res ./packages/new-namespace/src/Other_module2.res -rewatch build --no-timing=true &> ../tests/snapshots/rename-file-internal-dep-namespace.txt +rewatch build &> ../tests/snapshots/rename-file-internal-dep-namespace.txt # replace the absolute path so the snapshot is the same on all machines replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/rename-file-internal-dep-namespace.txt mv ./packages/new-namespace/src/Other_module2.res ./packages/new-namespace/src/Other_module.res rewatch build &> /dev/null mv ./packages/main/src/ModuleWithInterface.resi ./packages/main/src/ModuleWithInterface2.resi -rewatch build --no-timing=true &> ../tests/snapshots/rename-interface-file.txt +rewatch build &> ../tests/snapshots/rename-interface-file.txt mv ./packages/main/src/ModuleWithInterface2.resi ./packages/main/src/ModuleWithInterface.resi rewatch build &> /dev/null mv ./packages/main/src/ModuleWithInterface.res ./packages/main/src/ModuleWithInterface2.res -rewatch build --no-timing=true &> ../tests/snapshots/rename-file-with-interface.txt +rewatch build &> ../tests/snapshots/rename-file-with-interface.txt mv ./packages/main/src/ModuleWithInterface2.res ./packages/main/src/ModuleWithInterface.res rewatch build &> /dev/null # when deleting a file that other files depend on, the compile should fail rm packages/dep02/src/Dep02.res -rewatch build --no-timing=true &> ../tests/snapshots/remove-file.txt +rewatch build &> ../tests/snapshots/remove-file.txt # replace the absolute path so the snapshot is the same on all machines replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/remove-file.txt git checkout -- packages/dep02/src/Dep02.res @@ -70,14 +70,20 @@ 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 +rewatch build &> ../tests/snapshots/dependency-cycle.txt git checkout -- packages/new-namespace/src/NS_alias.res -rewatch build &> /dev/null # it should compile dev dependencies with the --dev flag -rewatch build --dev &> /dev/null -file_count=$(find ./packages/with-dev-deps -name *.mjs | wc -l) -if [ "$file_count" -eq 2 ]; +rewatch clean &> /dev/null +rewatch build --dev &> /dev/null; +if [ $? -ne 0 ]; +then + error "Failed to compile dev dependencies" + exit 1 +fi + +file_count=$(find ./packages/with-dev-deps/test -name *.mjs | wc -l) +if [ "$file_count" -eq 1 ]; then success "Compiled dev dependencies successfully" else diff --git a/tests/utils.sh b/tests/utils.sh index 3f6218d4..e02b2808 100644 --- a/tests/utils.sh +++ b/tests/utils.sh @@ -3,7 +3,7 @@ overwrite() { echo -e "\r\033[1A\033[0K$@"; } success() { echo -e "- โœ… \033[32m$1\033[0m"; } error() { echo -e "- ๐Ÿ›‘ \033[31m$1\033[0m"; } bold() { echo -e "\033[1m$1\033[0m"; } -rewatch() { RUST_BACKTRACE=1 ../target/release/rewatch --no-timing=true $1; } +rewatch() { RUST_BACKTRACE=1 ../target/release/rewatch --no-timing=true $@; } replace() { if [[ $OSTYPE == 'darwin'* ]]; From 0b25d0bd3eb02775dd3e3e83631909bd17f61a48 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 4 May 2025 17:49:42 +0200 Subject: [PATCH 3/7] fix: do not crash on missing dev deps --- src/build/compile.rs | 79 ++++++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/src/build/compile.rs b/src/build/compile.rs index 16757cfb..ead9c1ea 100644 --- a/src/build/compile.rs +++ b/src/build/compile.rs @@ -364,33 +364,9 @@ pub fn compiler_args( packages: &Option<&AHashMap>, build_dev_deps: bool, ) -> Vec { - let normal_deps = config.bs_dependencies.as_ref().unwrap_or(&vec![]).to_owned(); - let bsc_flags = config::flatten_flags(&config.bsc_flags); - // don't compile dev-deps yet - let dev_deps = if build_dev_deps { - config.bs_dev_dependencies.as_ref().unwrap_or(&vec![]).to_owned() - } else { - vec![] - }; - let deps = [dev_deps, normal_deps] - .concat() - .par_iter() - .map(|package_name| { - let canonicalized_path = if let Some(packages) = packages { - let package = packages.get(package_name).expect("expect package"); - package.path.to_string() - } else { - packages::read_dependency(package_name, project_root, project_root, workspace_root) - .expect("cannot find dep") - }; - vec![ - "-I".to_string(), - packages::get_ocaml_build_path(&canonicalized_path), - ] - }) - .collect::>>(); + let deps = get_deps(config, project_root, workspace_root, packages, build_dev_deps); let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace()); @@ -497,6 +473,59 @@ pub fn compiler_args( .concat() } +fn get_deps( + config: &config::Config, + project_root: &str, + workspace_root: &Option, + packages: &Option<&AHashMap>, + build_dev_deps: bool, +) -> Vec> { + let normal_deps = config + .bs_dependencies + .clone() + .unwrap_or_default() + .into_iter() + .map(|d| (d, false)) + .collect(); + let dev_deps = if build_dev_deps { + config + .bs_dev_dependencies + .clone() + .unwrap_or_default() + .into_iter() + .map(|d| (d, true)) + .collect() + } else { + vec![] + }; + + [dev_deps, normal_deps] + .concat() + .par_iter() + .filter_map(|(package_name, is_dev_dependency)| { + let dep = if let Some(packages) = packages { + packages.get(package_name).map(|package| package.path.to_string()) + } else { + packages::read_dependency(package_name, project_root, project_root, workspace_root).ok() + } + .map(|canonicalized_path| { + vec![ + "-I".to_string(), + packages::get_ocaml_build_path(&canonicalized_path), + ] + }); + + if !is_dev_dependency && dep.is_none() { + panic!( + "Expected to find dependent package {package_name} of {}", + config.name + ); + } + dep + }) + .collect::>>() +} + fn compile_file( package: &packages::Package, root_package: &packages::Package, From 0037f6aca5f097183f2212f91638dc814acf81eb Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 4 May 2025 17:50:01 +0200 Subject: [PATCH 4/7] test: update snapshots --- tests/snapshots/dependency-cycle.txt | 2 +- tests/snapshots/remove-file.txt | 2 +- tests/snapshots/rename-file-internal-dep-namespace.txt | 2 +- tests/snapshots/rename-file-internal-dep.txt | 2 +- tests/snapshots/rename-file-with-interface.txt | 2 +- tests/snapshots/rename-file.txt | 2 +- tests/snapshots/rename-interface-file.txt | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/snapshots/dependency-cycle.txt b/tests/snapshots/dependency-cycle.txt index e8b6db1b..6c5236a5 100644 --- a/tests/snapshots/dependency-cycle.txt +++ b/tests/snapshots/dependency-cycle.txt @@ -1,7 +1,7 @@ [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] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 0/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 0/94 0.00s  [5/7] ๐Ÿงฑ Parsed 1 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] โŒ Compiled 0 modules in 0.00s diff --git a/tests/snapshots/remove-file.txt b/tests/snapshots/remove-file.txt index 4d3a64e6..d17ec131 100644 --- a/tests/snapshots/remove-file.txt +++ b/tests/snapshots/remove-file.txt @@ -1,7 +1,7 @@ [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] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/94 0.00s  [5/7] ๐Ÿงฑ Parsed 0 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] โŒ Compiled 1 modules in 0.00s diff --git a/tests/snapshots/rename-file-internal-dep-namespace.txt b/tests/snapshots/rename-file-internal-dep-namespace.txt index bedaa2d8..72fedd6d 100644 --- a/tests/snapshots/rename-file-internal-dep-namespace.txt +++ b/tests/snapshots/rename-file-internal-dep-namespace.txt @@ -1,7 +1,7 @@ [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] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/94 0.00s  [5/7] ๐Ÿงฑ Parsed 2 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] โŒ Compiled 3 modules in 0.00s diff --git a/tests/snapshots/rename-file-internal-dep.txt b/tests/snapshots/rename-file-internal-dep.txt index deacd3ac..bb0e0f14 100644 --- a/tests/snapshots/rename-file-internal-dep.txt +++ b/tests/snapshots/rename-file-internal-dep.txt @@ -1,7 +1,7 @@ [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] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/94 0.00s  [5/7] ๐Ÿงฑ Parsed 2 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] โŒ Compiled 2 modules in 0.00s diff --git a/tests/snapshots/rename-file-with-interface.txt b/tests/snapshots/rename-file-with-interface.txt index 7f25c664..45d05ea9 100644 --- a/tests/snapshots/rename-file-with-interface.txt +++ b/tests/snapshots/rename-file-with-interface.txt @@ -3,7 +3,7 @@  No implementation file found for interface file (skipping): src/ModuleWithInterface.resi  [2/7] ๐Ÿ‘€ Found source files in 0.00s [3/7] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 2/94 0.00s  [5/7] ๐Ÿงฑ Parsed 1 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] ๐Ÿคบ Compiled 2 modules in 0.00s diff --git a/tests/snapshots/rename-file.txt b/tests/snapshots/rename-file.txt index cf5109cf..7965bd42 100644 --- a/tests/snapshots/rename-file.txt +++ b/tests/snapshots/rename-file.txt @@ -1,7 +1,7 @@ [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] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/94 0.00s  [5/7] ๐Ÿงฑ Parsed 1 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] ๐Ÿคบ Compiled 1 modules in 0.00s diff --git a/tests/snapshots/rename-interface-file.txt b/tests/snapshots/rename-interface-file.txt index 67475cbd..be2cd61a 100644 --- a/tests/snapshots/rename-interface-file.txt +++ b/tests/snapshots/rename-interface-file.txt @@ -3,7 +3,7 @@  No implementation file found for interface file (skipping): src/ModuleWithInterface2.resi  [2/7] ๐Ÿ‘€ Found source files in 0.00s [3/7] ๐Ÿ“ Reading compile state... [3/7] ๐Ÿ“ Read compile state 0.00s -[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/14 0.00s +[4/7] ๐Ÿงน Cleaning up previous build... [4/7] ๐Ÿงน Cleaned 1/94 0.00s  [5/7] ๐Ÿงฑ Parsed 1 source files in 0.00s  [6/7] ๐ŸŒด Collected deps in 0.00s  [7/7] ๐Ÿคบ Compiled 2 modules in 0.00s From 1f93100d246ca19e3300eabf0c7abed7aa0107e3 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 4 May 2025 23:59:46 +0200 Subject: [PATCH 5/7] test: suffix test checks file count in packages --- tests/suffix.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suffix.sh b/tests/suffix.sh index bd9d52f7..37680052 100755 --- a/tests/suffix.sh +++ b/tests/suffix.sh @@ -25,9 +25,9 @@ else fi # Count files with new extension -file_count=$(find . -name *.res.js | wc -l) +file_count=$(find ./packages -name *.res.js | wc -l) -if [ "$file_count" -eq 26 ]; +if [ "$file_count" -eq 24 ]; then success "Found files with correct suffix" else From 676d046fb93d817511ff9039c6c17489a93b2730 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Mon, 5 May 2025 00:00:08 +0200 Subject: [PATCH 6/7] refactor: clean up --- src/build/compile.rs | 47 +++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/src/build/compile.rs b/src/build/compile.rs index ead9c1ea..3a13fbe1 100644 --- a/src/build/compile.rs +++ b/src/build/compile.rs @@ -366,7 +366,8 @@ pub fn compiler_args( ) -> Vec { let bsc_flags = config::flatten_flags(&config.bsc_flags); - let deps = get_deps(config, project_root, workspace_root, packages, build_dev_deps); + let dependency_paths = + get_dependency_paths(config, project_root, workspace_root, packages, build_dev_deps); let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace()); @@ -452,7 +453,7 @@ pub fn compiler_args( namespace_args, read_cmi_args, vec!["-I".to_string(), "../ocaml".to_string()], - deps.concat(), + dependency_paths.concat(), uncurried_args, bsc_flags.to_owned(), warning_args, @@ -473,7 +474,29 @@ pub fn compiler_args( .concat() } -fn get_deps( +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +enum DependentPackage { + Normal(String), + Dev(String), +} + +impl DependentPackage { + fn name(&self) -> &str { + match self { + Self::Normal(name) => name, + Self::Dev(name) => name, + } + } + + fn is_dev(&self) -> bool { + match self { + Self::Normal(_) => false, + Self::Dev(_) => true, + } + } +} + +fn get_dependency_paths( config: &config::Config, project_root: &str, workspace_root: &Option, @@ -485,7 +508,7 @@ fn get_deps( .clone() .unwrap_or_default() .into_iter() - .map(|d| (d, false)) + .map(DependentPackage::Normal) .collect(); let dev_deps = if build_dev_deps { config @@ -493,7 +516,7 @@ fn get_deps( .clone() .unwrap_or_default() .into_iter() - .map(|d| (d, true)) + .map(DependentPackage::Dev) .collect() } else { vec![] @@ -502,8 +525,9 @@ fn get_deps( [dev_deps, normal_deps] .concat() .par_iter() - .filter_map(|(package_name, is_dev_dependency)| { - let dep = if let Some(packages) = packages { + .filter_map(|dependent_package| { + let package_name = dependent_package.name(); + let dependency_path = if let Some(packages) = packages { packages.get(package_name).map(|package| package.path.to_string()) } else { packages::read_dependency(package_name, project_root, project_root, workspace_root).ok() @@ -515,13 +539,14 @@ fn get_deps( ] }); - if !is_dev_dependency && dep.is_none() { + if !dependent_package.is_dev() && dependency_path.is_none() { panic!( - "Expected to find dependent package {package_name} of {}", - config.name + "Expected to find dependent package {} of {}", + package_name, config.name ); } - dep + + dependency_path }) .collect::>>() } From eb8214624f7fe7fcf1f84a5f899ce2003beafb92 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Mon, 5 May 2025 00:03:36 +0200 Subject: [PATCH 7/7] test: fix warning --- src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index c48b5d25..53e13535 100644 --- a/src/config.rs +++ b/src/config.rs @@ -289,7 +289,7 @@ fn check_if_rescript11_or_higher(version: &str) -> Result { fn namespace_from_package_name(package_name: &str) -> String { let len = package_name.len(); let mut buf = String::with_capacity(len); - + fn aux(s: &str, capital: bool, buf: &mut String, off: usize) { if off >= s.len() { return; @@ -529,7 +529,7 @@ mod tests { let config = serde_json::from_str::(json).unwrap(); if let Some(OneOrMore::Multiple(sources)) = config.sources { - let src_dir = sources[0].to_qualified_without_children(None); + assert_eq!(sources.len(), 2); let test_dir = sources[1].to_qualified_without_children(None); assert_eq!(test_dir.type_, Some(String::from("dev")));