fix: support workspace:^ and workspace:~ version constraints (#27096

) This commit adds support for understanding "workpace:^" and "workspace:~" version constraints in npm/pnpm workspaces. This is done by upgrading various crates to their latest versions. Closes #26726 --------- Co-authored-by: David Sherret <[email protected]>
denoland · Nov 29, 2024 · 1d49b3c · 1d49b3c
1 parent f624860
commit 1d49b3c
Show file tree

Hide file tree

Showing 32 changed files with 760 additions and 336 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -46,18 +46,18 @@ license = "MIT"
 repository = "https://github.com/denoland/deno"
 
 [workspace.dependencies]
-deno_ast = { version = "=0.43.3", features = ["transpiling"] }
+deno_ast = { version = "=0.44.0", features = ["transpiling"] }
 deno_core = { version = "0.323.0" }
 
 deno_bench_util = { version = "0.174.0", path = "./bench_util" }
-deno_config = { version = "=0.39.2", features = ["workspace", "sync"] }
-deno_lockfile = "=0.23.1"
+deno_config = { version = "=0.39.3", features = ["workspace", "sync"] }
+deno_lockfile = "=0.23.2"
 deno_media_type = { version = "0.2.0", features = ["module_specifier"] }
-deno_npm = "=0.25.4"
+deno_npm = "=0.25.5"
 deno_path_util = "=0.2.1"
 deno_permissions = { version = "0.40.0", path = "./runtime/permissions" }
 deno_runtime = { version = "0.189.0", path = "./runtime" }
-deno_semver = "=0.5.16"
+deno_semver = "=0.6.0"
 deno_terminal = "0.2.0"
 napi_sym = { version = "0.110.0", path = "./ext/napi/sym" }
 test_util = { package = "test_server", path = "./tests/util/server" }
@@ -115,8 +115,8 @@ console_static_text = "=0.8.1"
 dashmap = "5.5.3"
 data-encoding = "2.3.3"
 data-url = "=0.3.0"
-deno_cache_dir = "=0.13.2"
-deno_package_json = { version = "0.1.2", default-features = false }
+deno_cache_dir = "=0.14.0"
+deno_package_json = { version = "0.2.1", default-features = false }
 dlopen2 = "0.6.1"
 ecb = "=0.1.2"
 elliptic-curve = { version = "0.13.4", features = ["alloc", "arithmetic", "ecdh", "std", "pem", "jwk"] }

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
@@ -72,9 +72,9 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposa
 deno_cache_dir.workspace = true
 deno_config.workspace = true
 deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
-deno_doc = { version = "=0.161.1", features = ["rust", "comrak"] }
-deno_graph = { version = "=0.86.2" }
-deno_lint = { version = "=0.68.0", features = ["docs"] }
+deno_doc = { version = "=0.161.2", features = ["rust", "comrak"] }
+deno_graph = { version = "=0.86.3" }
+deno_lint = { version = "=0.68.1", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_npm.workspace = true
 deno_package_json.workspace = true
@@ -108,7 +108,7 @@ dotenvy = "0.15.7"
 dprint-plugin-json = "=0.19.4"
 dprint-plugin-jupyter = "=0.1.5"
 dprint-plugin-markdown = "=0.17.8"
-dprint-plugin-typescript = "=0.93.2"
+dprint-plugin-typescript = "=0.93.3"
 env_logger = "=0.10.0"
 fancy-regex = "=0.10.0"
 faster-hex.workspace = true

diff --git a/cli/args/deno_json.rs b/cli/args/deno_json.rs
@@ -18,12 +18,10 @@ impl<'a> deno_config::fs::DenoConfigFs for DenoConfigFsAdapter<'a> {
   fn read_to_string_lossy(
     &self,
     path: &std::path::Path,
-  ) -> Result<String, std::io::Error> {
+  ) -> Result<std::borrow::Cow<'static, str>, std::io::Error> {
     self
       .0
       .read_text_file_lossy_sync(path, None)
-      // todo(https://github.com/denoland/deno_config/pull/140): avoid clone
-      .map(|s| s.into_owned())
       .map_err(|err| err.into_io_error())
   }
 

diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs
@@ -109,9 +109,12 @@ impl CliLockfile {
       let Some(pkg_json) = maybe_pkg_json else {
         return Default::default();
       };
-      pkg_json
-        .resolve_local_package_json_deps()
+      let deps = pkg_json.resolve_local_package_json_deps();
+
+      deps
+        .dependencies
         .values()
+        .chain(deps.dev_dependencies.values())
         .filter_map(|dep| dep.as_ref().ok())
         .filter_map(|dep| match dep {
           PackageJsonDepValue::Req(req) => {

diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs
@@ -8,8 +8,10 @@ use deno_core::serde_json;
 use deno_core::url::Url;
 use deno_package_json::PackageJsonDepValue;
 use deno_package_json::PackageJsonDepValueParseError;
+use deno_package_json::PackageJsonDepWorkspaceReq;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
+use deno_semver::VersionReq;
 use thiserror::Error;
 
 #[derive(Debug)]
@@ -95,8 +97,14 @@ impl NpmInstallDepsProvider {
 
       if let Some(pkg_json) = &folder.pkg_json {
         let deps = pkg_json.resolve_local_package_json_deps();
-        let mut pkg_pkgs = Vec::with_capacity(deps.len());
-        for (alias, dep) in deps {
+        let mut pkg_pkgs = Vec::with_capacity(
+          deps.dependencies.len() + deps.dev_dependencies.len(),
+        );
+        for (alias, dep) in deps
+          .dependencies
+          .into_iter()
+          .chain(deps.dev_dependencies.into_iter())
+        {
           let dep = match dep {
             Ok(dep) => dep,
             Err(err) => {
@@ -131,7 +139,16 @@ impl NpmInstallDepsProvider {
                 });
               }
             }
-            PackageJsonDepValue::Workspace(version_req) => {
+            PackageJsonDepValue::Workspace(workspace_version_req) => {
+              let version_req = match workspace_version_req {
+                PackageJsonDepWorkspaceReq::VersionReq(version_req) => {
+                  version_req
+                }
+                PackageJsonDepWorkspaceReq::Tilde
+                | PackageJsonDepWorkspaceReq::Caret => {
+                  VersionReq::parse_from_npm("*").unwrap()
+                }
+              };
               if let Some(pkg) = workspace_npm_pkgs.iter().find(|pkg| {
                 pkg.matches_name_and_version_req(&alias, &version_req)
               }) {

diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs
@@ -23,6 +23,7 @@ use deno_graph::source::Loader;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_permissions::PermissionsContainer;
 use node_resolver::InNpmPackageChecker;
+use std::borrow::Cow;
 use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
@@ -67,8 +68,11 @@ pub const CACHE_PERM: u32 = 0o644;
 pub struct RealDenoCacheEnv;
 
 impl deno_cache_dir::DenoCacheEnv for RealDenoCacheEnv {
-  fn read_file_bytes(&self, path: &Path) -> std::io::Result<Vec<u8>> {
-    std::fs::read(path)
+  fn read_file_bytes(
+    &self,
+    path: &Path,
+  ) -> std::io::Result<Cow<'static, [u8]>> {
+    std::fs::read(path).map(Cow::Owned)
   }
 
   fn atomic_write_file(
@@ -112,12 +116,13 @@ pub struct DenoCacheEnvFsAdapter<'a>(
 );
 
 impl<'a> deno_cache_dir::DenoCacheEnv for DenoCacheEnvFsAdapter<'a> {
-  fn read_file_bytes(&self, path: &Path) -> std::io::Result<Vec<u8>> {
+  fn read_file_bytes(
+    &self,
+    path: &Path,
+  ) -> std::io::Result<Cow<'static, [u8]>> {
     self
       .0
       .read_file_sync(path, None)
-      // todo(https://github.com/denoland/deno_cache_dir/pull/66): avoid clone
-      .map(|bytes| bytes.into_owned())
       .map_err(|err| err.into_io_error())
   }
 

diff --git a/cli/factory.rs b/cli/factory.rs
@@ -504,7 +504,12 @@ impl CliFactory {
         let resolver = cli_options
           .create_workspace_resolver(
             self.file_fetcher()?,
-            if cli_options.use_byonm() {
+            if cli_options.use_byonm()
+              && !matches!(
+                cli_options.sub_command(),
+                DenoSubcommand::Publish(_)
+              )
+            {
               PackageJsonDepResolution::Disabled
             } else {
               // todo(dsherret): this should be false for nodeModulesDir: true

diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs
@@ -1540,7 +1540,7 @@ mod tests {
       .unwrap()
       .unwrap()
       .content;
-    String::from_utf8(bytes).unwrap()
+    String::from_utf8(bytes.into_owned()).unwrap()
   }
 
   #[track_caller]

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
@@ -41,6 +41,7 @@ use deno_path_util::url_to_file_path;
 use deno_runtime::deno_node::PackageJson;
 use indexmap::IndexSet;
 use lsp_types::ClientCapabilities;
+use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::HashMap;
@@ -2092,7 +2093,7 @@ impl<T: Clone> CachedFsItems<T> {
 #[derive(Default)]
 struct InnerData {
   stat_calls: CachedFsItems<deno_config::fs::FsMetadata>,
-  read_to_string_calls: CachedFsItems<String>,
+  read_to_string_calls: CachedFsItems<Cow<'static, str>>,
 }
 
 #[derive(Default)]
@@ -2113,7 +2114,7 @@ impl DenoConfigFs for CachedDenoConfigFs {
   fn read_to_string_lossy(
     &self,
     path: &Path,
-  ) -> Result<String, std::io::Error> {
+  ) -> Result<Cow<'static, str>, std::io::Error> {
     self
       .0
       .lock()

diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
@@ -925,7 +925,7 @@ impl FileSystemDocuments {
       let content = bytes_to_content(
         specifier,
         media_type,
-        cached_file.content,
+        cached_file.content.into_owned(),
         maybe_charset,
       )
       .ok()?;

diff --git a/cli/lsp/jsr.rs b/cli/lsp/jsr.rs
@@ -262,7 +262,7 @@ fn read_cached_url(
   cache
     .get(&cache.cache_item_key(url).ok()?, None)
     .ok()?
-    .map(|f| f.content)
+    .map(|f| f.content.into_owned())
 }
 
 #[derive(Debug)]

diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs
@@ -169,7 +169,7 @@ impl Diagnostic for PublishDiagnostic {
         ..
       }) => DiagnosticLevel::Warning,
       FastCheck(_) => DiagnosticLevel::Error,
-      SpecifierUnfurl(_) => DiagnosticLevel::Warning,
+      SpecifierUnfurl(d) => d.level(),
       InvalidPath { .. } => DiagnosticLevel::Error,
       DuplicatePath { .. } => DiagnosticLevel::Error,
       UnsupportedFileType { .. } => DiagnosticLevel::Warning,
@@ -187,7 +187,7 @@ impl Diagnostic for PublishDiagnostic {
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.code(),
-      SpecifierUnfurl(diagnostic) => Cow::Borrowed(diagnostic.code()),
+      SpecifierUnfurl(diagnostic) => diagnostic.code(),
       InvalidPath { .. } => Cow::Borrowed("invalid-path"),
       DuplicatePath { .. } => Cow::Borrowed("case-insensitive-duplicate-path"),
       UnsupportedFileType { .. } => Cow::Borrowed("unsupported-file-type"),
@@ -207,7 +207,7 @@ impl Diagnostic for PublishDiagnostic {
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.message(),
-      SpecifierUnfurl(diagnostic) => Cow::Borrowed(diagnostic.message()),
+      SpecifierUnfurl(diagnostic) => diagnostic.message(),
       InvalidPath { message, .. } => Cow::Borrowed(message.as_str()),
       DuplicatePath { .. } => {
         Cow::Borrowed("package path is a case insensitive duplicate of another path in the package")
@@ -243,17 +243,7 @@ impl Diagnostic for PublishDiagnostic {
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.location(),
-      SpecifierUnfurl(diagnostic) => match diagnostic {
-        SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport {
-          specifier,
-          text_info,
-          range,
-        } => DiagnosticLocation::ModulePosition {
-          specifier: Cow::Borrowed(specifier),
-          text_info: Cow::Borrowed(text_info),
-          source_pos: DiagnosticSourcePos::SourcePos(range.start),
-        },
-      },
+      SpecifierUnfurl(diagnostic) => diagnostic.location(),
       InvalidPath { path, .. } => {
         DiagnosticLocation::Path { path: path.clone() }
       }
@@ -325,24 +315,8 @@ impl Diagnostic for PublishDiagnostic {
 
     use PublishDiagnostic::*;
     match &self {
-      FastCheck(diagnostic) => diagnostic.snippet(),
-      SpecifierUnfurl(diagnostic) => match diagnostic {
-        SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport {
-          text_info,
-          range,
-          ..
-        } => Some(DiagnosticSnippet {
-          source: Cow::Borrowed(text_info),
-          highlights: vec![DiagnosticSnippetHighlight {
-            style: DiagnosticSnippetHighlightStyle::Warning,
-            range: DiagnosticSourceRange {
-              start: DiagnosticSourcePos::SourcePos(range.start),
-              end: DiagnosticSourcePos::SourcePos(range.end),
-            },
-            description: Some("the unanalyzable dynamic import".into()),
-          }],
-        }),
-      },
+      FastCheck(d) => d.snippet(),
+      SpecifierUnfurl(d) => d.snippet(),
       InvalidPath { .. } => None,
       DuplicatePath { .. } => None,
       UnsupportedFileType { .. } => None,
@@ -380,7 +354,7 @@ impl Diagnostic for PublishDiagnostic {
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.hint(),
-      SpecifierUnfurl(_) => None,
+      SpecifierUnfurl(d) => d.hint(),
       InvalidPath { .. } => Some(
         Cow::Borrowed("rename or remove the file, or add it to 'publish.exclude' in the config file"),
       ),
@@ -436,9 +410,9 @@ impl Diagnostic for PublishDiagnostic {
           None => None,
         }
       }
-      SyntaxError(diagnostic) => diagnostic.snippet_fixed(),
+      SyntaxError(d) => d.snippet_fixed(),
+      SpecifierUnfurl(d) => d.snippet_fixed(),
       FastCheck(_)
-      | SpecifierUnfurl(_)
       | InvalidPath { .. }
       | DuplicatePath { .. }
       | UnsupportedFileType { .. }
@@ -453,16 +427,8 @@ impl Diagnostic for PublishDiagnostic {
   fn info(&self) -> Cow<'_, [Cow<'_, str>]> {
     use PublishDiagnostic::*;
     match &self {
-      FastCheck(diagnostic) => {
-        diagnostic.info()
-      }
-      SpecifierUnfurl(diagnostic) => match diagnostic {
-        SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { .. } => Cow::Borrowed(&[
-          Cow::Borrowed("after publishing this package, imports from the local import map / package.json do not work"),
-          Cow::Borrowed("dynamic imports that can not be analyzed at publish time will not be rewritten automatically"),
-          Cow::Borrowed("make sure the dynamic import is resolvable at runtime without an import map / package.json")
-        ]),
-      },
+      FastCheck(d) => d.info(),
+      SpecifierUnfurl(d) => d.info(),
       InvalidPath { .. } => Cow::Borrowed(&[
         Cow::Borrowed("to portably support all platforms, including windows, the allowed characters in package paths are limited"),
       ]),
@@ -503,10 +469,8 @@ impl Diagnostic for PublishDiagnostic {
   fn docs_url(&self) -> Option<Cow<'_, str>> {
     use PublishDiagnostic::*;
     match &self {
-      FastCheck(diagnostic) => diagnostic.docs_url(),
-      SpecifierUnfurl(diagnostic) => match diagnostic {
-        SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { .. } => None,
-      },
+      FastCheck(d) => d.docs_url(),
+      SpecifierUnfurl(d) => d.docs_url(),
       InvalidPath { .. } => {
         Some(Cow::Borrowed("https://jsr.io/go/invalid-path"))
       }