Skip to content

Fix sourcedirs.json generation #7671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@

- Add rust linting to CI with `clippy`. https://github.com/rescript-lang/rescript/pull/7675

#### :bug: Bug fix

- Fix `--create-sourcedirs` generation with for a single project. https://github.com/rescript-lang/rescript/pull/7671

# 12.0.0-beta.2

#### :boom: Breaking Change
Expand Down
41 changes: 26 additions & 15 deletions rewatch/src/build/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,14 +331,14 @@ fn read_dependencies(
let (config, canonical_path) =
match read_dependency(package_name, parent_path, project_root, workspace_root) {
Err(error) => {
if show_progress {
println!(
"{} {} Error building package tree. {}",
style("[1/2]").bold().dim(),
CROSS,
error
);
}
if show_progress {
println!(
"{} {} Error building package tree. {}",
style("[1/2]").bold().dim(),
CROSS,
error
);
}

let parent_path_str = parent_path.to_string_lossy();
log::error!(
Expand All @@ -356,9 +356,9 @@ fn read_dependencies(
"We could not build package tree '{package_name}', at path '{parent_path_str}'. Error: {error}",
);
std::process::exit(2)
}
}
}
}
}
};

let is_pinned = parent_config
Expand All @@ -374,7 +374,7 @@ fn read_dependencies(
project_root,
workspace_root,
show_progress,
build_dev_deps
build_dev_deps,
);

Dependency {
Expand Down Expand Up @@ -413,7 +413,13 @@ pub fn read_package_name(package_dir: &Path) -> Result<String> {
.ok_or_else(|| anyhow!("No name field found in package.json"))
}

fn make_package(config: config::Config, package_path: &Path, is_pinned_dep: bool, is_root: bool) -> Package {
fn make_package(
config: config::Config,
package_path: &Path,
is_pinned_dep: bool,
is_root: bool,
project_root: &Path,
) -> Package {
let source_folders = match config.sources.to_owned() {
Some(config::OneOrMore::Single(source)) => get_source_dirs(source, None),
Some(config::OneOrMore::Multiple(sources)) => {
Expand Down Expand Up @@ -452,6 +458,11 @@ This inconsistency will cause issues with package resolution.\n",
);
}

let is_local_dep = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a simpler fix for the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

package_path.starts_with(project_root)
&& !package_path.components().any(|c| c.as_os_str() == "node_modules")
};

Package {
name: package_name,
config: config.to_owned(),
Expand All @@ -466,7 +477,7 @@ This inconsistency will cause issues with package resolution.\n",
.expect("Could not canonicalize"),
dirs: None,
is_pinned_dep,
is_local_dep: !package_path.components().any(|c| c.as_os_str() == "node_modules"),
is_local_dep,
is_root,
}
}
Expand All @@ -481,7 +492,7 @@ fn read_packages(

// Store all packages and completely deduplicate them
let mut map: AHashMap<String, Package> = AHashMap::new();
let root_package = make_package(root_config.to_owned(), project_root, false, true);
let root_package = make_package(root_config.to_owned(), project_root, false, true, project_root);
map.insert(root_package.name.to_string(), root_package);

let mut registered_dependencies_set: AHashSet<String> = AHashSet::new();
Expand All @@ -496,7 +507,7 @@ fn read_packages(
));
dependencies.iter().for_each(|d| {
if !map.contains_key(&d.name) {
let package = make_package(d.config.to_owned(), &d.path, d.is_pinned, false);
let package = make_package(d.config.to_owned(), &d.path, d.is_pinned, false, project_root);
map.insert(d.name.to_string(), package);
}
});
Expand Down
25 changes: 14 additions & 11 deletions rewatch/src/sourcedirs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ pub struct SourceDirs<'a> {
}

fn package_to_dirs(package: &Package, root_package_path: &Path) -> AHashSet<Dir> {
let relative_path = package.path.strip_prefix(root_package_path).unwrap();

package
.dirs
.as_ref()
.unwrap_or(&AHashSet::new())
.iter()
.map(|path| relative_path.join(path))
.collect::<AHashSet<PathBuf>>()
match package.path.strip_prefix(root_package_path) {
Err(_) => AHashSet::new(),
Ok(relative_path) => package
.dirs
.as_ref()
.unwrap_or(&AHashSet::new())
.iter()
.map(|path| relative_path.join(path))
.collect::<AHashSet<PathBuf>>(),
}
}

fn deps_to_pkgs<'a>(
Expand Down Expand Up @@ -61,11 +62,13 @@ pub fn print(buildstate: &BuildState) {
.find(|(_name, package)| package.is_root)
.expect("Could not find root package");

// Take all packages apart from the root package
// Take all local packages with source files.
// In the case of a monorepo, the root package typically won't have any source files.
// But in the case of a single package, it will be both local, root and have source files.
let (dirs, pkgs): (Vec<AHashSet<Dir>>, Vec<AHashMap<PackageName, AbsolutePath>>) = buildstate
.packages
.par_iter()
.filter(|(_name, package)| !package.is_root)
.filter(|(_name, package)| package.is_local_dep && package.source_files.is_some())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case there is only a single rescript.json, this will also ensure it gets processed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal here is to find all packages except for the root package, why is this behavior changing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh the root is never a local dep, and we don't want to have any other deps? Maybe you should update the comment if people run into this code later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When scaffolding a new (single) rescript project:

Image

it is both root and local, which is fine.
You only need to avoid the root package if it has no source files. So reanalyze can run on it.

.map(|(_name, package)| {
// Extract Directories
let dirs = package_to_dirs(package, &root_package.path);
Expand Down
Loading