Skip to content

feat: add package download button#1586

Merged
ghostdevv merged 37 commits intonpmx-dev:mainfrom
Adebesin-Cell:feat/download-button
Mar 21, 2026
Merged

feat: add package download button#1586
ghostdevv merged 37 commits intonpmx-dev:mainfrom
Adebesin-Cell:feat/download-button

Conversation

@Adebesin-Cell
Copy link
Contributor

@Adebesin-Cell Adebesin-Cell commented Feb 22, 2026

🔗 Linked issue

resolves #1528

🧭 Context

There was previously no way to directly download a package tarball or fetch all dependencies from the package detail page.

This PR introduces a Download button to make that happen.

📚 Description

This change adds a new Download button to the package detail page. The button includes a dropdown menu with two options:

  • Download the package .tgz tarball directly.
  • Generate and download a .jh script to fetch all dependencies.

Screenshot

-> OLD UI
Screenshot 2026-02-22 at 21 36 23

-> NEW UI
Screenshot 2026-03-17 at 04 41 54

@vercel
Copy link

vercel bot commented Feb 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 21, 2026 10:12pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 21, 2026 10:12pm
npmx-lunaria Ignored Ignored Mar 21, 2026 10:12pm

Request Review

@github-actions
Copy link

github-actions bot commented Feb 22, 2026

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
i18n/locales/en.json Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@Adebesin-Cell Adebesin-Cell changed the title feat: Add package download button feat: add package download button Feb 22, 2026
@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

❌ Patch coverage is 37.93103% with 18 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Package/DownloadButton.vue 33.33% 15 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces a download feature for npm packages. The changes extend the Button component with a 'subtle' variant for secondary actions, create a new DownloadButton component with dropdown functionality for package and dependency downloads, and establish a shared type system for install size data. Supporting changes include server-side dependency resolution enhancements to track tarball URLs, localisation strings for download actions, and accessibility testing for the new component.

Possibly related PRs

Suggested labels

front, a11y

Suggested reviewers

  • danielroe
  • alexdln
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR successfully implements the core requirements from issue #1528: adding a download button with options to download the package tarball and generate/download a script for dependencies.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the download functionality: new DownloadButton component, integration into package detail page, shared type definitions, i18n entries, and corresponding tests.
Description check ✅ Passed The pull request description is directly related to the changeset, clearly explaining the addition of a Download button with dropdown menu options.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
app/pages/package/[[org]]/[name].vue (1)

1144-1154: Consider showing the download button without waiting for install-size.
Right now the button only appears once install-size resolves; if the fetch is slow or fails, users lose direct tarball download. Consider rendering on displayVersion and disabling the dependencies action until installSize arrives.

Possible tweak
-            <PackageDownloadButton
-              v-if="displayVersion && installSize"
+            <PackageDownloadButton
+              v-if="displayVersion"
               :package-name="pkg.name"
               :version="displayVersion"
-              :install-size="installSize"
+              :install-size="installSize ?? undefined"
               size="small"
             />

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
app/components/Package/DownloadButton.vue (1)

185-187: Use same-name shorthand for size binding.

This can be simplified with Vue’s same-name shorthand.

♻️ Suggested tweak
-    :size="size"
+    :size

Based on learnings, "In Vue 3.4 and later, you can use same-name shorthand for attribute bindings: use :attributeName instead of :attributeName="attributeName" when binding to a variable with the same name in scope. Apply this shorthand in .vue components."


ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07261cb and dafc475.

📒 Files selected for processing (3)
  • app/components/Button/Base.vue
  • app/components/Package/DownloadButton.vue
  • test/nuxt/a11y.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Button/Base.vue

@serhalp serhalp added needs review This PR is waiting for a review from a maintainer stale This has become stale and may be closed soon labels Mar 15, 2026
@serhalp serhalp self-assigned this Mar 16, 2026
@serhalp
Copy link
Member

serhalp commented Mar 16, 2026

@Adebesin-Cell 👋🏼 I'd love to get this great feature reviewed and shipped! Would you mind rebasing it first? 🙏🏼

- Hide dependencies menu item when install size data is unavailable
- Guard against non-OK fetch responses before creating blob
- Use Vue 3.4 same-name shorthand for :size binding
- Remove console.error in favor of silent fallback
- Fix missing test closure in a11y spec
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/components/Package/DownloadButton.vue (1)

49-50: Prefer ButtonBase exposed methods over direct $el access.

Lines 49 and 95 use triggerRef.value?.$el, but ButtonBase already exposes getBoundingClientRect() and focus(). Using the exposed API is more robust and type-safe.

Suggested refactor
-    const rect = triggerRef.value?.$el?.getBoundingClientRect()
+    const rect = triggerRef.value?.getBoundingClientRect?.()
@@
-      triggerRef.value?.$el?.focus()
+      triggerRef.value?.focus?.()

As per coding guidelines: “Ensure you write strictly type-safe code”.

Also applies to: 95-95


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 136fb989-fa04-41b0-8ae2-f1cd9e05c286

📥 Commits

Reviewing files that changed from the base of the PR and between dafc475 and 14912af.

📒 Files selected for processing (5)
  • app/components/Button/Base.vue
  • app/components/Package/DownloadButton.vue
  • app/pages/package/[[org]]/[name].vue
  • i18n/locales/en.json
  • i18n/schema.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • i18n/schema.json

The DownloadButton prop expects `InstallSizeResult | null`, not
`undefined`. Fixes the vue-tsc type check failure.
useId() generates different IDs on server vs client, and the component
uses browser-only APIs (useMediaQuery, document, Teleport). Wrapping
in ClientOnly avoids the hydration mismatch entirely.
Copy link
Contributor

@ghostdevv ghostdevv left a comment

Choose a reason for hiding this comment

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

What if we downloaded the tarballs for the user instead of having them run a script?

@Adebesin-Cell
Copy link
Contributor Author

What if we downloaded the tarballs for the user instead of having them run a script?

Interesting idea, though my concern is the number of files, a package like express has ~60 deps. Serving all those tarballs individually would be a lot.

We could zip them into a single archive server-side (e.g., using something like archiver / zip-stream), fetching the tarballs on demand and streaming them into a zip as the user requests the download, rather than pre-fetching everything. That keeps it manageable.

That said, @serhalp also had second thoughts on the dependency download part of this PR and suggested punting on it to revisit in a follow-up. The single-package .tgz download works well as-is.

Maybe we land the single-package download now and open a separate issue to discuss the bulk dependency download approach (zipping, streaming, etc.)?

@ghostdevv
Copy link
Contributor

Interesting idea, though my concern is the number of files, a package like express has ~60 deps. Serving all those tarballs individually would be a lot.

But you'd need to download the files anyway? Is the idea that the script just gives you the option to do it later?

We could zip them into a single archive server-side (e.g., using something like archiver / zip-stream), fetching the tarballs on demand and streaming them into a zip as the user requests the download, rather than pre-fetching everything. That keeps it manageable.

I'd be worried about the memory usage

Adebesin-Cell and others added 3 commits March 21, 2026 18:51
Keep only the package tarball download option and remove the Node.js
script generation for downloading dependencies.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Adebesin-Cell
Copy link
Contributor Author

Interesting idea, though my concern is the number of files, a package like express has ~60 deps. Serving all those tarballs individually would be a lot.

But you'd need to download the files anyway? Is the idea that the script just gives you the option to do it later?

We could zip them into a single archive server-side (e.g., using something like archiver / zip-stream), fetching the tarballs on demand and streaming them into a zip as the user requests the download, rather than pre-fetching everything. That keeps it manageable.

I'd be worried about the memory usage

Yeah, you're right! I've removed the download dependencies script option for now in 5e0882f.

We can open a separate issue to discuss the best approach for bulk dependency downloads if it's something we want to revisit.

@ghostdevv ghostdevv force-pushed the feat/download-button branch from 3a75c84 to b942497 Compare March 21, 2026 22:04
@ghostdevv
Copy link
Contributor

@Adebesin-Cell I removed the dropdown for now, as well as simplified the subtle button code

We should do a follow up for subtle button because there are a few places that use button base but have some subtle styles on them. The package manager dropdown uses the old button and can probably be updated to use button base with a new subtle style. Here are some examples of how the newer button style has a subtle hint to it:

image

@ghostdevv ghostdevv force-pushed the feat/download-button branch from b942497 to 02d6bb1 Compare March 21, 2026 22:09
@ghostdevv ghostdevv enabled auto-merge March 21, 2026 22:10
@Adebesin-Cell
Copy link
Contributor Author

@Adebesin-Cell I removed the dropdown for now, as well as simplified the subtle button code

We should do a follow up for subtle button because there are a few places that use button base but have some subtle styles on them. The package manager dropdown uses the old button and can probably be updated to use button base with a new subtle style. Here are some examples of how the newer button style has a subtle hint to it:

image

Hey! Thanks for this, it looks neat 👏. I was actually going to ask if we should declutter the dropdown into a button. Glad you went ahead and did it!

Thanks for staying on top of the review and helping improve the PR 🙏

@ghostdevv ghostdevv added this pull request to the merge queue Mar 21, 2026
Merged via the queue into npmx-dev:main with commit 022d207 Mar 21, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review This PR is waiting for a review from a maintainer stale This has become stale and may be closed soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Add a button to direct download a dependency

3 participants