feat: add the types package of a mapped dependency to the dev dependencies (#509)

Author: dsherret

lib/package_json.test.ts

@@ -32,6 +32,7 @@ Deno.test("single entrypoint", () => {
         }],
       },
       warnings: [],
+      typesDependencies: [],
     },
     entryPoints: [{
       name: ".",
@@ -252,6 +253,7 @@ Deno.test("exports have default last", () => {
         dependencies: [],
       },
       warnings: [],
+      typesDependencies: [],
     },
     entryPoints: [
       {
@@ -299,6 +301,7 @@ Deno.test("multiple entrypoints", () => {
         dependencies: [],
       },
       warnings: [],
+      typesDependencies: [],
     },
     entryPoints: [{
       name: ".",
@@ -375,6 +378,7 @@ Deno.test("binary entrypoints", () => {
         dependencies: [],
       },
       warnings: [],
+      typesDependencies: [],
     },
     entryPoints: [{
       name: ".",
@@ -457,6 +461,7 @@ Deno.test("peer dependencies", () => {
         }],
       },
       warnings: [],
+      typesDependencies: [],
     },
     entryPoints: [{
       name: ".",
@@ -520,3 +525,58 @@ Deno.test("peer dependencies", () => {
     ),
   );
 });
+
+Deno.test("types dependencies", () => {
+  const props: GetPackageJsonOptions = {
+    transformOutput: {
+      main: {
+        files: [],
+        dependencies: [{
+          name: "svg-path-parser",
+          version: "1.1.0",
+        }, {
+          name: "other",
+          version: "^1.0.0",
+        }],
+        entryPoints: ["mod.ts"],
+      },
+      test: {
+        entryPoints: [],
+        files: [],
+        dependencies: [],
+      },
+      warnings: [],
+      typesDependencies: [{
+        name: "@types/svg-path-parser",
+        version: "~1.1.6",
+      }, {
+        // a types package that's already a dependency is not duplicated
+        name: "other",
+        version: "^1.0.0",
+      }],
+    },
+    entryPoints: [{
+      name: ".",
+      path: "./mod.ts",
+    }],
+    package: {
+      name: "package",
+      version: "0.1.0",
+    },
+    testEnabled: false,
+    includeEsModule: true,
+    includeScriptModule: true,
+    includeDeclarations: true,
+    includeTsLib: false,
+    shims: {},
+  };
+
+  const packageJson = getPackageJson(props);
+  assertEquals(packageJson.dependencies, {
+    "svg-path-parser": "1.1.0",
+    "other": "^1.0.0",
+  });
+  assertEquals(packageJson.devDependencies, {
+    "@types/svg-path-parser": "~1.1.6",
+  });
+});

lib/package_json.ts

@@ -79,6 +79,16 @@ export function getPackageJson({
     })
     : {};
   const devDependencies = {
+    // packages that provide the type declarations of a dependency
+    // (ex. an `@types/` package specified by an `X-TypeScript-Types` header)
+    ...Object.fromEntries(
+      transformOutput.typesDependencies
+        .filter((d) =>
+          !Object.keys(dependencies).includes(d.name) &&
+          !Object.keys(peerDependencies).includes(d.name)
+        )
+        .map((d) => [d.name, d.version]),
+    ),
     ...(shouldIncludeTypesNode()
       ? {
         "@types/node": "^20.9.0",

rs-lib/src/declaration_file_resolution.rs

@@ -10,6 +10,8 @@ use deno_graph::Module;
 use deno_graph::Resolution;
 
 use crate::graph::ModuleGraph;
+use crate::loader::get_all_specifier_mappers;
+use crate::loader::get_types_package_for_specifier;
 use crate::PackageMappedSpecifier;
 
 #[derive(Debug)]
@@ -27,29 +29,50 @@ pub struct TypesDependency {
   pub referrer: ModuleSpecifier,
 }
 
+pub struct DeclarationFileResolutions {
+  pub mappings: BTreeMap<ModuleSpecifier, DeclarationFileResolution>,
+  /// Packages that provide the declaration files of a mapped package
+  /// (ex. an `@types/` package served by a cdn).
+  pub types_packages: BTreeMap<String, PackageMappedSpecifier>,
+  /// Declaration files that a types package provides, which don't
+  /// need to be included in the output.
+  pub types_package_files: HashSet<ModuleSpecifier>,
+}
+
 pub fn resolve_declaration_file_mappings(
   module_graph: &ModuleGraph,
   modules: &[&Module],
   mapped_specifiers: &BTreeMap<ModuleSpecifier, PackageMappedSpecifier>,
-) -> Result<BTreeMap<ModuleSpecifier, DeclarationFileResolution>> {
+) -> Result<DeclarationFileResolutions> {
   let mut type_dependencies = BTreeMap::new();
 
   for module in modules.iter().filter_map(|m| m.js()) {
     fill_types_for_module(module_graph, module, &mut type_dependencies)?;
   }
 
   // get the resolved type dependencies
+  let mappers = get_all_specifier_mappers();
   let mut mappings = BTreeMap::new();
+  let mut types_packages = BTreeMap::new();
+  let mut types_package_files = HashSet::new();
   for (code_specifier, deps) in type_dependencies.into_iter() {
-    // if this type_dependency is mapped, then pass it.
-    if mapped_specifiers.contains_key(&code_specifier) {
-      continue;
-    }
-
     let deps = deps.into_iter().collect::<Vec<_>>();
     let selected_dep =
       select_best_types_dep(module_graph, &code_specifier, &deps);
 
+    // when the code is mapped to a package, the declaration file it
+    // specifies may be provided by a types package that needs to go in
+    // the package.json instead of being included in the output
+    if mapped_specifiers.contains_key(&code_specifier) {
+      if let Some(types_package) =
+        get_types_package_for_specifier(&mappers, &selected_dep.specifier)
+      {
+        types_packages.insert(types_package.name.clone(), types_package);
+        types_package_files.insert(selected_dep.specifier);
+      }
+      continue;
+    }
+
     // get the declaration file specifiers that weren't used
     let mut ignored = deps
       .into_iter()
@@ -66,7 +89,11 @@ pub fn resolve_declaration_file_mappings(
     );
   }
 
-  Ok(mappings)
+  Ok(DeclarationFileResolutions {
+    mappings,
+    types_packages,
+    types_package_files,
+  })
 }
 
 /// This resolution process works as follows:

rs-lib/src/lib.rs

@@ -15,6 +15,8 @@ use analyze::get_top_level_decls;
 use anyhow::Context;
 use anyhow::Result;
 
+use crate::loader::get_all_specifier_mappers;
+use crate::loader::get_types_package_for_specifier;
 use analyze::get_ignore_line_indexes;
 use anyhow::bail;
 use deno_ast::apply_text_changes;
@@ -107,6 +109,9 @@ pub struct TransformOutput {
   /// This is `None` when no config file was found, when one was explicitly
   /// provided, or when auto-discovery is disabled.
   pub discovered_config_file: Option<PathBuf>,
+  /// Packages that provide the type declarations of a mapped dependency
+  /// (ex. an `@types/` package specified by an `X-TypeScript-Types` header).
+  pub types_dependencies: Vec<Dependency>,
 }
 
 #[cfg_attr(feature = "serialization", derive(serde::Serialize))]
@@ -462,8 +467,9 @@ pub async fn transform(
     .await?
     .cloned();
 
+  let file_fetcher = Rc::new(file_fetcher);
   let loader = Rc::new(DenoGraphLoader::new(
-    Rc::new(file_fetcher),
+    file_fetcher.clone(),
     resolver_factory
       .workspace_factory()
       .global_http_cache()?
@@ -520,6 +526,8 @@ pub async fn transform(
       .collect();
 
   let mut warnings = get_declaration_warnings(&specifiers);
+  let types_dependencies =
+    get_types_dependencies(&specifiers, &file_fetcher, &mut warnings).await;
   let mut main_env_context = EnvironmentContext {
     environment: TransformOutputEnvironment {
       entry_points: options
@@ -705,6 +713,7 @@ pub async fn transform(
     test: test_env_context.environment,
     warnings,
     discovered_config_file,
+    types_dependencies,
   })
 }
 
@@ -974,6 +983,95 @@ fn check_add_shim_file_to_environment(
   }
 }
 
+/// Gets the packages that provide the type declarations of the mapped
+/// dependencies, which are added to the dev dependencies.
+///
+/// A mapped module isn't part of the module graph, so the `X-TypeScript-Types`
+/// header of each is resolved here.
+async fn get_types_dependencies<TSys: WorkspaceFactorySys>(
+  specifiers: &Specifiers,
+  file_fetcher: &PermissionedFileFetcher<
+    NullBlobStore,
+    TSys,
+    impl deno_cache_dir::file_fetcher::HttpClient,
+  >,
+  warnings: &mut Vec<String>,
+) -> Vec<Dependency> {
+  let mappers = get_all_specifier_mappers();
+  let mut types_packages = specifiers.types_packages.clone();
+  let header_packages = futures::future::join_all(
+    specifiers
+      .main
+      .mapped
+      .keys()
+      .chain(specifiers.test.mapped.keys())
+      // only a module mapped by one of the cdn mappers may have a types
+      // header, so don't fetch the modules the user mapped themselves
+      .filter(|s| mappers.iter().any(|m| m.map(s).is_some()))
+      .map(|specifier| {
+        let mappers = &mappers;
+        async move {
+          match get_types_header_specifier(file_fetcher, specifier).await {
+            Ok(types_specifier) => Ok(
+              types_specifier
+                .and_then(|s| get_types_package_for_specifier(mappers, &s)),
+            ),
+            Err(err) => Err(format!(
+              "Failed getting the type declarations of {}. {:#}",
+              specifier, err
+            )),
+          }
+        }
+      }),
+  )
+  .await;
+  for result in header_packages {
+    match result {
+      Ok(Some(package)) => {
+        // a declaration file specified in the code takes precedence
+        types_packages
+          .entry(package.name.clone())
+          .or_insert(package);
+      }
+      Ok(None) => {}
+      Err(warning) => warnings.push(warning),
+    }
+  }
+
+  types_packages
+    .into_values()
+    .filter_map(|p| {
+      Some(Dependency {
+        name: p.name,
+        version: p.version?,
+        peer_dependency: false,
+      })
+    })
+    .collect()
+}
+
+/// Gets the specifier of the declaration file in a module's
+/// `X-TypeScript-Types` header.
+async fn get_types_header_specifier<TSys: WorkspaceFactorySys>(
+  file_fetcher: &PermissionedFileFetcher<
+    NullBlobStore,
+    TSys,
+    impl deno_cache_dir::file_fetcher::HttpClient,
+  >,
+  specifier: &ModuleSpecifier,
+) -> Result<Option<ModuleSpecifier>> {
+  let mut file = file_fetcher.fetch_bypass_permissions(specifier).await?;
+  let Some(types_header) = file
+    .maybe_headers
+    .as_mut()
+    .and_then(|h| h.remove("x-typescript-types"))
+  else {
+    return Ok(None);
+  };
+  // resolve relative to the final url so that redirects are handled
+  Ok(file.url.join(&types_header).ok())
+}
+
 fn get_dependencies(
   mappings: BTreeMap<ModuleSpecifier, PackageMappedSpecifier>,
 ) -> Vec<Dependency> {