Repository: denoland/deno
Pull request: fix(publish): unfurl sloppy imports in d.ts files and type imports

Fixes a bug discovered while trying to publish a package with .js + .d.ts with sloppy imports.


Changed files:
filename: cli/tools/registry/unfurl.rs
patch: @@ -23,6 +23,7 @@ use deno_core::ModuleSpecifier;
 use deno_graph::DependencyDescriptor;
 use deno_graph::DynamicTemplatePart;
 use deno_graph::ParserModuleAnalyzer;
+use deno_graph::StaticDependencyKind;
 use deno_graph::TypeScriptReference;
 use deno_package_json::PackageJsonDepValue;
 use deno_package_json::PackageJsonDepWorkspaceReq;
@@ -214,11 +215,12 @@ impl SpecifierUnfurler {
     &self,
     referrer: &ModuleSpecifier,
     specifier: &str,
+    resolution_kind: SloppyImportsResolutionKind,
     text_info: &SourceTextInfo,
     range: &deno_graph::PositionRange,
     diagnostic_reporter: &mut dyn FnMut(SpecifierUnfurlerDiagnostic),
   ) -> Option<String> {
-    match self.unfurl_specifier(referrer, specifier) {
+    match self.unfurl_specifier(referrer, specifier, resolution_kind) {
       Ok(maybe_unfurled) => maybe_unfurled,
       Err(diagnostic) => match diagnostic {
         UnfurlSpecifierError::Workspace {
@@ -248,6 +250,7 @@ impl SpecifierUnfurler {
     &self,
     referrer: &ModuleSpecifier,
     specifier: &str,
+    resolution_kind: SloppyImportsResolutionKind,
   ) -> Result<Option<String>, UnfurlSpecifierError> {
     let resolved = if let Ok(resolved) =
       self.workspace_resolver.resolve(specifier, referrer)
@@ -395,7 +398,7 @@ impl SpecifierUnfurler {
     let resolved =
       if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver {
         sloppy_imports_resolver
-          .resolve(&resolved, SloppyImportsResolutionKind::Execution)
+          .resolve(&resolved, resolution_kind)
           .map(|res| res.into_specifier())
           .unwrap_or(resolved)
       } else {
@@ -458,6 +461,7 @@ impl SpecifierUnfurler {
         let maybe_unfurled = self.unfurl_specifier_reporting_diagnostic(
           module_url,
           specifier,
+          SloppyImportsResolutionKind::Execution, // dynamic imports are always execution
           text_info,
           &dep.argument_range,
           diagnostic_reporter,
@@ -485,6 +489,7 @@ impl SpecifierUnfurler {
           let unfurled = self.unfurl_specifier_reporting_diagnostic(
             module_url,
             specifier,
+            SloppyImportsResolutionKind::Execution, // dynamic imports are always execution
             text_info,
             &dep.argument_range,
             diagnostic_reporter,
@@ -530,11 +535,13 @@ impl SpecifierUnfurler {
     let analyze_specifier =
       |specifier: &str,
        range: &deno_graph::PositionRange,
+       resolution_kind: SloppyImportsResolutionKind,
        text_changes: &mut Vec<deno_ast::TextChange>,
        diagnostic_reporter: &mut dyn FnMut(SpecifierUnfurlerDiagnostic)| {
         if let Some(unfurled) = self.unfurl_specifier_reporting_diagnostic(
           url,
           specifier,
+          resolution_kind,
           text_info,
           range,
           diagnostic_reporter,
@@ -548,9 +555,27 @@ impl SpecifierUnfurler {
     for dep in &module_info.dependencies {
       match dep {
         DependencyDescriptor::Static(dep) => {
+          let resolution_kind = if parsed_source.media_type().is_declaration() {
+            SloppyImportsResolutionKind::Types
+          } else {
+            match dep.kind {
+              StaticDependencyKind::Export
+              | StaticDependencyKind::Import
+              | StaticDependencyKind::ExportEquals
+              | StaticDependencyKind::ImportEquals => {
+                SloppyImportsResolutionKind::Execution
+              }
+              StaticDependencyKind::ExportType
+              | StaticDependencyKind::ImportType => {
+                SloppyImportsResolutionKind::Types
+              }
+            }
+          };
+
           analyze_specifier(
             &dep.specifier,
             &dep.specifier_range,
+            resolution_kind,
             &mut text_changes,
             diagnostic_reporter,
           );
@@ -588,6 +613,7 @@ impl SpecifierUnfurler {
       analyze_specifier(
         &specifier_with_range.text,
         &specifier_with_range.range,
+        SloppyImportsResolutionKind::Types,
         &mut text_changes,
         diagnostic_reporter,
       );
@@ -596,6 +622,7 @@ impl SpecifierUnfurler {
       analyze_specifier(
         &jsdoc.specifier.text,
         &jsdoc.specifier.range,
+        SloppyImportsResolutionKind::Types,
         &mut text_changes,
         diagnostic_reporter,
       );
@@ -604,6 +631,16 @@ impl SpecifierUnfurler {
       analyze_specifier(
         &specifier_with_range.text,
         &specifier_with_range.range,
+        SloppyImportsResolutionKind::Execution,
+        &mut text_changes,
+        diagnostic_reporter,
+      );
+    }
+    if let Some(specifier_with_range) = &module_info.jsx_import_source_types {
+      analyze_specifier(
+        &specifier_with_range.text,
+        &specifier_with_range.range,
+        SloppyImportsResolutionKind::Types,
         &mut text_changes,
         diagnostic_reporter,
       );
@@ -741,6 +778,8 @@ import baz from "./baz";
 import b from "./b.js";
 import b2 from "./b";
 import "./mod.ts";
+import { } from "./c";
+import type { } from "./c";
 import url from "url";
 import "@denotest/example";
 // TODO: unfurl these to jsr
@@ -791,6 +830,8 @@ import baz from "./baz/index.js";
 import b from "./b.ts";
 import b2 from "./b.ts";
 import "./mod.ts";
+import { } from "./c.js";
+import type { } from "./c.d.ts";
 import url from "node:url";
 import "jsr:@denotest/example@^1.0.0";
 // TODO: unfurl these to jsr
@@ -808,6 +849,24 @@ const test6 = await import(`./lib/something.ts`);
 // will warn
 const warn1 = await import(`lib${expr}`);
 const warn2 = await import(`${expr}`);
+"#;
+      assert_eq!(unfurled_source, expected_source);
+    }
+
+    // Unfurling .d.ts file should use types resolution.
+    {
+      let source_code = r#"import express from "express";"
+export type * from "./c";
+"#;
+      let specifier =
+        ModuleSpecifier::from_file_path(cwd.join("mod.d.ts")).unwrap();
+      let source = parse_ast(&specifier, source_code);
+      let mut d = Vec::new();
+      let mut reporter = |diagnostic| d.push(diagnostic);
+      let unfurled_source = unfurler.unfurl(&specifier, &source, &mut reporter);
+      assert_eq!(d.len(), 0);
+      let expected_source = r#"import express from "npm:express@5";"
+export type * from "./c.d.ts";
 "#;
       assert_eq!(unfurled_source, expected_source);
     }