fix(task): --recursive - order tasks by package dependencies (#30129)

Closes https://github.com/denoland/deno/issues/29377

cli/tools/task.rs

@@ -99,14 +99,16 @@ pub async fn execute_script(
     let mut packages_task_info: Vec<PackageTaskInfo> = vec![];
 
     let workspace = cli_options.workspace();
-    for folder in workspace.config_folders() {
+    for (folder_url, folder) in
+      workspace.config_folders_sorted_by_dependencies()
+    {
       if !task_flags.recursive
-        && !matches_package(folder.1, force_use_pkg_json, &package_regex)
+        && !matches_package(folder, force_use_pkg_json, &package_regex)
       {
         continue;
       }
 
-      let member_dir = workspace.resolve_member_dir(folder.0);
+      let member_dir = workspace.resolve_member_dir(folder_url);
       let mut tasks_config = member_dir.to_tasks_config()?;
       if force_use_pkg_json {
         tasks_config = tasks_config.with_only_pkg_json();
@@ -727,27 +729,26 @@ fn print_available_tasks_workspace(
   let workspace = cli_options.workspace();
 
   let mut matched = false;
-  for folder in workspace.config_folders() {
-    if !recursive
-      && !matches_package(folder.1, force_use_pkg_json, package_regex)
+  for (folder_url, folder) in workspace.config_folders_sorted_by_dependencies()
+  {
+    if !recursive && !matches_package(folder, force_use_pkg_json, package_regex)
     {
       continue;
     }
     matched = true;
 
-    let member_dir = workspace.resolve_member_dir(folder.0);
+    let member_dir = workspace.resolve_member_dir(folder_url);
     let mut tasks_config = member_dir.to_tasks_config()?;
 
     let mut pkg_name = folder
-      .1
       .deno_json
       .as_ref()
       .and_then(|deno| deno.json.name.clone())
-      .or(folder.1.pkg_json.as_ref().and_then(|pkg| pkg.name.clone()));
+      .or(folder.pkg_json.as_ref().and_then(|pkg| pkg.name.clone()));
 
     if force_use_pkg_json {
       tasks_config = tasks_config.with_only_pkg_json();
-      pkg_name = folder.1.pkg_json.as_ref().and_then(|pkg| pkg.name.clone());
+      pkg_name = folder.pkg_json.as_ref().and_then(|pkg| pkg.name.clone());
     }
 
     print_available_tasks(

libs/config/Cargo.toml

@@ -13,10 +13,10 @@ path = "lib.rs"
 
 [features]
 default = ["workspace"]
-deno_json = ["jsonc-parser", "glob", "ignore", "import_map"]
+deno_json = ["deno_semver", "jsonc-parser", "glob", "ignore", "import_map"]
 package_json = ["deno_package_json"]
 sync = ["deno_package_json/sync"]
-workspace = ["deno_json", "deno_semver", "package_json"]
+workspace = ["deno_json", "package_json"]
 
 [dependencies]
 boxed_error.workspace = true

libs/config/deno_json/mod.rs

@@ -2,6 +2,7 @@
 
 use std::borrow::Cow;
 use std::collections::BTreeMap;
+use std::collections::HashSet;
 use std::path::Path;
 use std::path::PathBuf;
 
@@ -10,6 +11,7 @@ use deno_error::JsError;
 use deno_path_util::url_from_file_path;
 use deno_path_util::url_parent;
 use deno_path_util::url_to_file_path;
+use deno_semver::jsr::JsrDepPackageReq;
 use import_map::ImportMapWithDiagnostics;
 use indexmap::IndexMap;
 use jsonc_parser::ParseResult;
@@ -28,6 +30,10 @@ use url::Url;
 use crate::UrlToFilePathError;
 use crate::glob::FilePatterns;
 use crate::glob::PathOrPatternSet;
+use crate::import_map::imports_values;
+use crate::import_map::scope_values;
+use crate::import_map::value_to_dep_req;
+use crate::import_map::values_to_set;
 use crate::util::is_skippable_io_error;
 
 mod ts;
@@ -1850,6 +1856,48 @@ impl ConfigFile {
       }
     }
   }
+
+  pub fn dependencies(&self) -> HashSet<JsrDepPackageReq> {
+    let values = imports_values(self.json.imports.as_ref())
+      .into_iter()
+      .chain(scope_values(self.json.scopes.as_ref()));
+    let mut set = values_to_set(values);
+
+    if let Some(serde_json::Value::Object(compiler_options)) =
+      &self.json.compiler_options
+    {
+      // add jsxImportSource
+      if let Some(serde_json::Value::String(value)) =
+        compiler_options.get("jsxImportSource")
+      {
+        if let Some(dep_req) = value_to_dep_req(value) {
+          set.insert(dep_req);
+        }
+      }
+      // add jsxImportSourceTypes
+      if let Some(serde_json::Value::String(value)) =
+        compiler_options.get("jsxImportSourceTypes")
+      {
+        if let Some(dep_req) = value_to_dep_req(value) {
+          set.insert(dep_req);
+        }
+      }
+      // add the dependencies in the types array
+      if let Some(serde_json::Value::Array(types)) =
+        compiler_options.get("types")
+      {
+        for value in types {
+          if let serde_json::Value::String(value) = value {
+            if let Some(dep_req) = value_to_dep_req(value) {
+              set.insert(dep_req);
+            }
+          }
+        }
+      }
+    }
+
+    set
+  }
 }
 
 #[cfg(test)]

libs/config/import_map.rs

@@ -0,0 +1,62 @@
+// Copyright 2018-2025 the Deno authors. MIT license.
+
+use std::collections::HashSet;
+
+use deno_semver::jsr::JsrDepPackageReq;
+use deno_semver::jsr::JsrPackageReqReference;
+use deno_semver::npm::NpmPackageReqReference;
+
+/// Attempts to resolve any `npm:` and `jsr:` dependencies
+/// in the import map's imports and scopes.
+pub fn import_map_deps(
+  import_map: &serde_json::Value,
+) -> HashSet<JsrDepPackageReq> {
+  let values = imports_values(import_map.get("imports"))
+    .into_iter()
+    .chain(scope_values(import_map.get("scopes")));
+  values_to_set(values)
+}
+
+pub(crate) fn imports_values(
+  value: Option<&serde_json::Value>,
+) -> Vec<&String> {
+  let Some(obj) = value.and_then(|v| v.as_object()) else {
+    return Vec::new();
+  };
+  let mut items = Vec::with_capacity(obj.len());
+  for value in obj.values() {
+    if let serde_json::Value::String(value) = value {
+      items.push(value);
+    }
+  }
+  items
+}
+
+pub(crate) fn scope_values(value: Option<&serde_json::Value>) -> Vec<&String> {
+  let Some(obj) = value.and_then(|v| v.as_object()) else {
+    return Vec::new();
+  };
+  obj.values().flat_map(|v| imports_values(Some(v))).collect()
+}
+
+pub(crate) fn values_to_set<'a>(
+  values: impl Iterator<Item = &'a String>,
+) -> HashSet<JsrDepPackageReq> {
+  let mut entries = HashSet::new();
+  for value in values {
+    if let Some(dep_req) = value_to_dep_req(value) {
+      entries.insert(dep_req);
+    }
+  }
+  entries
+}
+
+pub(crate) fn value_to_dep_req(value: &str) -> Option<JsrDepPackageReq> {
+  match JsrPackageReqReference::from_str(value) {
+    Ok(req_ref) => Some(JsrDepPackageReq::jsr(req_ref.into_inner().req)),
+    _ => match NpmPackageReqReference::from_str(value) {
+      Ok(req_ref) => Some(JsrDepPackageReq::npm(req_ref.into_inner().req)),
+      _ => None,
+    },
+  }
+}

libs/config/lib.rs

@@ -10,6 +10,8 @@ pub mod deno_json;
 #[cfg(feature = "deno_json")]
 pub mod glob;
 #[cfg(feature = "deno_json")]
+pub mod import_map;
+#[cfg(feature = "deno_json")]
 mod sync;
 #[cfg(feature = "deno_json")]
 mod util;

libs/config/workspace/mod.rs

@@ -10,6 +10,8 @@ use std::path::PathBuf;
 use boxed_error::Boxed;
 use deno_error::JsError;
 use deno_package_json::PackageJson;
+use deno_package_json::PackageJsonDepValue;
+use deno_package_json::PackageJsonDepWorkspaceReq;
 use deno_package_json::PackageJsonLoadError;
 use deno_package_json::PackageJsonRc;
 use deno_path_util::url_from_directory_path;
@@ -18,6 +20,8 @@ use deno_path_util::url_to_file_path;
 use deno_semver::RangeSetOrTag;
 use deno_semver::Version;
 use deno_semver::VersionReq;
+use deno_semver::jsr::JsrDepPackageReq;
+use deno_semver::package::PackageKind;
 use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
 use discovery::ConfigFileDiscovery;
@@ -462,6 +466,225 @@ impl Workspace {
     &self.config_folders
   }
 
+  /// Gets the folders sorted by whether they have a dependency on each other.
+  pub fn config_folders_sorted_by_dependencies(
+    &self,
+  ) -> IndexMap<&UrlRc, &FolderConfigs> {
+    struct PackageNameMaybeVersion<'a> {
+      name: &'a str,
+      version: Option<Version>,
+    }
+
+    enum Dep {
+      Req(JsrDepPackageReq),
+      Path(Url),
+    }
+
+    impl Dep {
+      pub fn matches_pkg(
+        &self,
+        package_kind: PackageKind,
+        pkg: &PackageNameMaybeVersion,
+        folder_url: &Url,
+      ) -> bool {
+        match self {
+          Dep::Req(req) => {
+            req.kind == package_kind
+              && req.req.name == pkg.name
+              && pkg
+                .version
+                .as_ref()
+                .map(|v| {
+                  // just match if it's a tag
+                  req.req.version_req.tag().is_some()
+                    || req.req.version_req.matches(v)
+                })
+                .unwrap_or(true)
+          }
+          Dep::Path(url) => {
+            folder_url.as_str().trim_end_matches('/')
+              == url.as_str().trim_end_matches('/')
+          }
+        }
+      }
+    }
+
+    struct Folder<'a> {
+      index: usize,
+      dir_url: &'a UrlRc,
+      folder: &'a FolderConfigs,
+      npm_nv: Option<PackageNameMaybeVersion<'a>>,
+      jsr_nv: Option<PackageNameMaybeVersion<'a>>,
+      deps: Vec<Dep>,
+    }
+
+    impl<'a> Folder<'a> {
+      pub fn depends_on(&self, other: &Folder<'a>) -> bool {
+        if let Some(other_nv) = &other.npm_nv {
+          if self.has_matching_dep(PackageKind::Npm, other_nv, other.dir_url) {
+            return true;
+          }
+        }
+        if let Some(other_nv) = &other.jsr_nv {
+          if self.has_matching_dep(PackageKind::Jsr, other_nv, other.dir_url) {
+            return true;
+          }
+        }
+        false
+      }
+
+      fn has_matching_dep(
+        &self,
+        pkg_kind: PackageKind,
+        pkg: &PackageNameMaybeVersion,
+        folder_url: &Url,
+      ) -> bool {
+        self
+          .deps
+          .iter()
+          .any(|dep| dep.matches_pkg(pkg_kind, pkg, folder_url))
+      }
+    }
+
+    let mut folders = Vec::with_capacity(self.config_folders.len());
+    for (index, (dir_url, folder)) in self.config_folders.iter().enumerate() {
+      folders.push(Folder {
+        index,
+        folder,
+        dir_url,
+        jsr_nv: folder.deno_json.as_ref().and_then(|deno_json| {
+          deno_json
+            .json
+            .name
+            .as_ref()
+            .map(|name| PackageNameMaybeVersion {
+              name,
+              version: deno_json
+                .json
+                .version
+                .as_ref()
+                .and_then(|v| Version::parse_standard(v).ok()),
+            })
+        }),
+        npm_nv: folder.pkg_json.as_ref().and_then(|pkg_json| {
+          pkg_json.name.as_ref().map(|name| PackageNameMaybeVersion {
+            name,
+            version: pkg_json
+              .version
+              .as_ref()
+              .and_then(|v| Version::parse_from_npm(v).ok()),
+          })
+        }),
+        deps: folder
+          .deno_json
+          .as_ref()
+          .map(|d| d.dependencies().into_iter().map(Dep::Req))
+          .into_iter()
+          .flatten()
+          .chain(
+            folder
+              .pkg_json
+              .as_ref()
+              .map(|d| {
+                let deps = d.resolve_local_package_json_deps();
+                deps
+                  .dependencies
+                  .iter()
+                  .chain(deps.dev_dependencies.iter())
+                  .filter_map(|(k, v)| match v.as_ref().ok()? {
+                    PackageJsonDepValue::File(path) => {
+                      dir_url.join(path).ok().map(Dep::Path)
+                    }
+                    PackageJsonDepValue::Req(package_req) => {
+                      Some(Dep::Req(JsrDepPackageReq {
+                        kind: PackageKind::Npm,
+                        req: package_req.clone(),
+                      }))
+                    }
+                    PackageJsonDepValue::Workspace(workspace_req) => {
+                      Some(Dep::Req(JsrDepPackageReq {
+                        kind: PackageKind::Npm,
+                        req: PackageReq {
+                          name: k.clone(),
+                          version_req: match workspace_req {
+                            PackageJsonDepWorkspaceReq::VersionReq(
+                              version_req,
+                            ) => version_req.clone(),
+                            PackageJsonDepWorkspaceReq::Tilde
+                            | PackageJsonDepWorkspaceReq::Caret => {
+                              VersionReq::parse_from_npm("*").unwrap()
+                            }
+                          },
+                        },
+                      }))
+                    }
+                    PackageJsonDepValue::JsrReq(req) => {
+                      Some(Dep::Req(JsrDepPackageReq {
+                        kind: PackageKind::Npm,
+                        req: req.clone(),
+                      }))
+                    }
+                  })
+              })
+              .into_iter()
+              .flatten(),
+          )
+          .collect(),
+      })
+    }
+
+    // build adjacency + in-degree
+    let n = folders.len();
+    let mut adj: Vec<Vec<usize>> = vec![Vec::new(); n];
+    let mut indeg = vec![0_u32; n];
+
+    for i in 0..n {
+      for j in 0..n {
+        if i != j && folders[i].depends_on(&folders[j]) {
+          adj[j].push(i);
+          indeg[i] += 1;
+        }
+      }
+    }
+
+    // kahn's algorithm
+    let mut queue: VecDeque<usize> = indeg
+      .iter()
+      .enumerate()
+      .filter(|&(_, &d)| d == 0)
+      .map(|(i, _)| i)
+      .collect();
+    // preserve original insertion order for deterministic output
+    queue.make_contiguous().sort_by_key(|&i| folders[i].index);
+
+    let mut output = Vec::<usize>::with_capacity(n);
+    while let Some(i) = queue.pop_front() {
+      output.push(i);
+      for &j in &adj[i] {
+        indeg[j] -= 1;
+        if indeg[j] == 0 {
+          queue.push_back(j);
+        }
+      }
+    }
+
+    // handle possible cycles
+    if output.len() < n {
+      // collect the still-cyclic nodes
+      let mut cyclic: Vec<usize> = (0..n).filter(|&i| indeg[i] > 0).collect();
+
+      // stable, deterministic: lowest original index first
+      cyclic.sort_by_key(|&i| folders[i].index);
+
+      output.extend(cyclic);
+    }
+
+    output
+      .into_iter()
+      .map(|i| (folders[i].dir_url, folders[i].folder))
+      .collect()
+  }
+
   pub fn deno_jsons(&self) -> impl Iterator<Item = &ConfigFileRc> {
     self
       .config_folders
@@ -5654,6 +5877,154 @@ pub mod test {
     }
   }
 
+  #[test]
+  fn test_folder_sorted_dependencies() {
+    #[track_caller]
+    fn assert_order(sys: InMemorySys, expected: Vec<PathBuf>) {
+      let workspace_dir = workspace_at_start_dir(&sys, &root_dir());
+      assert_eq!(
+        workspace_dir
+          .workspace
+          .config_folders_sorted_by_dependencies()
+          .keys()
+          .map(|k| k.to_file_path().unwrap())
+          .collect::<Vec<_>>(),
+        expected,
+      );
+    }
+
+    {
+      let sys = InMemorySys::default();
+      sys.fs_insert_json(
+        root_dir().join("deno.json"),
+        json!({
+          "workspace": ["./a", "./b", "./c"]
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("a/package.json"),
+        json!({
+          "dependencies": {
+            "c": "*"
+          }
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("b/package.json"),
+        json!({
+          "name": "b",
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("c/package.json"),
+        json!({
+          "name": "c",
+          "dependencies": {
+            "b": "workspace:~"
+          }
+        }),
+      );
+      assert_order(
+        sys,
+        vec![
+          root_dir(),
+          root_dir().join("b"),
+          root_dir().join("c"),
+          root_dir().join("a"),
+        ],
+      );
+    }
+
+    // circular
+    {
+      let sys = InMemorySys::default();
+      sys.fs_insert_json(
+        root_dir().join("deno.json"),
+        json!({
+          "workspace": ["./a", "./b", "./c"]
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("a/package.json"),
+        json!({
+          "dependencies": {
+            "b": "*"
+          }
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("b/package.json"),
+        json!({
+          "name": "b",
+          "dependencies": {
+            "c": "*"
+          }
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("c/package.json"),
+        json!({
+          "name": "c",
+          "dependencies": {
+            "a": "*"
+          }
+        }),
+      );
+      assert_order(
+        sys,
+        vec![
+          root_dir(),
+          root_dir().join("c"),
+          root_dir().join("b"),
+          root_dir().join("a"),
+        ],
+      );
+    }
+
+    // file specifier
+    {
+      let sys = InMemorySys::default();
+      sys.fs_insert_json(
+        root_dir().join("deno.json"),
+        json!({
+          "workspace": ["./a", "./b", "./c"]
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("a/package.json"),
+        json!({
+          "dependencies": {
+            "b": "file:../b"
+          }
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("b/package.json"),
+        json!({
+          "name": "b",
+          "dependencies": {
+            "c": "file:../c/"
+          }
+        }),
+      );
+      sys.fs_insert_json(
+        root_dir().join("c/package.json"),
+        json!({
+          "name": "c"
+        }),
+      );
+      assert_order(
+        sys,
+        vec![
+          root_dir(),
+          root_dir().join("c"),
+          root_dir().join("b"),
+          root_dir().join("a"),
+        ],
+      );
+    }
+  }
+
   fn workspace_for_root_and_member(
     root: serde_json::Value,
     member: serde_json::Value,

libs/resolver/lockfile.rs

@@ -16,8 +16,6 @@ use deno_npm::resolution::NpmRegistryDefaultTarballUrlProvider;
 use deno_package_json::PackageJsonDepValue;
 use deno_path_util::fs::atomic_write_file_with_retries;
 use deno_semver::jsr::JsrDepPackageReq;
-use deno_semver::jsr::JsrPackageReqReference;
-use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageNv;
 use futures::TryStreamExt;
 use futures::stream::FuturesOrdered;
@@ -327,12 +325,12 @@ impl<TSys: LockfileSys> LockfileLock<TSys> {
       root: WorkspaceMemberConfig {
         package_json_deps: pkg_json_deps(root_folder.pkg_json.as_deref()),
         dependencies: if let Some(map) = maybe_external_import_map {
-          import_map_deps(map)
+          deno_config::import_map::import_map_deps(map)
         } else {
           root_folder
             .deno_json
             .as_deref()
-            .map(deno_json_deps)
+            .map(|d| d.dependencies())
             .unwrap_or_default()
         },
       },
@@ -358,7 +356,7 @@ impl<TSys: LockfileSys> LockfileLock<TSys> {
                 dependencies: folder
                   .deno_json
                   .as_deref()
-                  .map(deno_json_deps)
+                  .map(|d| d.dependencies())
                   .unwrap_or_default(),
               };
               if config.package_json_deps.is_empty()
@@ -546,97 +544,3 @@ impl<TSys: LockfileSys> deno_graph::source::Locker
       .insert_package(package_nv.clone(), checksum.into_string());
   }
 }
-
-fn import_map_deps(
-  import_map: &serde_json::Value,
-) -> HashSet<JsrDepPackageReq> {
-  let values = imports_values(import_map.get("imports"))
-    .into_iter()
-    .chain(scope_values(import_map.get("scopes")));
-  values_to_set(values)
-}
-
-fn deno_json_deps(
-  config: &deno_config::deno_json::ConfigFile,
-) -> HashSet<JsrDepPackageReq> {
-  let values = imports_values(config.json.imports.as_ref())
-    .into_iter()
-    .chain(scope_values(config.json.scopes.as_ref()));
-  let mut set = values_to_set(values);
-
-  if let Some(serde_json::Value::Object(compiler_options)) =
-    &config.json.compiler_options
-  {
-    // add jsxImportSource
-    if let Some(serde_json::Value::String(value)) =
-      compiler_options.get("jsxImportSource")
-    {
-      if let Some(dep_req) = value_to_dep_req(value) {
-        set.insert(dep_req);
-      }
-    }
-    // add jsxImportSourceTypes
-    if let Some(serde_json::Value::String(value)) =
-      compiler_options.get("jsxImportSourceTypes")
-    {
-      if let Some(dep_req) = value_to_dep_req(value) {
-        set.insert(dep_req);
-      }
-    }
-    // add the dependencies in the types array
-    if let Some(serde_json::Value::Array(types)) = compiler_options.get("types")
-    {
-      for value in types {
-        if let serde_json::Value::String(value) = value {
-          if let Some(dep_req) = value_to_dep_req(value) {
-            set.insert(dep_req);
-          }
-        }
-      }
-    }
-  }
-
-  set
-}
-
-fn imports_values(value: Option<&serde_json::Value>) -> Vec<&String> {
-  let Some(obj) = value.and_then(|v| v.as_object()) else {
-    return Vec::new();
-  };
-  let mut items = Vec::with_capacity(obj.len());
-  for value in obj.values() {
-    if let serde_json::Value::String(value) = value {
-      items.push(value);
-    }
-  }
-  items
-}
-
-fn scope_values(value: Option<&serde_json::Value>) -> Vec<&String> {
-  let Some(obj) = value.and_then(|v| v.as_object()) else {
-    return Vec::new();
-  };
-  obj.values().flat_map(|v| imports_values(Some(v))).collect()
-}
-
-fn values_to_set<'a>(
-  values: impl Iterator<Item = &'a String>,
-) -> HashSet<JsrDepPackageReq> {
-  let mut entries = HashSet::new();
-  for value in values {
-    if let Some(dep_req) = value_to_dep_req(value) {
-      entries.insert(dep_req);
-    }
-  }
-  entries
-}
-
-fn value_to_dep_req(value: &str) -> Option<JsrDepPackageReq> {
-  match JsrPackageReqReference::from_str(value) {
-    Ok(req_ref) => Some(JsrDepPackageReq::jsr(req_ref.into_inner().req)),
-    _ => match NpmPackageReqReference::from_str(value) {
-      Ok(req_ref) => Some(JsrDepPackageReq::npm(req_ref.into_inner().req)),
-      _ => None,
-    },
-  }
-}

tests/specs/task/recursive_order/__test__.jsonc

@@ -0,0 +1,4 @@
+{
+  "args": "task -r build",
+  "output": "task.out"
+}

tests/specs/task/recursive_order/a/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "a",
+  "scripts": {
+    "build": "echo a"
+  },
+  "dependencies": {
+    "b": "*"
+  }
+}

tests/specs/task/recursive_order/b/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "b",
+  "scripts": {
+    "build": "echo b"
+  }
+}

tests/specs/task/recursive_order/package.json

@@ -0,0 +1,6 @@
+{
+  "workspaces": [
+    "./a",
+    "./b"
+  ]
+}

tests/specs/task/recursive_order/task.out

@@ -0,0 +1,4 @@
+Task build echo b
+b
+Task build echo a
+a