Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add bin field to npm compat tarballs #284

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/migrations/20240314142321_npm_tarball_bins.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE npm_tarballs ADD COLUMN bin jsonb NOT NULL DEFAULT '{}';
18 changes: 10 additions & 8 deletions api/src/db/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1439,15 +1439,16 @@ impl Database {
}

sqlx::query!(
r#"INSERT INTO npm_tarballs (scope, name, version, revision, sha1, sha512, size)
VALUES ($1, $2, $3, $4, $5, $6, $7)"#,
r#"INSERT INTO npm_tarballs (scope, name, version, revision, sha1, sha512, size, bin)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)"#,
new_npm_tarball.scope as _,
new_npm_tarball.name as _,
new_npm_tarball.version as _,
new_npm_tarball.revision,
new_npm_tarball.sha1,
new_npm_tarball.sha512,
new_npm_tarball.size,
new_npm_tarball.bin as _,
)
.execute(&mut *tx)
.await?;
Expand Down Expand Up @@ -1614,16 +1615,17 @@ impl Database {
) -> Result<NpmTarball> {
sqlx::query_as!(
NpmTarball,
r#"INSERT INTO npm_tarballs (scope, name, version, revision, sha1, sha512, size)
VALUES ($1, $2, $3, $4, $5, $6, $7)
RETURNING scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, updated_at, created_at"#,
r#"INSERT INTO npm_tarballs (scope, name, version, revision, sha1, sha512, size, bin)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
RETURNING scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, bin as "bin: NpmBinEntries", updated_at, created_at"#,
new_npm_tarball.scope as _,
new_npm_tarball.name as _,
new_npm_tarball.version as _,
new_npm_tarball.revision,
new_npm_tarball.sha1,
new_npm_tarball.sha512,
new_npm_tarball.size
new_npm_tarball.size,
new_npm_tarball.bin as _
)
.fetch_one(&self.pool)
.await
Expand Down Expand Up @@ -2725,7 +2727,7 @@ impl Database {
) -> Result<Option<NpmTarball>> {
sqlx::query_as!(
NpmTarball,
r#"SELECT scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, updated_at, created_at
r#"SELECT scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, bin as "bin: NpmBinEntries", updated_at, created_at
FROM npm_tarballs
WHERE scope = $1 AND name = $2 AND version = $3
ORDER BY revision DESC
Expand All @@ -2752,7 +2754,7 @@ impl Database {
) -> Result<Option<NpmTarball>> {
sqlx::query_as!(
NpmTarball,
r#"SELECT scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, updated_at, created_at
r#"SELECT scope as "scope: ScopeName", name as "name: PackageName", version as "version: Version", revision, sha1, sha512, size, bin as "bin: NpmBinEntries", updated_at, created_at
FROM npm_tarballs
WHERE scope = $1 AND name = $2 AND version = $3 AND revision = $4
LIMIT 1"#,
Expand Down
54 changes: 54 additions & 0 deletions api/src/db/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,10 @@ impl ExportsMap {
pub fn into_inner(self) -> IndexMap<String, String> {
self.0
}

pub fn get(&self, key: &str) -> Option<&String> {
self.0.get(key)
}
}

impl sqlx::Decode<'_, sqlx::Postgres> for ExportsMap {
Expand Down Expand Up @@ -692,6 +696,7 @@ pub struct NpmTarball {
pub sha1: String,
pub sha512: String,
pub size: i32,
pub bin: NpmBinEntries,
pub updated_at: DateTime<Utc>,
pub created_at: DateTime<Utc>,
}
Expand All @@ -705,6 +710,55 @@ pub struct NewNpmTarball<'s> {
pub sha1: &'s str,
pub sha512: &'s str,
pub size: i32,
pub bin: &'s NpmBinEntries,
}

#[derive(Debug, Clone)]
pub struct NpmBinEntries(IndexMap<String, String>);

impl NpmBinEntries {
pub fn new(bins: IndexMap<String, String>) -> Self {
Self(bins)
}

#[cfg(test)]
pub fn mock() -> Self {
Self::new(IndexMap::new())
}

pub fn into_inner(self) -> IndexMap<String, String> {
self.0
}
}

impl sqlx::Decode<'_, sqlx::Postgres> for NpmBinEntries {
fn decode(
value: sqlx::postgres::PgValueRef<'_>,
) -> Result<Self, Box<dyn std::error::Error + 'static + Send + Sync>> {
let s: sqlx::types::Json<IndexMap<String, String>> =
sqlx::Decode::<'_, sqlx::Postgres>::decode(value)?;
Ok(NpmBinEntries(s.0))
}
}

impl<'q> sqlx::Encode<'q, sqlx::Postgres> for NpmBinEntries {
fn encode_by_ref(
&self,
buf: &mut <sqlx::Postgres as sqlx::database::HasArguments<'q>>::ArgumentBuffer,
) -> sqlx::encode::IsNull {
<sqlx::types::Json<&IndexMap<String, String>> as sqlx::Encode<
'_,
sqlx::Postgres,
>>::encode_by_ref(&sqlx::types::Json(&self.0), buf)
}
}

impl sqlx::Type<sqlx::Postgres> for NpmBinEntries {
fn type_info() -> <sqlx::Postgres as sqlx::Database>::TypeInfo {
<sqlx::types::Json<IndexMap<String, String>> as sqlx::Type<
sqlx::Postgres,
>>::type_info()
}
}

/// Keys reference https://runtime-keys.proposal.wintercg.org/.
Expand Down
3 changes: 2 additions & 1 deletion api/src/db/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ async fn create_package_version_and_finalize_publishing_task() {
sha1: "",
sha512: "",
size: 0,
bin: &NpmBinEntries::mock(),
};

let task = db
Expand All @@ -376,7 +377,7 @@ async fn create_package_version_and_finalize_publishing_task() {
readme_path: None,
uses_npm: true,
exports: &ExportsMap::mock(),
meta: Default::default(),
meta: PackageVersionMeta::default(),
},
&package_files,
&package_version_dependencies,
Expand Down
3 changes: 2 additions & 1 deletion api/src/npm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub use self::tarball::NpmTarballOptions;
pub use self::types::NpmMappedJsrPackageName;
use self::types::NpmVersionInfo;

pub const NPM_TARBALL_REVISION: u32 = 7;
pub const NPM_TARBALL_REVISION: u32 = 8;

pub async fn generate_npm_version_manifest<'a>(
db: &Database,
Expand Down Expand Up @@ -124,6 +124,7 @@ pub async fn generate_npm_version_manifest<'a>(
integrity: format!("sha512-{}", npm_tarball.sha512),
},
dependencies: npm_dependencies,
bin: npm_tarball.bin.into_inner(),
};

out
Expand Down
64 changes: 58 additions & 6 deletions api/src/npm/tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use url::Url;
use crate::buckets::BucketWithQueue;
use crate::db::DependencyKind;
use crate::db::ExportsMap;
use crate::db::NpmBinEntries;
use crate::ids::PackageName;
use crate::ids::PackagePath;
use crate::ids::ScopeName;
Expand All @@ -47,6 +48,9 @@ pub struct NpmTarball {
pub sha1: String,
/// The base64 encoded sha512 hash of the gzipped tarball.
pub sha512: String,
/// The bin field from the package.json. This is used to create the bin field
/// in the package version manifest.
pub bin: NpmBinEntries,
}

pub enum NpmTarballFiles<'a> {
Expand Down Expand Up @@ -92,24 +96,27 @@ pub async fn create_npm_tarball<'a>(

let npm_package_id = NpmMappedJsrPackageName { scope, package };

let npm_exports = create_npm_exports(exports);

let npm_dependencies =
create_npm_dependencies(dependencies.map(Cow::Borrowed))?;

let homepage = Url::options()
.base_url(Some(registry_url))
.parse(&format!("./@{scope}/{package}",))
.unwrap()
.to_string();

let npm_exports = create_npm_exports(exports);

let npm_dependencies =
create_npm_dependencies(dependencies.map(Cow::Borrowed))?;

let npm_bin = create_npm_bin(package, exports, sources);

let pkg_json = NpmPackageJson {
name: npm_package_id,
version: version.clone(),
homepage,
module_type: "module".to_string(),
exports: npm_exports,
dependencies: npm_dependencies,
homepage,
bin: npm_bin.clone(),
revision: NPM_TARBALL_REVISION,
};

Expand Down Expand Up @@ -321,6 +328,7 @@ pub async fn create_npm_tarball<'a>(
tarball: tar_gz_bytes,
sha1,
sha512,
bin: NpmBinEntries::new(npm_bin),
})
}

Expand Down Expand Up @@ -361,6 +369,50 @@ pub fn create_npm_exports(exports: &ExportsMap) -> IndexMap<String, String> {
npm_exports
}

pub fn create_npm_bin(
package_name: &PackageName,
exports: &ExportsMap,
sources: &dyn ParsedSourceStore,
) -> IndexMap<String, String> {
let mut npm_bin = IndexMap::new();
for (key, path) in exports.iter() {
let Ok(url) = format!("file://{}", &path[1..]).parse() else {
continue;
};
let Some(source) = sources.get_parsed_source(&url) else {
continue;
};
if source.module().shebang.is_none() {
continue;
}

let bin_name = source
.comments()
.leading_map()
.iter()
.flat_map(|entry| entry.1.iter())
.find_map(|comment| {
if let Some(name) = comment.text.trim().strip_prefix("@jsrBin=") {
Some(name.to_string())
} else {
None
}
})
.unwrap_or_else(|| {
if key == "." {
package_name.to_string()
} else {
format!("{}-{}", package_name, key[2..].replace("/", "-"))
}
});

let import_path =
rewrite_specifier(path).unwrap_or_else(|| path.to_owned());
npm_bin.insert(bin_name, import_path);
}
npm_bin
}

fn to_range(
parsed_source: &ParsedSource,
range: &PositionRange,
Expand Down
5 changes: 5 additions & 0 deletions api/src/npm/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ pub struct NpmVersionInfo<'a> {
pub description: String,
pub dist: NpmDistInfo,
pub dependencies: IndexMap<String, String>,
#[serde(skip_serializing_if = "IndexMap::is_empty")]
pub bin: IndexMap<String, String>,
}

#[derive(Debug, Serialize)]
Expand All @@ -75,6 +77,9 @@ pub struct NpmPackageJson<'a> {
pub dependencies: IndexMap<String, String>,
pub exports: IndexMap<String, String>,

#[serde(skip_serializing_if = "IndexMap::is_empty")]
pub bin: IndexMap<String, String>,

#[serde(rename = "_jsr_revision")]
pub revision: u32,
}
1 change: 1 addition & 0 deletions api/src/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ async fn create_package_version_and_npm_tarball_and_update_publishing_task(
sha1: &npm_tarball_info.sha1,
sha512: &npm_tarball_info.sha512,
size: npm_tarball_info.size as i32,
bin: &npm_tarball_info.bin,
};

*publishing_task = db
Expand Down
4 changes: 4 additions & 0 deletions api/src/tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::analysis::RegistryLoader;
use crate::buckets::Buckets;
use crate::buckets::UploadTaskBody;
use crate::db::ExportsMap;
use crate::db::NpmBinEntries;
use crate::db::PublishingTask;
use crate::db::{DependencyKind, PackageVersionMeta};
use crate::gcp::GcsError;
Expand Down Expand Up @@ -71,6 +72,8 @@ pub struct NpmTarballInfo {
pub sha512: String,
/// The size of the tarball in bytes.
pub size: u64,
/// The bin field from the package.json. Empty if there is no bin field.
pub bin: NpmBinEntries,
}

#[instrument(
Expand Down Expand Up @@ -302,6 +305,7 @@ pub async fn process_tarball(
sha1: npm_tarball.sha1,
sha512: npm_tarball.sha512,
size: npm_tarball.tarball.len() as u64,
bin: npm_tarball.bin,
};

let npm_tarball_path = npm_tarball_path(
Expand Down
1 change: 1 addition & 0 deletions api/src/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub async fn npm_tarball_build_handler(
size: npm_tarball.tarball.len() as i32,
sha1: &npm_tarball.sha1,
sha512: &npm_tarball.sha512,
bin: &npm_tarball.bin,
};

let npm_tarball_path = gcs_paths::npm_tarball_path(
Expand Down
36 changes: 36 additions & 0 deletions frontend/docs/publishing-packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,39 @@ case, you can un-ignore the `dist/` directory by using a negation in the

In this case, the `dist/` directory will be included when publishing, even
though it is listed in the `.gitignore` file.

## Publishing command line interfaces (CLIs)

JSR supports publishing of command-line interfaces (CLIs) as part of a package.
This allows package users to execute your CLI from the command line using
`deno run`, `npx`, `yarn dlx`, and similar tools.

Unlike npm, JSR does not have a special field in the `jsr.json` / `deno.json` to
specify bin entrypoints. Instead, include the bin entrypoints in the `exports`
Comment on lines +457 to +458

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might make it difficult to publish existing packages to JSR, unless https://github.com/nodejs/modules/issues/274

Copy link
Member Author

@lucacasonato lucacasonato Mar 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you say so? Your bin entrypoint does not have to be in the root entrypoint.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly thrilled about having to resort to hacks, or rethink my package.json to match JSR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine with this if JSR transpiled import.meta.main to something Node can understand.

field. If your package only has a single bin entrypoint and no other
entrypoints, you can specify it as the default entrypoint in the `exports`
field.

```json
// jsr.json
{
"name": "@luca/greet",
"version": "1.0.0",
"exports": {
".": "./cli.ts"
}
}
```

Deno users will be able to use this CLI with `deno run`, as they can with any
remote package: `deno run jsr:@luca/greet`. If the bin is instead specified in a
non-default exports, they can still call it: `deno run jsr:@luca/greet/bin`.

For users using [JSR's npm compatibility layer](/docs/npm-compatibility), a
`bin` field is added to the `package.json` in the generated npm compatible
tarball. JSR will put all files specified in the `exports` in this field that
contain a [shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)#Examples). JSR
Comment on lines +479 to +481

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs should specify what shebang should universal code use. I believe it should be a Node shebang, since Deno will ignore it.

will automatically determine a binary name based on the package name and exports
key (`<PACKAGE_NAME>-<EXPORT_KEY>`, except for the root `.` export, where the
key is just `<PACKAGE_NAME>`). A package author can override the binary name by
specifying a `// @jsrBin=<BIN_NAME>` comment in the relevant JS/TS file.
Loading