fix: improve npm tarball emit (#493)

Some relative specifiers were being incorrectly emitted when referencing a `.js` file from a `.d.ts` file in the dist folder. Additionally, `.js` files were previously emitted into a `_dist` folder, which broke users that need to relatively access files in the NPM package using `import.meta.url` or similar. Now only `.d.ts` files are emitted into the `_dist` folder. Fixes #491 Fixes #477
jsr-io · May 8, 2024 · 7e28877 · 7e28877
1 parent 16e07f5
commit 7e28877
Show file tree

Hide file tree

Showing 12 changed files with 141 additions and 83 deletions.
diff --git a/api/src/npm/mod.rs b/api/src/npm/mod.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 = 10;
+pub const NPM_TARBALL_REVISION: u32 = 11;
 
 pub async fn generate_npm_version_manifest<'a>(
   db: &Database,

diff --git a/api/src/npm/specifiers.rs b/api/src/npm/specifiers.rs
@@ -29,6 +29,7 @@ pub struct SpecifierRewriter<'a> {
 
 impl<'a> SpecifierRewriter<'a> {
   pub fn rewrite(&self, specifier: &str, kind: RewriteKind) -> Option<String> {
+    let source_text_specifier = specifier;
     let dep = self.dependencies.get(specifier)?;
 
     let specifier = match kind {
@@ -68,18 +69,18 @@ impl<'a> SpecifierRewriter<'a> {
       }
     }
 
-    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)
     } else {
       resolved_specifier.to_string()
     };
 
+    if new_specifier == source_text_specifier {
+      // No need to rewrite if the specifier is the same as the resolved
+      // specifier.
+      return None;
+    }
+
     Some(new_specifier)
   }
 }

diff --git a/api/src/npm/tarball.rs b/api/src/npm/tarball.rs
@@ -136,7 +136,7 @@ pub async fn create_npm_tarball<'a>(
       }
       deno_ast::MediaType::Jsx => {
         let source_specifier =
-          rewrite_file_specifier(module.specifier(), "/_dist", Extension::Js);
+          rewrite_file_specifier(module.specifier(), "", Extension::Js);
         if let Some(source_specifier) = source_specifier {
           source_rewrites.insert(module.specifier(), source_specifier);
         }
@@ -153,7 +153,7 @@ pub async fn create_npm_tarball<'a>(
       }
       deno_ast::MediaType::TypeScript | deno_ast::MediaType::Mts => {
         let source_specifier =
-          rewrite_file_specifier(module.specifier(), "/_dist", Extension::Js);
+          rewrite_file_specifier(module.specifier(), "", Extension::Js);
         if let Some(source_specifier) = source_specifier.clone() {
           source_rewrites.insert(module.specifier(), source_specifier);
         }

diff --git a/api/testdata/specs/npm_tarballs/additional_files.txt b/api/testdata/specs/npm_tarballs/additional_files.txt
@@ -28,13 +28,6 @@ export declare const foo: string;
 == /_dist/foo.d.ts.map ==
 {"version":3,"file":"foo.d.ts","sources":["../foo.ts"],"names":[],"mappings":"AAAA,OAAO,cAAM,KAAK,MAAM,CAAS"}
 
-== /_dist/foo.js ==
-export const foo = 'bar';
-//# sourceMappingURL=foo.js.map
-
-== /_dist/foo.js.map ==
-{"version":3,"file":"foo.js","sources":["../foo.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAc,MAAM"}
-
 == /bar.json ==
 console.log('foo');
 
@@ -44,6 +37,13 @@ this is data
 == /foo.d.ts ==
 // unrelated content is overwritten
 
+== /foo.js ==
+export const foo = 'bar';
+//# sourceMappingURL=foo.js.map
+
+== /foo.js.map ==
+{"version":3,"file":"foo.js","sources":["./foo.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAc,MAAM"}
+
 == /foo.ts ==
 export const foo: string = 'bar';
 
@@ -67,7 +67,7 @@ export const foo: string = 'bar';
   "exports": {
     "./foo": {
       "types": "./_dist/foo.d.ts",
-      "default": "./_dist/foo.js"
+      "default": "./foo.js"
     },
     "./bar": {
       "default": "./bar.json"

diff --git a/api/testdata/specs/npm_tarballs/import_jsr.txt b/api/testdata/specs/npm_tarballs/import_jsr.txt
@@ -40,19 +40,19 @@ await import("jsr:@luca/flag@1")
 == /_dist/baz.d.ts.map ==
 {"version":3,"file":"baz.d.ts","sources":[],"names":[],"mappings":""}
 
-== /_dist/baz.js ==
+== /bar.mjs ==
+
+import { html } from "@jsr/luca__flag";
+await import("@jsr/luca__flag")
+
+== /baz.js ==
 import { html } from "@jsr/luca__flag";
 html();
 await import("@jsr/luca__flag");
 //# sourceMappingURL=baz.js.map
 
-== /_dist/baz.js.map ==
-{"version":3,"file":"baz.js","sources":["../baz.ts"],"names":[],"mappings":"AAAA,SAAS,IAAI,0BAA2B;AACxC;AACA,MAAM,MAAM,CAAC"}
-
-== /bar.mjs ==
-
-import { html } from "@jsr/luca__flag";
-await import("@jsr/luca__flag")
+== /baz.js.map ==
+{"version":3,"file":"baz.js","sources":["./baz.ts"],"names":[],"mappings":"AAAA,SAAS,IAAI,0BAA2B;AACxC;AACA,MAAM,MAAM,CAAC"}
 
 == /baz.ts ==
 import { html } from "@jsr/luca__flag";
@@ -98,7 +98,7 @@ await import("@jsr/luca__flag")
     },
     "./baz": {
       "types": "./_dist/baz.d.ts",
-      "default": "./_dist/baz.js"
+      "default": "./baz.js"
     },
     "./fizz": {
       "default": "./fizz.d.ts"

diff --git a/api/testdata/specs/npm_tarballs/import_npm.txt b/api/testdata/specs/npm_tarballs/import_npm.txt
@@ -40,19 +40,19 @@ await import("npm:lit@^2.2.7")
 == /_dist/baz.d.ts.map ==
 {"version":3,"file":"baz.d.ts","sources":[],"names":[],"mappings":""}
 
-== /_dist/baz.js ==
+== /bar.mjs ==
+
+import { html } from "lit";
+await import("lit")
+
+== /baz.js ==
 import { html } from "lit";
 html();
 await import("lit");
 //# sourceMappingURL=baz.js.map
 
-== /_dist/baz.js.map ==
-{"version":3,"file":"baz.js","sources":["../baz.ts"],"names":[],"mappings":"AAAA,SAAS,IAAI,cAAyB;AACtC;AACA,MAAM,MAAM,CAAC"}
-
-== /bar.mjs ==
-
-import { html } from "lit";
-await import("lit")
+== /baz.js.map ==
+{"version":3,"file":"baz.js","sources":["./baz.ts"],"names":[],"mappings":"AAAA,SAAS,IAAI,cAAyB;AACtC;AACA,MAAM,MAAM,CAAC"}
 
 == /baz.ts ==
 import { html } from "lit";
@@ -98,7 +98,7 @@ await import("lit")
     },
     "./baz": {
       "types": "./_dist/baz.d.ts",
-      "default": "./_dist/baz.js"
+      "default": "./baz.js"
     },
     "./fizz": {
       "default": "./fizz.d.ts"

diff --git a/api/testdata/specs/npm_tarballs/jsdoc_import.txt b/api/testdata/specs/npm_tarballs/jsdoc_import.txt
@@ -26,11 +26,11 @@ export type Num = number;
 == /_dist/foo.d.ts.map ==
 {"version":3,"file":"foo.d.ts","sources":["../foo.ts"],"names":[],"mappings":"AAAA,YAAY,MAAM,MAAM"}
 
-== /_dist/foo.js ==
+== /foo.js ==
 
 //# sourceMappingURL=foo.js.map
 
-== /_dist/foo.js.map ==
+== /foo.js.map ==
 {"version":3,"file":"foo.js","sources":[],"names":[],"mappings":""}
 
 == /foo.ts ==

diff --git a/api/testdata/specs/npm_tarballs/slow_types_transpile.txt b/api/testdata/specs/npm_tarballs/slow_types_transpile.txt
@@ -9,20 +9,20 @@ export const foo = bar();
 }
 
 # output
-== /_dist/main.js ==
-export const foo = bar();
-//# sourceMappingURL=main.js.map
-
-== /_dist/main.js.map ==
-{"version":3,"file":"main.js","sources":["../main.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAM,MAAM"}
-
 == /jsr.json ==
 {
   "name": "@scope/foo",
   "version": "1.0.0",
   "exports": "./main.ts"
 }
 
+== /main.js ==
+export const foo = bar();
+//# sourceMappingURL=main.js.map
+
+== /main.js.map ==
+{"version":3,"file":"main.js","sources":["./main.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAM,MAAM"}
+
 == /main.ts ==
 export const foo = bar();
 
@@ -35,7 +35,7 @@ export const foo = bar();
   "dependencies": {},
   "exports": {
     ".": {
-      "default": "./_dist/main.js"
+      "default": "./main.js"
     }
   },
   "_jsr_revision": 0

diff --git a/api/testdata/specs/npm_tarballs/transpile.txt b/api/testdata/specs/npm_tarballs/transpile.txt
@@ -30,23 +30,23 @@ export declare function bar(): string;
 == /_dist/main.d.ts.map ==
 {"version":3,"file":"main.d.ts","sources":["../main.ts"],"names":[],"mappings":"AAAA,OAAO,cAAM,KAAK,MAAM,CAAS;AACjC,OAAO,cAAM,KAAM,MAAe;AAElC,iBAAiB;EACf,KAAK,MAAM;;AAGb,OAAO,iBAAS,OAAO,MAAM"}
 
-== /_dist/main.js ==
+== /jsr.json ==
+{
+  "name": "@scope/foo",
+  "version": "1.0.0",
+  "exports": "./main.ts"
+}
+
+== /main.js ==
 export const foo = 'foo';
 export const bar = "bar";
 export function bar() {
   return 'bar';
 }
 //# sourceMappingURL=main.js.map
 
-== /_dist/main.js.map ==
-{"version":3,"file":"main.js","sources":["../main.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAc,MAAM;AACjC,OAAO,MAAM,MAAM,MAAe;AAMlC,OAAO,SAAS;EACd,OAAO;AACT"}
-
-== /jsr.json ==
-{
-  "name": "@scope/foo",
-  "version": "1.0.0",
-  "exports": "./main.ts"
-}
+== /main.js.map ==
+{"version":3,"file":"main.js","sources":["./main.ts"],"names":[],"mappings":"AAAA,OAAO,MAAM,MAAc,MAAM;AACjC,OAAO,MAAM,MAAM,MAAe;AAMlC,OAAO,SAAS;EACd,OAAO;AACT"}
 
 == /main.ts ==
 export const foo: string = 'foo';
@@ -70,7 +70,7 @@ export function bar(): string {
   "exports": {
     ".": {
       "types": "./_dist/main.d.ts",
-      "default": "./_dist/main.js"
+      "default": "./main.js"
     }
   },
   "_jsr_revision": 0

diff --git a/api/testdata/specs/npm_tarballs/transpile_with_imports.txt b/api/testdata/specs/npm_tarballs/transpile_with_imports.txt
@@ -23,36 +23,36 @@ export declare function add(a: number, b: number): number;
 == /_dist/bar.d.ts.map ==
 {"version":3,"file":"bar.d.ts","sources":["../bar.ts"],"names":[],"mappings":"AAAA,OAAO,iBAAS,IAAI,GAAG,MAAM,EAAE,GAAG,MAAM,GAAG,MAAM"}
 
-== /_dist/bar.js ==
-export function add(a, b) {
-  return a + b;
-}
-//# sourceMappingURL=bar.js.map
-
-== /_dist/bar.js.map ==
-{"version":3,"file":"bar.js","sources":["../bar.ts"],"names":[],"mappings":"AAAA,OAAO,SAAS,IAAI,CAAS,EAAE,CAAS;EACtC,OAAO,IAAI;AACb"}
-
 == /_dist/foo.d.ts ==
 export { add } from "./bar.js";
 //# sourceMappingURL=foo.d.ts.map
 
 == /_dist/foo.d.ts.map ==
 {"version":3,"file":"foo.d.ts","sources":["../foo.ts"],"names":[],"mappings":"AAAA,SAAS,GAAG,mBAAmB"}
 
-== /_dist/foo.js ==
-export { add } from "./bar.js";
-//# sourceMappingURL=foo.js.map
+== /bar.js ==
+export function add(a, b) {
+  return a + b;
+}
+//# sourceMappingURL=bar.js.map
 
-== /_dist/foo.js.map ==
-{"version":3,"file":"foo.js","sources":["../foo.ts"],"names":[],"mappings":"AAAA,SAAS,GAAG,mBAAmB"}
+== /bar.js.map ==
+{"version":3,"file":"bar.js","sources":["./bar.ts"],"names":[],"mappings":"AAAA,OAAO,SAAS,IAAI,CAAS,EAAE,CAAS;EACtC,OAAO,IAAI;AACb"}
 
 == /bar.ts ==
 export function add(a: number, b: number): number {
   return a + b;
 }
 
+== /foo.js ==
+export { add } from "./bar.js";
+//# sourceMappingURL=foo.js.map
+
+== /foo.js.map ==
+{"version":3,"file":"foo.js","sources":["./foo.ts"],"names":[],"mappings":"AAAA,SAAS,GAAG,mBAAmB"}
+
 == /foo.ts ==
-export { add } from "./_dist/bar.js";
+export { add } from "./bar.js";
 
 == /jsr.json ==
 {
@@ -73,7 +73,7 @@ export { add } from "./_dist/bar.js";
   "exports": {
     ".": {
       "types": "./_dist/foo.d.ts",
-      "default": "./_dist/foo.js"
+      "default": "./foo.js"
     }
   },
   "_jsr_revision": 0

diff --git a/api/testdata/specs/npm_tarballs/ts_importing_js_dts.txt b/api/testdata/specs/npm_tarballs/ts_importing_js_dts.txt
@@ -0,0 +1,57 @@
+# mod.ts
+export * from "./sub/lib.js";
+
+# sub/lib.js
+export default 1;
+
+# sub/lib.d.ts
+declare const _default: number;
+export default _default;
+
+# jsr.json
+{
+  "name": "@scope/foo",
+  "version": "0.0.1",
+  "exports": "./mod.ts"
+}
+
+# output
+== /jsr.json ==
+{
+  "name": "@scope/foo",
+  "version": "0.0.1",
+  "exports": "./mod.ts"
+}
+
+== /mod.js ==
+export * from "./sub/lib.js";
+//# sourceMappingURL=mod.js.map
+
+== /mod.js.map ==
+{"version":3,"file":"mod.js","sources":["./mod.ts"],"names":[],"mappings":"AAAA,cAAc,eAAe"}
+
+== /mod.ts ==
+export * from "./sub/lib.js";
+
+== /package.json ==
+{
+  "name": "@jsr/scope__foo",
+  "version": "0.0.1",
+  "homepage": "http://jsr.test/@scope/foo",
+  "type": "module",
+  "dependencies": {},
+  "exports": {
+    ".": {
+      "default": "./mod.js"
+    }
+  },
+  "_jsr_revision": 0
+}
+
+== /sub/lib.d.ts ==
+declare const _default: number;
+export default _default;
+
+== /sub/lib.js ==
+export default 1;
+