diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c7c199062..f380198e9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ - Improve option optimization for constants. https://github.com/rescript-lang/rescript/pull/7913 - Option optimization: do not create redundant local vars. https://github.com/rescript-lang/rescript/pull/7915 - Js output: remove superfluous newline after every `if`. https://github.com/rescript-lang/rescript/pull/7920 +- Rewatch: Traverse upwards for package resolution in single context projects. https://github.com/rescript-lang/rescript/pull/7896 #### :house: Internal diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index b0e192a28e..2d2c0e8d98 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -352,7 +352,7 @@ pub fn flatten_ppx_flags( let path = helpers::try_package_path( package_config, project_context, - format!("{}{}{}", &package_config.name, MAIN_SEPARATOR, y).as_str(), + &format!("{}{}{}", &package_config.name, MAIN_SEPARATOR, y), ) .map(|p| p.to_string_lossy().to_string())?; @@ -374,7 +374,7 @@ pub fn flatten_ppx_flags( Some('.') => helpers::try_package_path( package_config, project_context, - format!("{}{}{}", package_config.name, MAIN_SEPARATOR, &ys[0]).as_str(), + &format!("{}{}{}", package_config.name, MAIN_SEPARATOR, &ys[0]), ) .map(|p| p.to_string_lossy().to_string())?, _ => helpers::try_package_path(package_config, project_context, &ys[0]) diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index 9a85f94e99..553190c488 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -9,6 +9,7 @@ use std::fs::File; use std::io::Read; use std::io::{self, BufRead}; use std::path::{Component, Path, PathBuf}; + use std::time::{SystemTime, UNIX_EPOCH}; pub type StdErr = String; @@ -31,6 +32,36 @@ pub mod emojis { pub static LINE_CLEAR: &str = "\x1b[2K\r"; } +// Cached check: does the given directory contain a node_modules subfolder? +fn has_node_modules_cached(project_context: &ProjectContext, dir: &Path) -> bool { + match project_context.node_modules_exist_cache.read() { + Ok(cache) => { + if let Some(exists) = cache.get(dir) { + return *exists; + } + } + Err(poisoned) => { + log::warn!("node_modules_exist_cache read lock poisoned; recovering"); + let cache = poisoned.into_inner(); + if let Some(exists) = cache.get(dir) { + return *exists; + } + } + } + let exists = dir.join("node_modules").exists(); + match project_context.node_modules_exist_cache.write() { + Ok(mut cache) => { + cache.insert(dir.to_path_buf(), exists); + } + Err(poisoned) => { + log::warn!("node_modules_exist_cache write lock poisoned; recovering"); + let mut cache = poisoned.into_inner(); + cache.insert(dir.to_path_buf(), exists); + } + } + exists +} + /// This trait is used to strip the verbatim prefix from a Windows path. /// On non-Windows systems, it simply returns the original path. /// This is needed until the rescript compiler can handle such paths. @@ -106,6 +137,25 @@ pub fn package_path(root: &Path, package_name: &str) -> PathBuf { root.join("node_modules").join(package_name) } +// Tap-style helper: cache and return the value (single clone for cache insert) +fn cache_package_tap( + project_context: &ProjectContext, + key: &(PathBuf, String), + value: PathBuf, +) -> anyhow::Result { + match project_context.packages_cache.write() { + Ok(mut cache) => { + cache.insert(key.clone(), value.clone()); + } + Err(poisoned) => { + log::warn!("packages_cache write lock poisoned; recovering"); + let mut cache = poisoned.into_inner(); + cache.insert(key.clone(), value.clone()); + } + } + Ok(value) +} + /// Tries to find a path for input package_name. /// The node_modules folder may be found at different levels in the case of a monorepo. /// This helper tries a variety of paths. @@ -114,14 +164,9 @@ pub fn try_package_path( project_context: &ProjectContext, package_name: &str, ) -> anyhow::Result { - // package folder + node_modules + package_name - // This can happen in the following scenario: - // The ProjectContext has a MonoRepoContext::MonorepoRoot. - // We are reading a dependency from the root package. - // And that local dependency has a hoisted dependency. - // Example, we need to find package_name `foo` in the following scenario: - // root/packages/a/node_modules/foo - let path_from_current_package = package_config + // try cached result first, keyed by (package_dir, package_name) + let pkg_name = package_name.to_string(); + let package_dir = package_config .path .parent() .ok_or_else(|| { @@ -129,8 +174,33 @@ pub fn try_package_path( "Expected {} to have a parent folder", package_config.path.to_string_lossy() ) - }) - .map(|parent_path| helpers::package_path(parent_path, package_name))?; + })? + .to_path_buf(); + + let cache_key = (package_dir.clone(), pkg_name.clone()); + match project_context.packages_cache.read() { + Ok(cache) => { + if let Some(cached) = cache.get(&cache_key) { + return Ok(cached.clone()); + } + } + Err(poisoned) => { + log::warn!("packages_cache read lock poisoned; recovering"); + let cache = poisoned.into_inner(); + if let Some(cached) = cache.get(&cache_key) { + return Ok(cached.clone()); + } + } + } + + // package folder + node_modules + package_name + // This can happen in the following scenario: + // The ProjectContext has a MonoRepoContext::MonorepoRoot. + // We are reading a dependency from the root package. + // And that local dependency has a hoisted dependency. + // Example, we need to find package_name `foo` in the following scenario: + // root/packages/a/node_modules/foo + let path_from_current_package = helpers::package_path(&package_dir, package_name); // current folder + node_modules + package_name let path_from_current_config = project_context @@ -148,18 +218,76 @@ pub fn try_package_path( // root folder + node_modules + package_name let path_from_root = package_path(project_context.get_root_path(), package_name); if path_from_current_package.exists() { - Ok(path_from_current_package) + cache_package_tap(project_context, &cache_key, path_from_current_package) } else if path_from_current_config.exists() { - Ok(path_from_current_config) + cache_package_tap(project_context, &cache_key, path_from_current_config) } else if path_from_root.exists() { - Ok(path_from_root) + cache_package_tap(project_context, &cache_key, path_from_root) } else { + // As a last resort, when we're in a Single project context, traverse upwards + // starting from the parent of the package root (package_config.path.parent().parent()) + // and probe each ancestor's node_modules for the dependency. This covers hoisted + // workspace setups when building a package standalone. + if project_context.monorepo_context.is_none() { + match package_config.path.parent().and_then(|p| p.parent()) { + Some(start_dir) => { + return find_dep_in_upward_node_modules(project_context, start_dir, package_name) + .and_then(|p| cache_package_tap(project_context, &cache_key, p)); + } + None => { + log::debug!( + "try_package_path: cannot compute start directory for upward traversal from '{}'", + package_config.path.to_string_lossy() + ); + } + } + } + Err(anyhow!( "The package \"{package_name}\" is not found (are node_modules up-to-date?)..." )) } } +fn find_dep_in_upward_node_modules( + project_context: &ProjectContext, + start_dir: &Path, + package_name: &str, +) -> anyhow::Result { + log::debug!( + "try_package_path: falling back to upward traversal for '{}' starting at '{}'", + package_name, + start_dir.to_string_lossy() + ); + + let mut current = Some(start_dir); + while let Some(dir) = current { + if has_node_modules_cached(project_context, dir) { + let candidate = package_path(dir, package_name); + log::debug!("try_package_path: checking '{}'", candidate.to_string_lossy()); + if candidate.exists() { + log::debug!( + "try_package_path: found '{}' at '{}' via upward traversal", + package_name, + candidate.to_string_lossy() + ); + return Ok(candidate); + } + } + current = dir.parent(); + } + log::debug!( + "try_package_path: no '{}' found during upward traversal from '{}'", + package_name, + start_dir.to_string_lossy() + ); + Err(anyhow!( + "try_package_path: upward traversal did not find '{}' starting at '{}'", + package_name, + start_dir.to_string_lossy() + )) +} + pub fn get_abs_path(path: &Path) -> PathBuf { let abs_path_buf = PathBuf::from(path); diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index edf35e1b4d..439e3d58e2 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -1,12 +1,13 @@ use crate::build::packages; use crate::config::Config; use crate::helpers; -use ahash::AHashSet; +use ahash::{AHashMap, AHashSet}; use anyhow::Result; use anyhow::anyhow; use log::debug; use std::fmt; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::sync::RwLock; pub enum MonoRepoContext { /// Monorepo root - contains local dependencies (symlinked in node_modules) @@ -21,6 +22,8 @@ pub enum MonoRepoContext { pub struct ProjectContext { pub current_config: Config, pub monorepo_context: Option, + pub node_modules_exist_cache: RwLock>, // caches whether a directory contains a node_modules subfolder + pub packages_cache: RwLock>, // caches full results of helpers::try_package_path per (package_dir, package_name) } fn format_dependencies(dependencies: &AHashSet) -> String { @@ -130,6 +133,8 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result Result { diff --git a/rewatch/testrepo/package.json b/rewatch/testrepo/package.json index 4de0800eac..f54aef232d 100644 --- a/rewatch/testrepo/package.json +++ b/rewatch/testrepo/package.json @@ -16,7 +16,8 @@ "packages/file-casing-no-namespace", "packages/pure-dev", "packages/with-ppx", - "packages/nohoist" + "packages/nohoist", + "packages/standalone" ], "nohoist": [ "rescript-bun" @@ -26,11 +27,11 @@ "rescript": "12.0.0-beta.1" }, "scripts": { - "build": "../target/release/rewatch build .", + "build": "../target/release/rescript build .", "build:rescript": "rescript legacy build", - "watch": "../target/release/rewatch watch .", + "watch": "../target/release/rescript watch .", "watch:rescript": "rescript legacy watch", - "clean": "../target/release/rewatch clean .", + "clean": "../target/release/rescript clean .", "clean:rescript": "rescript clean" } } diff --git a/rewatch/testrepo/packages/standalone/package.json b/rewatch/testrepo/packages/standalone/package.json new file mode 100644 index 0000000000..3d21e324b0 --- /dev/null +++ b/rewatch/testrepo/packages/standalone/package.json @@ -0,0 +1,7 @@ +{ + "name": "@testrepo/standalone", + "version": "1.0.0", + "dependencies": { + "@testrepo/dep01": "*" + } +} diff --git a/rewatch/testrepo/packages/standalone/rescript.json b/rewatch/testrepo/packages/standalone/rescript.json new file mode 100644 index 0000000000..c04ff06f22 --- /dev/null +++ b/rewatch/testrepo/packages/standalone/rescript.json @@ -0,0 +1,13 @@ +{ + "name": "standalone", + "sources": { + "dir": "src", + "subdirs": true + }, + "package-specs": { + "module": "es6", + "in-source": true + }, + "suffix": ".mjs", + "dependencies": ["@testrepo/dep01"] +} \ No newline at end of file diff --git a/rewatch/testrepo/packages/standalone/src/Standalone.mjs b/rewatch/testrepo/packages/standalone/src/Standalone.mjs new file mode 100644 index 0000000000..18efa44368 --- /dev/null +++ b/rewatch/testrepo/packages/standalone/src/Standalone.mjs @@ -0,0 +1,13 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Dep01 from "@testrepo/dep01/src/Dep01.mjs"; + +function standalone() { + Dep01.log(); + console.log("standalone"); +} + +export { + standalone, +} +/* Dep01 Not a pure module */ diff --git a/rewatch/testrepo/packages/standalone/src/Standalone.res b/rewatch/testrepo/packages/standalone/src/Standalone.res new file mode 100644 index 0000000000..202c4f430a --- /dev/null +++ b/rewatch/testrepo/packages/standalone/src/Standalone.res @@ -0,0 +1,4 @@ +let standalone = () => { + Dep01.log() + Js.log("standalone") +} \ No newline at end of file diff --git a/rewatch/testrepo/yarn.lock b/rewatch/testrepo/yarn.lock index 153c6db527..5b46a73174 100644 --- a/rewatch/testrepo/yarn.lock +++ b/rewatch/testrepo/yarn.lock @@ -164,6 +164,14 @@ __metadata: languageName: unknown linkType: soft +"@testrepo/standalone@workspace:packages/standalone": + version: 0.0.0-use.local + resolution: "@testrepo/standalone@workspace:packages/standalone" + dependencies: + "@testrepo/dep01": "npm:*" + 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" diff --git a/rewatch/tests/compile.sh b/rewatch/tests/compile.sh index 6e98209466..9287eada3f 100755 --- a/rewatch/tests/compile.sh +++ b/rewatch/tests/compile.sh @@ -34,6 +34,21 @@ else exit 1 fi +# Build from standalone package folder using `rescript build` +bold "Test: Standalone package can build via rescript from package folder" +pushd ./packages/standalone > /dev/null +error_output=$("../../$REWATCH_EXECUTABLE" build 2>&1) +if [ $? -eq 0 ]; +then + success "Standalone package built" +else + error "Error building standalone package" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi +popd > /dev/null + node ./packages/main/src/Main.mjs > ./packages/main/src/output.txt mv ./packages/main/src/Main.res ./packages/main/src/Main2.res