fix(publish): unfurling should always be done with the package json (#24435)

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

6 files changed, 72 insertions, 61 deletions

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index f747271b8..83f038ec0 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -1060,6 +1060,7 @@ impl CliOptions {
   pub async fn create_workspace_resolver(
     &self,
     file_fetcher: &FileFetcher,
+    pkg_json_dep_resolution: PackageJsonDepResolution,
   ) -> Result<WorkspaceResolver, AnyError> {
     let overrode_no_import_map = self
       .overrides
@@ -1102,12 +1103,7 @@ impl CliOptions {
         .workspace
         .create_resolver(
           CreateResolverOptions {
-            // todo(dsherret): this should be false for nodeModulesDir: true
-            pkg_json_dep_resolution: if self.use_byonm() {
-              PackageJsonDepResolution::Disabled
-            } else {
-              PackageJsonDepResolution::Enabled
-            },
+            pkg_json_dep_resolution,
             specified_import_map: cli_arg_specified_import_map,
           },
           |specifier| {
diff --git a/cli/factory.rs b/cli/factory.rs
index 62ab251f1..5b066c67f 100644
--- a/cli/factory.rs
+++ b/cli/factory.rs
@@ -55,6 +55,7 @@ use std::collections::BTreeSet;
 use std::path::PathBuf;
 
 use deno_config::package_json::PackageJsonDepValue;
+use deno_config::workspace::PackageJsonDepResolution;
 use deno_config::workspace::WorkspaceResolver;
 use deno_config::ConfigFile;
 use deno_core::error::AnyError;
@@ -458,7 +459,15 @@ impl CliFactory {
       .get_or_try_init_async(async {
         let resolver = self
           .options
-          .create_workspace_resolver(self.file_fetcher()?)
+          .create_workspace_resolver(
+            self.file_fetcher()?,
+            if self.options.use_byonm() {
+              PackageJsonDepResolution::Disabled
+            } else {
+              // todo(dsherret): this should be false for nodeModulesDir: true
+              PackageJsonDepResolution::Enabled
+            },
+          )
           .await?;
         if !resolver.diagnostics().is_empty() {
           warn!(
@@ -759,6 +768,7 @@ impl CliFactory {
       self.file_fetcher()?,
       self.http_client_provider(),
       self.npm_resolver().await?.as_ref(),
+      self.workspace_resolver().await?.as_ref(),
       self.options.npm_system_info(),
     ))
   }
diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs
index bf035577c..c9371d853 100644
--- a/cli/standalone/binary.rs
+++ b/cli/standalone/binary.rs
@@ -18,6 +18,7 @@ use std::process::Command;
 use deno_ast::ModuleSpecifier;
 use deno_config::workspace::PackageJsonDepResolution;
 use deno_config::workspace::Workspace;
+use deno_config::workspace::WorkspaceResolver;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
@@ -376,6 +377,7 @@ pub struct DenoCompileBinaryWriter<'a> {
   file_fetcher: &'a FileFetcher,
   http_client_provider: &'a HttpClientProvider,
   npm_resolver: &'a dyn CliNpmResolver,
+  workspace_resolver: &'a WorkspaceResolver,
   npm_system_info: NpmSystemInfo,
 }
 
@@ -386,6 +388,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
     file_fetcher: &'a FileFetcher,
     http_client_provider: &'a HttpClientProvider,
     npm_resolver: &'a dyn CliNpmResolver,
+    workspace_resolver: &'a WorkspaceResolver,
     npm_system_info: NpmSystemInfo,
   ) -> Self {
     Self {
@@ -393,6 +396,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       file_fetcher,
       http_client_provider,
       npm_resolver,
+      workspace_resolver,
       npm_system_info,
     }
   }
@@ -419,17 +423,15 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       }
       set_windows_binary_to_gui(&mut original_binary)?;
     }
-    self
-      .write_standalone_binary(
-        writer,
-        original_binary,
-        eszip,
-        root_dir_url,
-        entrypoint,
-        cli_options,
-        compile_flags,
-      )
-      .await
+    self.write_standalone_binary(
+      writer,
+      original_binary,
+      eszip,
+      root_dir_url,
+      entrypoint,
+      cli_options,
+      compile_flags,
+    )
   }
 
   async fn get_base_binary(
@@ -512,7 +514,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
   /// This functions creates a standalone deno binary by appending a bundle
   /// and magic trailer to the currently executing binary.
   #[allow(clippy::too_many_arguments)]
-  async fn write_standalone_binary(
+  fn write_standalone_binary(
     &self,
     writer: &mut impl Write,
     original_bin: Vec<u8>,
@@ -530,9 +532,6 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       Some(CaData::Bytes(bytes)) => Some(bytes.clone()),
       None => None,
     };
-    let workspace_resolver = cli_options
-      .create_workspace_resolver(self.file_fetcher)
-      .await?;
     let root_path = root_dir_url.inner().to_file_path().unwrap();
     let (npm_vfs, npm_files, node_modules) = match self.npm_resolver.as_inner()
     {
@@ -599,7 +598,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       ca_data,
       entrypoint_key: root_dir_url.specifier_key(entrypoint).into_owned(),
       workspace_resolver: SerializedWorkspaceResolver {
-        import_map: workspace_resolver.maybe_import_map().map(|i| {
+        import_map: self.workspace_resolver.maybe_import_map().map(|i| {
           SerializedWorkspaceResolverImportMap {
             specifier: if i.base_url().scheme() == "file" {
               root_dir_url.specifier_key(i.base_url()).into_owned()
@@ -610,7 +609,8 @@ impl<'a> DenoCompileBinaryWriter<'a> {
             json: i.to_json(),
           }
         }),
-        package_jsons: workspace_resolver
+        package_jsons: self
+          .workspace_resolver
           .package_jsons()
           .map(|pkg_json| {
             (
@@ -621,7 +621,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
             )
           })
           .collect(),
-        pkg_json_resolution: workspace_resolver.pkg_json_dep_resolution(),
+        pkg_json_resolution: self.workspace_resolver.pkg_json_dep_resolution(),
       },
       node_modules,
       disable_deprecated_api_warning: cli_options
diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs
index 134a973f7..3f59f4e14 100644
--- a/cli/tools/registry/mod.rs
+++ b/cli/tools/registry/mod.rs
@@ -12,7 +12,7 @@ use base64::prelude::BASE64_STANDARD;
 use base64::Engine;
 use deno_ast::ModuleSpecifier;
 use deno_config::workspace::JsrPackageConfig;
-use deno_config::workspace::WorkspaceResolver;
+use deno_config::workspace::PackageJsonDepResolution;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
@@ -24,7 +24,6 @@ use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::serde_json::Value;
 use deno_runtime::deno_fetch::reqwest;
-use deno_runtime::deno_fs::FileSystem;
 use deno_terminal::colors;
 use lsp_types::Url;
 use serde::Deserialize;
@@ -81,8 +80,6 @@ pub async fn publish(
   let auth_method =
     get_auth_method(publish_flags.token, publish_flags.dry_run)?;
 
-  let workspace_resolver = cli_factory.workspace_resolver().await?.clone();
-
   let directory_path = cli_factory.cli_options().initial_cwd();
   let cli_options = cli_factory.cli_options();
   let publish_configs = cli_options.workspace.jsr_packages_for_publish();
@@ -103,6 +100,20 @@ pub async fn publish(
       }
     }
   }
+  let specifier_unfurler = Arc::new(SpecifierUnfurler::new(
+    if cli_options.unstable_sloppy_imports() {
+      Some(SloppyImportsResolver::new(cli_factory.fs().clone()))
+    } else {
+      None
+    },
+    cli_options
+      .create_workspace_resolver(
+        cli_factory.file_fetcher()?,
+        PackageJsonDepResolution::Enabled,
+      )
+      .await?,
+    cli_options.unstable_bare_node_builtins(),
+  ));
 
   let diagnostics_collector = PublishDiagnosticsCollector::default();
   let publish_preparer = PublishPreparer::new(
@@ -110,9 +121,8 @@ pub async fn publish(
     cli_factory.module_graph_creator().await?.clone(),
     cli_factory.parsed_source_cache().clone(),
     cli_factory.type_checker().await?.clone(),
-    cli_factory.fs().clone(),
     cli_factory.cli_options().clone(),
-    workspace_resolver,
+    specifier_unfurler,
   );
 
   let prepared_data = publish_preparer
@@ -191,8 +201,7 @@ struct PublishPreparer {
   source_cache: Arc<ParsedSourceCache>,
   type_checker: Arc<TypeChecker>,
   cli_options: Arc<CliOptions>,
-  sloppy_imports_resolver: Option<Arc<SloppyImportsResolver>>,
-  workspace_resolver: Arc<WorkspaceResolver>,
+  specifier_unfurler: Arc<SpecifierUnfurler>,
 }
 
 impl PublishPreparer {
@@ -201,23 +210,16 @@ impl PublishPreparer {
     module_graph_creator: Arc<ModuleGraphCreator>,
     source_cache: Arc<ParsedSourceCache>,
     type_checker: Arc<TypeChecker>,
-    fs: Arc<dyn FileSystem>,
     cli_options: Arc<CliOptions>,
-    workspace_resolver: Arc<WorkspaceResolver>,
+    specifier_unfurler: Arc<SpecifierUnfurler>,
   ) -> Self {
-    let sloppy_imports_resolver = if cli_options.unstable_sloppy_imports() {
-      Some(Arc::new(SloppyImportsResolver::new(fs.clone())))
-    } else {
-      None
-    };
     Self {
       graph_diagnostics_collector,
       module_graph_creator,
       source_cache,
       type_checker,
       cli_options,
-      sloppy_imports_resolver,
-      workspace_resolver,
+      specifier_unfurler,
     }
   }
 
@@ -432,18 +434,11 @@ impl PublishPreparer {
 
     let tarball = deno_core::unsync::spawn_blocking({
       let diagnostics_collector = diagnostics_collector.clone();
-      let workspace_resolver = self.workspace_resolver.clone();
-      let sloppy_imports_resolver = self.sloppy_imports_resolver.clone();
+      let unfurler = self.specifier_unfurler.clone();
       let cli_options = self.cli_options.clone();
       let source_cache = self.source_cache.clone();
       let config_path = config_path.clone();
       move || {
-        let bare_node_builtins = cli_options.unstable_bare_node_builtins();
-        let unfurler = SpecifierUnfurler::new(
-          sloppy_imports_resolver.as_deref(),
-          &workspace_resolver,
-          bare_node_builtins,
-        );
         let root_specifier =
           ModuleSpecifier::from_directory_path(&root_dir).unwrap();
         let publish_paths =
diff --git a/cli/tools/registry/unfurl.rs b/cli/tools/registry/unfurl.rs
index 147b59f30..758db0796 100644
--- a/cli/tools/registry/unfurl.rs
+++ b/cli/tools/registry/unfurl.rs
@@ -5,6 +5,7 @@ use deno_ast::SourceRange;
 use deno_ast::SourceTextInfo;
 use deno_config::package_json::PackageJsonDepValue;
 use deno_config::workspace::MappedResolution;
+use deno_config::workspace::PackageJsonDepResolution;
 use deno_config::workspace::WorkspaceResolver;
 use deno_core::ModuleSpecifier;
 use deno_graph::DependencyDescriptor;
@@ -40,18 +41,22 @@ impl SpecifierUnfurlerDiagnostic {
   }
 }
 
-pub struct SpecifierUnfurler<'a> {
-  sloppy_imports_resolver: Option<&'a SloppyImportsResolver>,
-  workspace_resolver: &'a WorkspaceResolver,
+pub struct SpecifierUnfurler {
+  sloppy_imports_resolver: Option<SloppyImportsResolver>,
+  workspace_resolver: WorkspaceResolver,
   bare_node_builtins: bool,
 }
 
-impl<'a> SpecifierUnfurler<'a> {
+impl SpecifierUnfurler {
   pub fn new(
-    sloppy_imports_resolver: Option<&'a SloppyImportsResolver>,
-    workspace_resolver: &'a WorkspaceResolver,
+    sloppy_imports_resolver: Option<SloppyImportsResolver>,
+    workspace_resolver: WorkspaceResolver,
     bare_node_builtins: bool,
   ) -> Self {
+    debug_assert_eq!(
+      workspace_resolver.pkg_json_dep_resolution(),
+      PackageJsonDepResolution::Enabled
+    );
     Self {
       sloppy_imports_resolver,
       workspace_resolver,
@@ -136,7 +141,7 @@ impl<'a> SpecifierUnfurler<'a> {
     //   resolved
     // };
     let resolved =
-      if let Some(sloppy_imports_resolver) = self.sloppy_imports_resolver {
+      if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver {
         sloppy_imports_resolver
           .resolve(&resolved, deno_graph::source::ResolutionMode::Execution)
           .as_specifier()
@@ -148,6 +153,12 @@ impl<'a> SpecifierUnfurler<'a> {
     if relative_resolved == specifier {
       None // nothing to unfurl
     } else {
+      log::debug!(
+        "Unfurled specifier: {} from {} -> {}",
+        specifier,
+        referrer,
+        relative_resolved
+      );
       Some(relative_resolved)
     }
   }
@@ -395,11 +406,9 @@ mod tests {
       deno_config::workspace::PackageJsonDepResolution::Enabled,
     );
     let fs = Arc::new(RealFs);
-    let sloppy_imports_resolver = SloppyImportsResolver::new(fs);
-
     let unfurler = SpecifierUnfurler::new(
-      Some(&sloppy_imports_resolver),
-      &workspace_resolver,
+      Some(SloppyImportsResolver::new(fs)),
+      workspace_resolver,
       true,
     );
 
diff --git a/cli/util/logger.rs b/cli/util/logger.rs
index 3cd0cbe5d..f3510c502 100644
--- a/cli/util/logger.rs
+++ b/cli/util/logger.rs
@@ -41,6 +41,7 @@ pub fn init(maybe_level: Option<log::Level>) {
   // wgpu crates (gfx_backend), have a lot of useless INFO and WARN logs
   .filter_module("wgpu", log::LevelFilter::Error)
   .filter_module("gfx", log::LevelFilter::Error)
+  .filter_module("globset", log::LevelFilter::Error)
   // used to make available the lsp_debug which is then filtered out at runtime
   // in the cli logger
   .filter_module("deno::lsp::performance", log::LevelFilter::Debug)