diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bca0d5da..f72308fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/examples/demo/package.json b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/examples/demo/package.json new file mode 100644 index 000000000..eabc90957 --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/examples/demo/package.json @@ -0,0 +1,6 @@ +{ + "name": "demo", + "scripts": { + "build": "vt tool print demo" + } +} diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/package.json b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/package.json new file mode 100644 index 000000000..1d397f3c2 --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/package.json @@ -0,0 +1,10 @@ +{ + "name": "root", + "workspaces": [ + "packages/*", + "examples/*" + ], + "scripts": { + "build": "vt tool print root" + } +} diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/packages/app/package.json b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/packages/app/package.json new file mode 100644 index 000000000..de461c161 --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/packages/app/package.json @@ -0,0 +1,6 @@ +{ + "name": "app", + "scripts": { + "build": "vt tool print app" + } +} diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/pnpm-workspace.yaml b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/pnpm-workspace.yaml new file mode 100644 index 000000000..18ec407ef --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - 'packages/*' diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots.toml b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots.toml new file mode 100644 index 000000000..4dba57cb6 --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots.toml @@ -0,0 +1,10 @@ +# A package excluded by pnpm stays standalone even when package.json workspaces includes it. + +[fields] +key = true +execution_item_display = { command = true } + +[[plan]] +name = "non_member_package_is_its_own_workspace" +args = ["run", "--recursive", "build"] +cwd = "examples/demo" diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/query_non_member_package_is_its_own_workspace.jsonc b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/query_non_member_package_is_its_own_workspace.jsonc new file mode 100644 index 000000000..72af26126 --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/query_non_member_package_is_its_own_workspace.jsonc @@ -0,0 +1,20 @@ +// run --recursive build +{ + "graph": [ + { + "key": [ + "/examples/demo/", + "build" + ], + "node": { + "items": [ + { + "execution_item_display": { + "command": "vt tool print demo" + } + } + ] + } + } + ] +} diff --git a/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/task_graph.md b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/task_graph.md new file mode 100644 index 000000000..e2d15939c --- /dev/null +++ b/crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/task_graph.md @@ -0,0 +1,88 @@ +# task graph + +```mermaid +flowchart TD + task_0["/#build"] + task_1["/packages/app#build"] +``` + +## `/#build` + +```json +{ + "task_display": { + "package_name": "root", + "task_name": "build", + "package_path": "/" + }, + "resolved_config": { + "commands": [ + "vt tool print root" + ], + "resolved_options": { + "cwd": "/", + "cache_config": { + "remote_cache_allowed": true, + "env_config": { + "fingerprinted_envs": [], + "untracked_env": [ + "" + ] + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + }, + "output_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + } + } + }, + "source": "PackageJsonScript" +} +``` + +## `/packages/app#build` + +```json +{ + "task_display": { + "package_name": "app", + "task_name": "build", + "package_path": "/packages/app" + }, + "resolved_config": { + "commands": [ + "vt tool print app" + ], + "resolved_options": { + "cwd": "/packages/app", + "cache_config": { + "remote_cache_allowed": true, + "env_config": { + "fingerprinted_envs": [], + "untracked_env": [ + "" + ] + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + }, + "output_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + } + } + }, + "source": "PackageJsonScript" +} +``` + diff --git a/crates/vt_workspace/src/lib.rs b/crates/vt_workspace/src/lib.rs index 6d86c2167..ec8b22a7f 100644 --- a/crates/vt_workspace/src/lib.rs +++ b/crates/vt_workspace/src/lib.rs @@ -11,7 +11,7 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; use serde::Deserialize; use vec1::smallvec_v1::SmallVec1; use vt_glob::path::PathGlobSet; -use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf}; +use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePath, RelativePathBuf}; use vt_str::Str; use wax::{ Glob, @@ -88,20 +88,29 @@ struct WorkspaceMemberGlobs { workspaces: Vec, } impl WorkspaceMemberGlobs { - const fn new(workspaces: Vec) -> Self { - Self { workspaces } + fn from_pnpm_workspace_yaml(file_with_path: &FileWithPath) -> Result { + let workspace: PnpmWorkspace = + serde_norway::from_slice(strip_bom(file_with_path.content())).map_err(|e| { + Error::SerdeYaml { + file_path: Arc::clone(file_with_path.path()), + serde_yaml_error: e, + } + })?; + Ok(Self { workspaces: workspace.packages }) } - fn get_package_json_paths( - self, - workspace_root: impl AsRef, - ) -> Result, Error> { - let workspace_root = workspace_root.as_ref(); - let mut package_json_paths = HashSet::::default(); - let mut has_negated = false; - let mut inclusions = Vec::::new(); - let mut all = Vec::::new(); - for pattern in self.workspaces { + fn from_npm_workspace_json(file_with_path: &FileWithPath) -> Result { + let workspace: NpmWorkspace = serde_json::from_slice(strip_bom(file_with_path.content())) + .map_err(|e| Error::SerdeJson { + file_path: Arc::clone(file_with_path.path()), + serde_json_error: e, + })?; + Ok(Self { workspaces: workspace.workspaces.into_packages() }) + } + + /// Converts each member glob into a glob of `package.json` files, paired with whether it is negated. + fn package_json_patterns(&self) -> impl Iterator + '_ { + self.workspaces.iter().map(|pattern| { // Ported from npm's workspace pattern normalization: // https://github.com/npm/map-workspaces/blob/76ea3c852171790e245052e006329de15b09e60e/lib/index.js#L17 let exclusion_count = pattern.bytes().take_while(|byte| *byte == b'!').count(); @@ -124,7 +133,31 @@ impl WorkspaceMemberGlobs { } else { "/package.json" }); + (pattern, is_negated) + }) + } + /// Checks whether the package at `package_path`, relative to the workspace root, is a member. + fn includes(&self, package_path: &RelativePath) -> Result { + // `get_package_json_paths` never walks into `node_modules`. + if package_path.as_str().split('/').any(|component| component == "node_modules") { + return Ok(false); + } + let patterns = self.package_json_patterns().map(|(pattern, _)| pattern).collect::>(); + let package_json_path = vt_str::format!("{}/package.json", package_path.as_str()); + Ok(PathGlobSet::new(&patterns)?.is_match(package_json_path.as_str())) + } + + fn get_package_json_paths( + self, + workspace_root: impl AsRef, + ) -> Result, Error> { + let workspace_root = workspace_root.as_ref(); + let mut package_json_paths = HashSet::::default(); + let mut has_negated = false; + let mut inclusions = Vec::::new(); + let mut all = Vec::::new(); + for (pattern, is_negated) in self.package_json_patterns() { if is_negated { has_negated = true; } else { @@ -283,26 +316,12 @@ pub fn load_package_graph( workspace_root: &WorkspaceRoot, ) -> Result, Error> { let mut graph_builder = PackageGraphBuilder::default(); - let workspaces = match &workspace_root.workspace_file { + let member_globs = match &workspace_root.workspace_file { WorkspaceFile::PnpmWorkspaceYaml(file_with_path) => { - let workspace: PnpmWorkspace = - serde_norway::from_slice(strip_bom(file_with_path.content())).map_err(|e| { - Error::SerdeYaml { - file_path: Arc::clone(file_with_path.path()), - serde_yaml_error: e, - } - })?; - workspace.packages + WorkspaceMemberGlobs::from_pnpm_workspace_yaml(file_with_path)? } WorkspaceFile::NpmWorkspaceJson(file_with_path) => { - let workspace: NpmWorkspace = - serde_json::from_slice(strip_bom(file_with_path.content())).map_err(|e| { - Error::SerdeJson { - file_path: Arc::clone(file_with_path.path()), - serde_json_error: e, - } - })?; - workspace.workspaces.into_packages() + WorkspaceMemberGlobs::from_npm_workspace_json(file_with_path)? } WorkspaceFile::NonWorkspacePackage(file_with_path) => { // For non-workspace packages, add the package.json to the graph as a root package @@ -323,7 +342,6 @@ pub fn load_package_graph( } }; - let member_globs = WorkspaceMemberGlobs::new(workspaces); let mut has_root_package = false; for package_json_path in member_globs.get_package_json_paths(&*workspace_root.path)? { let package_json_path: Arc = package_json_path.clone().into(); diff --git a/crates/vt_workspace/src/package_manager.rs b/crates/vt_workspace/src/package_manager.rs index 84f99b922..92699cb3e 100644 --- a/crates/vt_workspace/src/package_manager.rs +++ b/crates/vt_workspace/src/package_manager.rs @@ -2,7 +2,7 @@ use std::{fs, sync::Arc}; use vt_path::{AbsolutePath, RelativePathBuf}; -use crate::Error; +use crate::{Error, WorkspaceMemberGlobs}; /// The contents of a file bundled with its absolute path for error context. /// @@ -123,6 +123,9 @@ pub struct WorkspaceRoot { /// /// Returns the workspace root and the relative path from the workspace root to the original cwd. /// +/// Like npm, a workspace is only used if its member globs include the package nearest to `original_cwd`, so a package +/// nested in a workspace directory without being declared as a member is not treated as part of that workspace. +/// /// If the workspace file is not found, but a package is found, `workspace_file` will be `NonWorkspacePackage` with the `package.json` File. /// /// If neither workspace nor package is found, will return `PackageJsonNotFound` error. @@ -136,11 +139,18 @@ pub fn find_workspace_root( original_cwd: &AbsolutePath, ) -> Result<(WorkspaceRoot, RelativePathBuf), Error> { let mut cwd = original_cwd; + let mut nearest_package: Option<&AbsolutePath> = None; loop { // Check for pnpm-workspace.yaml for pnpm workspace let pnpm_workspace_path: Arc = cwd.join("pnpm-workspace.yaml").into(); - if let Some(file_with_path) = FileWithPath::open_if_exists(pnpm_workspace_path)? { + let pnpm_workspace = FileWithPath::open_if_exists(pnpm_workspace_path)?; + let is_pnpm_workspace = pnpm_workspace.is_some(); + if let Some(file_with_path) = pnpm_workspace + && includes_package(cwd, nearest_package, || { + WorkspaceMemberGlobs::from_pnpm_workspace_yaml(&file_with_path) + })? + { let relative_cwd = original_cwd.strip_prefix(cwd)?.expect("cwd must be within the pnpm workspace"); return Ok(( @@ -162,7 +172,13 @@ pub fn find_workspace_root( file_path: Arc::clone(file_with_path.path()), serde_json_error: e, })?; - if package_json.get("workspaces").is_some() { + // pnpm defines membership even when its globs exclude the current package. + if !is_pnpm_workspace + && package_json.get("workspaces").is_some() + && includes_package(cwd, nearest_package, || { + WorkspaceMemberGlobs::from_npm_workspace_json(&file_with_path) + })? + { let relative_cwd = original_cwd.strip_prefix(cwd)?.expect("cwd must be within the workspace"); return Ok(( @@ -173,6 +189,7 @@ pub fn find_workspace_root( relative_cwd, )); } + nearest_package.get_or_insert(cwd); } // TODO(@fengmk2): other package manager support @@ -192,6 +209,24 @@ pub fn find_workspace_root( } } +/// Checks whether the workspace at `workspace_root` includes the nearest package found below it. +/// +/// Without a package below it, `original_cwd` belongs to the workspace root package itself. +fn includes_package( + workspace_root: &AbsolutePath, + nearest_package: Option<&AbsolutePath>, + member_globs: impl FnOnce() -> Result, +) -> Result { + let Some(nearest_package) = nearest_package else { + return Ok(true); + }; + // SAFETY: `nearest_package` is found while walking up from `original_cwd` to `workspace_root`. + let package_path = nearest_package + .strip_prefix(workspace_root)? + .expect("nearest package must be within the workspace root"); + member_globs()?.includes(&package_path) +} + #[cfg(test)] mod tests { use tempfile::TempDir;