fix: emit .d.ts and .js files into _dist folder and other fixes (#463)

Fixes #462 

To make this work, also bans publishing packages containing a top level
folder called `_dist`.

Also we now do not reference `.d.ts` files from other `.d.ts` files
using the `.d.ts` extension, because otherwise tsc gets upset. So we
instead use `.js` (even if the file does not exist!)

Also we emit source maps into separate files, because TSC doesn't
understand embedded source maps.

Author: lucacasonato

Cargo.lock

@@ -79,7 +79,7 @@ flate2 = "1"
 thiserror = "1"
 async-tar = "0.4.2"
 deno_graph = "0.74.1"
-deno_ast = "0.38.1"
+deno_ast = { version = "0.38.1", features = ["view"] }
 deno_doc = { version = "0.128.0", features = ["tree-sitter"] }
 comrak = { version = "0.20.0", default-features = false }
 async-trait = "0.1.73"

api/Cargo.toml

@@ -459,6 +459,9 @@ pub enum VersionValidateError {
 /// The path must only contain ascii alphanumeric characters, and the characters
 /// '$', '(', ')', '+', '-', '.', '@', '[', ']', '_', '{', '}',  '~'.
 ///
+/// The path must not start with `/_dist/`, as this is the directory JSR will
+/// emit `.d.ts` and `.js` files to when building the npm tarball.
+///
 /// Path's are case sensitive, and comparisons and hashing are also case
 /// sensitive. However, to ensure no collisions based only on case-sensitivity,
 /// one may use the `CaseInsensitivePackagePath` type to compare paths in a
@@ -487,6 +490,7 @@ impl PackagePath {
     };
 
     let mut last = None;
+    let mut first = true;
 
     while let Some(component) = components.next() {
       last = Some(component);
@@ -524,6 +528,11 @@ impl PackagePath {
           component.to_owned(),
         ));
       }
+
+      if first && component.eq_ignore_ascii_case("_dist") {
+        return Err(PackagePathValidationError::ReservedUnderscoreDist);
+      }
+      first = false;
     }
 
     // Due to restrictions in how tarballs are built, we need the ensure that
@@ -703,6 +712,11 @@ pub enum PackagePathValidationError {
   )]
   ReservedName(String),
 
+  #[error(
+    "package path must not start with /_dist/, as this is the directory JSR will emit .d.ts and .js files to when building the npm tarball"
+  )]
+  ReservedUnderscoreDist,
+
   #[error("path segment must not end in a dot (found '{0}')")]
   TrailingDot(String),
 
@@ -960,6 +974,7 @@ mod tests {
     assert!(PackagePath::try_from("/~/foo").is_ok());
     assert!(PackagePath::try_from("/~foo/bar").is_ok());
     assert!(PackagePath::try_from("/(section)/32").is_ok());
+    assert!(PackagePath::try_from("/foo/_dist/23").is_ok());
 
     // Test invalid package paths
     assert!(PackagePath::try_from("").is_err());
@@ -996,6 +1011,8 @@ mod tests {
     assert!(PackagePath::try_from("/CON.txt").is_err());
     assert!(PackagePath::try_from("/foo.").is_err());
     assert!(PackagePath::try_from("/f".repeat(81)).is_err());
+    assert!(PackagePath::try_from("/_dist").is_err());
+    assert!(PackagePath::try_from("/_dist/foo.txt").is_err());
   }
 
   #[test]

api/src/ids.rs

@@ -9,24 +9,29 @@ use deno_ast::SourceMap;
 use deno_ast::SourceMapOption;
 use deno_ast::TranspileOptions;
 use deno_graph::FastCheckTypeModule;
+use url::Url;
 
 use crate::npm::import_transform::ImportRewriteTransformer;
+use crate::npm::specifiers::relative_import_specifier;
 
 use super::specifiers::RewriteKind;
 use super::specifiers::SpecifierRewriter;
 
 pub fn transpile_to_js(
   source: &ParsedSource,
   specifier_rewriter: SpecifierRewriter,
-) -> Result<String, anyhow::Error> {
+  target_specifier: &Url,
+) -> Result<(String, String), anyhow::Error> {
+  let basename = target_specifier.path().rsplit_once('/').unwrap().1;
   let emit_options = deno_ast::EmitOptions {
-    source_map: SourceMapOption::Inline,
-    source_map_file: None,
+    source_map: SourceMapOption::Separate,
+    source_map_file: Some(basename.to_owned()),
     inline_sources: false,
     keep_comments: true,
   };
 
-  let file_name = source.specifier().path().split('/').last().unwrap();
+  let file_name =
+    relative_import_specifier(target_specifier, source.specifier());
   let source_map =
     SourceMap::single(file_name, source.text_info().text_str().to_owned());
 
@@ -60,27 +65,37 @@ pub fn transpile_to_js(
       source.diagnostics(),
     )?;
 
-    let emitted = emit(&program, &comments, &source_map, &emit_options)?;
+    let EmittedSource {
+      mut text,
+      source_map,
+    } = emit(&program, &comments, &source_map, &emit_options)?;
+    if !text.ends_with('\n') {
+      text.push('\n');
+    }
+    text.push_str(format!("//# sourceMappingURL={}.map", basename).as_str());
 
-    Ok(emitted.text)
+    Ok((text, source_map.unwrap()))
   })
 }
 
 pub fn transpile_to_dts(
   source: &ParsedSource,
   fast_check_module: &FastCheckTypeModule,
   specifier_rewriter: SpecifierRewriter,
-) -> Result<String, anyhow::Error> {
+  target_specifier: &Url,
+) -> Result<(String, String), anyhow::Error> {
   let dts = fast_check_module.dts.as_ref().unwrap();
 
+  let basename = target_specifier.path().rsplit_once('/').unwrap().1;
   let emit_options = deno_ast::EmitOptions {
-    source_map: SourceMapOption::Inline,
-    source_map_file: None,
+    source_map: SourceMapOption::Separate,
+    source_map_file: Some(basename.to_owned()),
     inline_sources: false,
     keep_comments: true,
   };
 
-  let file_name = source.specifier().path().split('/').last().unwrap();
+  let file_name =
+    relative_import_specifier(target_specifier, source.specifier());
   let source_map =
     SourceMap::single(file_name, source.text_info().text_str().to_owned());
 
@@ -94,8 +109,14 @@ pub fn transpile_to_dts(
   };
   program.visit_mut_with(&mut import_rewrite_transformer);
 
-  let EmittedSource { text, .. } =
-    emit(&program, &comments, &source_map, &emit_options)?;
+  let EmittedSource {
+    mut text,
+    source_map,
+  } = emit(&program, &comments, &source_map, &emit_options)?;
+  if !text.ends_with('\n') {
+    text.push('\n');
+  }
+  text.push_str(format!("//# sourceMappingURL={}.map", basename).as_str());
 
-  Ok(text)
+  Ok((text, source_map.unwrap()))
 }

api/src/npm/emit.rs

@@ -29,7 +29,7 @@ pub use self::tarball::NpmTarballOptions;
 pub use self::types::NpmMappedJsrPackageName;
 use self::types::NpmVersionInfo;
 
-pub const NPM_TARBALL_REVISION: u32 = 9;
+pub const NPM_TARBALL_REVISION: u32 = 10;
 
 pub async fn generate_npm_version_manifest<'a>(
   db: &Database,

api/src/npm/mod.rs

@@ -1,5 +1,6 @@
 // Copyright 2024 the JSR authors. All rights reserved. MIT license.
 
+use std::borrow::Cow;
 use std::collections::HashMap;
 
 use deno_ast::ModuleSpecifier;
@@ -40,22 +41,41 @@ impl<'a> SpecifierRewriter<'a> {
       RewriteKind::Declaration => self.declaration_rewrites,
     };
 
-    let resolved_specifier = follow_specifier(specifier, rewrites)?;
+    let mut resolved_specifier =
+      Cow::Borrowed(follow_specifier(specifier, rewrites)?);
 
     if let Some(specifier) =
       rewrite_npm_and_jsr_specifier(resolved_specifier.as_str())
     {
       return Some(specifier);
     };
 
-    if resolved_specifier == specifier {
+    if matches!(kind, RewriteKind::Declaration)
+      && resolved_specifier.scheme() == "file"
+    {
+      let path = resolved_specifier.path();
+      if path.ends_with(".d.ts") || path.ends_with(".d.mts") {
+        // If the base specifier is a declaration file, and a dependency is also a
+        // declaration file, TypeScript will not allow the import (TS2846). In
+        // this case, replace the `.d.ts` extension in the resolved specifier
+        // with `.js` so that TypeScript thinks we're importing a source file,
+        // which is allowed. It will then probe for the `.d.ts` file, which it
+        // will find.
+        // We do not use extensionless imports, because TypeScript does not
+        // allow them under `moduleResolution: "nodenext"` (TS2835).
+        let path = rewrite_path_extension(path, Extension::Js).unwrap();
+        resolved_specifier.to_mut().set_path(&path);
+      }
+    }
+
+    if *resolved_specifier == *specifier {
       // No need to rewrite if the specifier is the same as the resolved
       // specifier.
       return None;
     }
 
     let new_specifier = if resolved_specifier.scheme() == "file" {
-      relative_import_specifier(self.base_specifier, resolved_specifier)
+      relative_import_specifier(self.base_specifier, &resolved_specifier)
     } else {
       resolved_specifier.to_string()
     };
@@ -142,14 +162,18 @@ pub enum Extension {
   Dts,
 }
 
-pub fn rewrite_file_specifier_extension(
+pub fn rewrite_file_specifier(
   specifier: &ModuleSpecifier,
+  prefix: &str,
   new_extension: Extension,
 ) -> Option<ModuleSpecifier> {
   assert_eq!(specifier.scheme(), "file");
   let path = specifier.path();
   let rewritten_path = rewrite_path_extension(path, new_extension)?;
-  Some(ModuleSpecifier::parse(&format!("file://{}", rewritten_path)).unwrap())
+  Some(
+    ModuleSpecifier::parse(&format!("file://{prefix}{rewritten_path}"))
+      .unwrap(),
+  )
 }
 
 pub fn rewrite_path_extension(