feat: link to releases #1368
feat: link to releases #1368
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughAdds repository release support surfaced in the package UI: a new Release type and getMatchedRelease utility; provider adapters gain fetchReleases and releases links; useRepoMeta exposes releases, pending/error state and refresh; Package Versions and package page accept and use a Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/pages/package/[[org]]/[name].vue (1)
764-770: Both icon syntax forms are functionally equivalent in UnoCSS preset-icons;i-carbon-catalog(dash) andi-carbon:catalog(colon) resolve to the same icon. The dash syntax is often preferred in practice due to escaping considerations in CSS tooling. If the project has established a colon-syntax convention for consistency, update accordingly; otherwise, the current dash syntax is acceptable.app/components/Package/Versions.vue (2)
487-497: Duplicate function call for each version entry.
getMatchedRelease()is called twice per version—once for thev-ifcheck and again for the:tobinding. Consider computing the matched release once per row using a helper or inline binding to avoid redundant lookups.Additionally, per project learnings, prefer colon-syntax for UnoCSS icons:
i-carbon:cataloginstead ofi-carbon-catalog.♻️ Suggested approach
Option 1: Use a
v-bindwith a computed release in a wrapper element or use template variables.Option 2: Create a computed map of version→release at component level:
const versionReleaseMap = computed(() => { const releases = props.releases ?? [] const map = new Map<string, Release>() for (const r of releases) { // populate based on getMatchedRelease logic } return map })For the icon syntax fix:
-classicon="i-carbon-catalog size-3.5" +classicon="i-carbon:catalog size-3.5"Based on learnings: "In Vue components that use UnoCSS with the preset-icons collection, prefer colon-syntax for icons (e.g., i-carbon:checkmark) over the dash-separated form (i-carbon-checkmark)."
681-693: Inconsistent accessibility pattern across release links.This release link uses a manual icon span with
sr-onlytext (Lines 691-692), whereas the earlier release links (Lines 487-497, 562-568) use theclassiconprop withoutsr-only. This creates inconsistent screen reader experiences.Consider standardising all release links to use either:
- The
classiconprop consistently (if it handles accessibility internally), or- Manual icon +
sr-onlyspan everywhere
| return releases.map(release => ({ | ||
| tag: release.tag, | ||
| url: `https://github.com/${ref.owner}/${ref.repo}/releases/tag/${release.tag}`, | ||
| name: release.name, | ||
| publishedAt: release.publishedAt, |
There was a problem hiding this comment.
Encode tag values when building release URLs.
Tags can contain /, spaces, or other characters; interpolating them directly can break links.
🛠️ Suggested fix
- url: `https://github.com/${ref.owner}/${ref.repo}/releases/tag/${release.tag}`,
+ url: `https://github.com/${ref.owner}/${ref.repo}/releases/tag/${encodeURIComponent(release.tag)}`,- url: `https://${baseHost}/${ref.owner}/${ref.repo}/-/releases/${release.tag_name}`,
+ url: `https://${baseHost}/${ref.owner}/${ref.repo}/-/releases/${encodeURIComponent(release.tag_name)}`,- url: `https://bitbucket.org/${ref.owner}/${ref.repo}/src/${tag.name}`,
+ url: `https://bitbucket.org/${ref.owner}/${ref.repo}/src/${encodeURIComponent(tag.name)}`,Also applies to: 300-304, 377-380
|
Acknowledged, I will review this. |
| <LinkBase | ||
| v-if="getMatchedRelease(row.primaryVersion.version, props.releases ?? [])" | ||
| :to="getMatchedRelease(row.primaryVersion.version, props.releases ?? [])!.url" | ||
| classicon="i-carbon-catalog size-3.5" | ||
| :title=" | ||
| $t('package.releases.view_release_for_version', { | ||
| version: row.primaryVersion.version, | ||
| }) | ||
| " | ||
| class="inline-flex items-center text-fg-muted hover:text-fg transition-colors shrink-0" | ||
| /> |
There was a problem hiding this comment.
Add an accessible label for icon-only release links.
These LinkBase instances render icon-only links without visible text or an explicit aria-label, which risks an unnamed control for screen readers.
🧩 Suggested change
<LinkBase
v-if="getMatchedRelease(row.primaryVersion.version, props.releases ?? [])"
:to="getMatchedRelease(row.primaryVersion.version, props.releases ?? [])!.url"
classicon="i-carbon:catalog size-3.5"
:title="
$t('package.releases.view_release_for_version', {
version: row.primaryVersion.version,
})
"
+ :aria-label="
+ $t('package.releases.view_release_for_version', {
+ version: row.primaryVersion.version,
+ })
+ "
class="inline-flex items-center text-fg-muted hover:text-fg transition-colors shrink-0"
/>Also applies to: 562-568, 861-874
🧩 Analysis chain
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '485,500p'Repository: npmx-dev/npmx.dev
Length of output: 957
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '560,570p'Repository: npmx-dev/npmx.dev
Length of output: 733
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '689,695p'Repository: npmx-dev/npmx.dev
Length of output: 515
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '790,796p'Repository: npmx-dev/npmx.dev
Length of output: 510
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '870,876p'Repository: npmx-dev/npmx.dev
Length of output: 446
🏁 Script executed:
cat -n app/components/Package/Versions.vue | sed -n '945,951p'Repository: npmx-dev/npmx.dev
Length of output: 557
🏁 Script executed:
rg 'i-carbon-catalog' app/components/Package/Versions.vueRepository: npmx-dev/npmx.dev
Length of output: 474
🏁 Script executed:
rg 'classicon="i-carbon-' app/components/Package/Versions.vueRepository: npmx-dev/npmx.dev
Length of output: 228
Use UnoCSS colon-syntax for icons (i-carbon:catalog).
All six instances of the release catalogue icon use dash-separated syntax, which should be changed to colon-syntax. This aligns with the project's icon naming conventions and improves UnoCSS resolution performance.
Suggested changes
- classicon="i-carbon-catalog size-3.5"
+ classicon="i-carbon:catalog size-3.5"
- classicon="i-carbon-catalog size-3"
+ classicon="i-carbon:catalog size-3"
- <span class="i-carbon-catalog size-3" aria-hidden="true" />
+ <span class="i-carbon:catalog size-3" aria-hidden="true" />Affected locations: lines 490, 566, 691, 792, 872, 947
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
I love this! I think we should have a page in npmx to show releases, if possible. (the bar at the top: page/code/compare - I think should only link to internal pages within npmx, so maybe let's move it down to the 'repo meta' - stars, forks, etc.) --- or maybe render as a small icon button next to the 'provenance badge' next to the package name? (that way it's clear that it's about that version can't wait to see the ui improvements you make 🙏 |
3 participants
I don't know how common the case is for everyone, but I found that one of the most played actions for me to visit a package's npm page is when I found a new major version released, and I wanted to know what has changed. So my flow would be
As many release notes are hosted in GitHub, I think it would be very useful for npmx to offer an easy entry for accessing the release notes.
A few open questions:
fetchReleasesfor providers other than GitHub, I didn't test them, and actually don't really know how. Should we keep them as-is for future contribution to improve, or should we remove them entirely and add back one by one later?Edit: I just notice it has some conflicts to the existing work #1233 (issue #501), linking for ack