Skip to content

Commit 84e45c5

Browse files
authored
Traverse upwards for single context projects (#7896)
* Traverse upwards for single context projects * Add cache for upward traversal * Add sample to test repo * Invoke rewatch executable * Add changelog * Apply code review feedback * Fix unrelated clippy warnings * Use a fixed rust version so there are no clippy surprises during CI * Add clippy? * Add fmt * Move node_modules_exist_cache to ProjectContext * Cache node_modules exists check instead. * Add project_cache to project_context.
1 parent a034ed7 commit 84e45c5

File tree

11 files changed

+220
-21
lines changed

11 files changed

+220
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
- Improve option optimization for constants. https://github.com/rescript-lang/rescript/pull/7913
3232
- Option optimization: do not create redundant local vars. https://github.com/rescript-lang/rescript/pull/7915
3333
- Js output: remove superfluous newline after every `if`. https://github.com/rescript-lang/rescript/pull/7920
34+
- Rewatch: Traverse upwards for package resolution in single context projects. https://github.com/rescript-lang/rescript/pull/7896
3435

3536
#### :house: Internal
3637

rewatch/src/config.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ pub fn flatten_ppx_flags(
352352
let path = helpers::try_package_path(
353353
package_config,
354354
project_context,
355-
format!("{}{}{}", &package_config.name, MAIN_SEPARATOR, y).as_str(),
355+
&format!("{}{}{}", &package_config.name, MAIN_SEPARATOR, y),
356356
)
357357
.map(|p| p.to_string_lossy().to_string())?;
358358

@@ -374,7 +374,7 @@ pub fn flatten_ppx_flags(
374374
Some('.') => helpers::try_package_path(
375375
package_config,
376376
project_context,
377-
format!("{}{}{}", package_config.name, MAIN_SEPARATOR, &ys[0]).as_str(),
377+
&format!("{}{}{}", package_config.name, MAIN_SEPARATOR, &ys[0]),
378378
)
379379
.map(|p| p.to_string_lossy().to_string())?,
380380
_ => helpers::try_package_path(package_config, project_context, &ys[0])

rewatch/src/helpers.rs

Lines changed: 141 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::fs::File;
99
use std::io::Read;
1010
use std::io::{self, BufRead};
1111
use std::path::{Component, Path, PathBuf};
12+
1213
use std::time::{SystemTime, UNIX_EPOCH};
1314

1415
pub type StdErr = String;
@@ -31,6 +32,36 @@ pub mod emojis {
3132
pub static LINE_CLEAR: &str = "\x1b[2K\r";
3233
}
3334

35+
// Cached check: does the given directory contain a node_modules subfolder?
36+
fn has_node_modules_cached(project_context: &ProjectContext, dir: &Path) -> bool {
37+
match project_context.node_modules_exist_cache.read() {
38+
Ok(cache) => {
39+
if let Some(exists) = cache.get(dir) {
40+
return *exists;
41+
}
42+
}
43+
Err(poisoned) => {
44+
log::warn!("node_modules_exist_cache read lock poisoned; recovering");
45+
let cache = poisoned.into_inner();
46+
if let Some(exists) = cache.get(dir) {
47+
return *exists;
48+
}
49+
}
50+
}
51+
let exists = dir.join("node_modules").exists();
52+
match project_context.node_modules_exist_cache.write() {
53+
Ok(mut cache) => {
54+
cache.insert(dir.to_path_buf(), exists);
55+
}
56+
Err(poisoned) => {
57+
log::warn!("node_modules_exist_cache write lock poisoned; recovering");
58+
let mut cache = poisoned.into_inner();
59+
cache.insert(dir.to_path_buf(), exists);
60+
}
61+
}
62+
exists
63+
}
64+
3465
/// This trait is used to strip the verbatim prefix from a Windows path.
3566
/// On non-Windows systems, it simply returns the original path.
3667
/// 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 {
106137
root.join("node_modules").join(package_name)
107138
}
108139

140+
// Tap-style helper: cache and return the value (single clone for cache insert)
141+
fn cache_package_tap(
142+
project_context: &ProjectContext,
143+
key: &(PathBuf, String),
144+
value: PathBuf,
145+
) -> anyhow::Result<PathBuf> {
146+
match project_context.packages_cache.write() {
147+
Ok(mut cache) => {
148+
cache.insert(key.clone(), value.clone());
149+
}
150+
Err(poisoned) => {
151+
log::warn!("packages_cache write lock poisoned; recovering");
152+
let mut cache = poisoned.into_inner();
153+
cache.insert(key.clone(), value.clone());
154+
}
155+
}
156+
Ok(value)
157+
}
158+
109159
/// Tries to find a path for input package_name.
110160
/// The node_modules folder may be found at different levels in the case of a monorepo.
111161
/// This helper tries a variety of paths.
@@ -114,23 +164,43 @@ pub fn try_package_path(
114164
project_context: &ProjectContext,
115165
package_name: &str,
116166
) -> anyhow::Result<PathBuf> {
117-
// package folder + node_modules + package_name
118-
// This can happen in the following scenario:
119-
// The ProjectContext has a MonoRepoContext::MonorepoRoot.
120-
// We are reading a dependency from the root package.
121-
// And that local dependency has a hoisted dependency.
122-
// Example, we need to find package_name `foo` in the following scenario:
123-
// root/packages/a/node_modules/foo
124-
let path_from_current_package = package_config
167+
// try cached result first, keyed by (package_dir, package_name)
168+
let pkg_name = package_name.to_string();
169+
let package_dir = package_config
125170
.path
126171
.parent()
127172
.ok_or_else(|| {
128173
anyhow!(
129174
"Expected {} to have a parent folder",
130175
package_config.path.to_string_lossy()
131176
)
132-
})
133-
.map(|parent_path| helpers::package_path(parent_path, package_name))?;
177+
})?
178+
.to_path_buf();
179+
180+
let cache_key = (package_dir.clone(), pkg_name.clone());
181+
match project_context.packages_cache.read() {
182+
Ok(cache) => {
183+
if let Some(cached) = cache.get(&cache_key) {
184+
return Ok(cached.clone());
185+
}
186+
}
187+
Err(poisoned) => {
188+
log::warn!("packages_cache read lock poisoned; recovering");
189+
let cache = poisoned.into_inner();
190+
if let Some(cached) = cache.get(&cache_key) {
191+
return Ok(cached.clone());
192+
}
193+
}
194+
}
195+
196+
// package folder + node_modules + package_name
197+
// This can happen in the following scenario:
198+
// The ProjectContext has a MonoRepoContext::MonorepoRoot.
199+
// We are reading a dependency from the root package.
200+
// And that local dependency has a hoisted dependency.
201+
// Example, we need to find package_name `foo` in the following scenario:
202+
// root/packages/a/node_modules/foo
203+
let path_from_current_package = helpers::package_path(&package_dir, package_name);
134204

135205
// current folder + node_modules + package_name
136206
let path_from_current_config = project_context
@@ -148,18 +218,76 @@ pub fn try_package_path(
148218
// root folder + node_modules + package_name
149219
let path_from_root = package_path(project_context.get_root_path(), package_name);
150220
if path_from_current_package.exists() {
151-
Ok(path_from_current_package)
221+
cache_package_tap(project_context, &cache_key, path_from_current_package)
152222
} else if path_from_current_config.exists() {
153-
Ok(path_from_current_config)
223+
cache_package_tap(project_context, &cache_key, path_from_current_config)
154224
} else if path_from_root.exists() {
155-
Ok(path_from_root)
225+
cache_package_tap(project_context, &cache_key, path_from_root)
156226
} else {
227+
// As a last resort, when we're in a Single project context, traverse upwards
228+
// starting from the parent of the package root (package_config.path.parent().parent())
229+
// and probe each ancestor's node_modules for the dependency. This covers hoisted
230+
// workspace setups when building a package standalone.
231+
if project_context.monorepo_context.is_none() {
232+
match package_config.path.parent().and_then(|p| p.parent()) {
233+
Some(start_dir) => {
234+
return find_dep_in_upward_node_modules(project_context, start_dir, package_name)
235+
.and_then(|p| cache_package_tap(project_context, &cache_key, p));
236+
}
237+
None => {
238+
log::debug!(
239+
"try_package_path: cannot compute start directory for upward traversal from '{}'",
240+
package_config.path.to_string_lossy()
241+
);
242+
}
243+
}
244+
}
245+
157246
Err(anyhow!(
158247
"The package \"{package_name}\" is not found (are node_modules up-to-date?)..."
159248
))
160249
}
161250
}
162251

252+
fn find_dep_in_upward_node_modules(
253+
project_context: &ProjectContext,
254+
start_dir: &Path,
255+
package_name: &str,
256+
) -> anyhow::Result<PathBuf> {
257+
log::debug!(
258+
"try_package_path: falling back to upward traversal for '{}' starting at '{}'",
259+
package_name,
260+
start_dir.to_string_lossy()
261+
);
262+
263+
let mut current = Some(start_dir);
264+
while let Some(dir) = current {
265+
if has_node_modules_cached(project_context, dir) {
266+
let candidate = package_path(dir, package_name);
267+
log::debug!("try_package_path: checking '{}'", candidate.to_string_lossy());
268+
if candidate.exists() {
269+
log::debug!(
270+
"try_package_path: found '{}' at '{}' via upward traversal",
271+
package_name,
272+
candidate.to_string_lossy()
273+
);
274+
return Ok(candidate);
275+
}
276+
}
277+
current = dir.parent();
278+
}
279+
log::debug!(
280+
"try_package_path: no '{}' found during upward traversal from '{}'",
281+
package_name,
282+
start_dir.to_string_lossy()
283+
);
284+
Err(anyhow!(
285+
"try_package_path: upward traversal did not find '{}' starting at '{}'",
286+
package_name,
287+
start_dir.to_string_lossy()
288+
))
289+
}
290+
163291
pub fn get_abs_path(path: &Path) -> PathBuf {
164292
let abs_path_buf = PathBuf::from(path);
165293

rewatch/src/project_context.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use crate::build::packages;
22
use crate::config::Config;
33
use crate::helpers;
4-
use ahash::AHashSet;
4+
use ahash::{AHashMap, AHashSet};
55
use anyhow::Result;
66
use anyhow::anyhow;
77
use log::debug;
88
use std::fmt;
9-
use std::path::Path;
9+
use std::path::{Path, PathBuf};
10+
use std::sync::RwLock;
1011

1112
pub enum MonoRepoContext {
1213
/// Monorepo root - contains local dependencies (symlinked in node_modules)
@@ -21,6 +22,8 @@ pub enum MonoRepoContext {
2122
pub struct ProjectContext {
2223
pub current_config: Config,
2324
pub monorepo_context: Option<MonoRepoContext>,
25+
pub node_modules_exist_cache: RwLock<AHashMap<PathBuf, bool>>, // caches whether a directory contains a node_modules subfolder
26+
pub packages_cache: RwLock<AHashMap<(PathBuf, String), PathBuf>>, // caches full results of helpers::try_package_path per (package_dir, package_name)
2427
}
2528

2629
fn format_dependencies(dependencies: &AHashSet<String>) -> String {
@@ -130,6 +133,8 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result<Pro
130133
Ok(ProjectContext {
131134
current_config,
132135
monorepo_context: None,
136+
node_modules_exist_cache: RwLock::new(AHashMap::new()),
137+
packages_cache: RwLock::new(AHashMap::new()),
133138
})
134139
} else {
135140
Ok(ProjectContext {
@@ -138,6 +143,8 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result<Pro
138143
local_dependencies,
139144
local_dev_dependencies,
140145
}),
146+
node_modules_exist_cache: RwLock::new(AHashMap::new()),
147+
packages_cache: RwLock::new(AHashMap::new()),
141148
})
142149
}
143150
}
@@ -187,6 +194,8 @@ impl ProjectContext {
187194
monorepo_context: Some(MonoRepoContext::MonorepoPackage {
188195
parent_config: Box::new(workspace_config),
189196
}),
197+
node_modules_exist_cache: RwLock::new(AHashMap::new()),
198+
packages_cache: RwLock::new(AHashMap::new()),
190199
})
191200
}
192201
Ok(_) => {

rewatch/testrepo/package.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
"packages/file-casing-no-namespace",
1717
"packages/pure-dev",
1818
"packages/with-ppx",
19-
"packages/nohoist"
19+
"packages/nohoist",
20+
"packages/standalone"
2021
],
2122
"nohoist": [
2223
"rescript-bun"
@@ -26,11 +27,11 @@
2627
"rescript": "12.0.0-beta.1"
2728
},
2829
"scripts": {
29-
"build": "../target/release/rewatch build .",
30+
"build": "../target/release/rescript build .",
3031
"build:rescript": "rescript legacy build",
31-
"watch": "../target/release/rewatch watch .",
32+
"watch": "../target/release/rescript watch .",
3233
"watch:rescript": "rescript legacy watch",
33-
"clean": "../target/release/rewatch clean .",
34+
"clean": "../target/release/rescript clean .",
3435
"clean:rescript": "rescript clean"
3536
}
3637
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "@testrepo/standalone",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@testrepo/dep01": "*"
6+
}
7+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"name": "standalone",
3+
"sources": {
4+
"dir": "src",
5+
"subdirs": true
6+
},
7+
"package-specs": {
8+
"module": "es6",
9+
"in-source": true
10+
},
11+
"suffix": ".mjs",
12+
"dependencies": ["@testrepo/dep01"]
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
import * as Dep01 from "@testrepo/dep01/src/Dep01.mjs";
4+
5+
function standalone() {
6+
Dep01.log();
7+
console.log("standalone");
8+
}
9+
10+
export {
11+
standalone,
12+
}
13+
/* Dep01 Not a pure module */
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
let standalone = () => {
2+
Dep01.log()
3+
Js.log("standalone")
4+
}

rewatch/testrepo/yarn.lock

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ __metadata:
164164
languageName: unknown
165165
linkType: soft
166166

167+
"@testrepo/standalone@workspace:packages/standalone":
168+
version: 0.0.0-use.local
169+
resolution: "@testrepo/standalone@workspace:packages/standalone"
170+
dependencies:
171+
"@testrepo/dep01": "npm:*"
172+
languageName: unknown
173+
linkType: soft
174+
167175
"@testrepo/with-dev-deps@workspace:packages/with-dev-deps":
168176
version: 0.0.0-use.local
169177
resolution: "@testrepo/with-dev-deps@workspace:packages/with-dev-deps"

0 commit comments

Comments
 (0)