fix: fix tag issue and sort tagged versions#2105
fix: fix tag issue and sort tagged versions#2105ShroXd wants to merge 4 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a dist-tag priority system and applies it to the package versions page. Introduces Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils/versions.ts (1)
77-83: Fuzzy matching may cause unexpected priority assignment for unconventional tag names.The
String.includes()check matches substrings anywhere in the tag, so tags like"arc"would match"rc"(priority 1) and"betamax"would match"beta"(priority 2). Additionally, since iteration stops on first match, the order of entries inTAG_PRIORITYdetermines which keyword wins for tags containing multiple keywords (e.g.,"beta-rc"matches"rc"first).This is likely acceptable for real-world npm dist-tags, but consider word-boundary matching if edge cases arise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0522bfda-26e4-4b24-bcd8-83f8d51327a6
📒 Files selected for processing (2)
app/pages/package/[[org]]/[name]/versions.vueapp/utils/versions.ts
| /** | ||
| * Priority order for well-known dist-tags. | ||
| * Lower number = higher priority in display order. | ||
| * Unknown tags fall back to Infinity and are sorted by publish date descending. |
There was a problem hiding this comment.
I was reading through the code and wondering how it's possible that "latest" is not a well-known tag. It might be worth having a comment here explaining that it's handled manually elsewhere :)
There was a problem hiding this comment.
I was reading through the code and wondering how it's possible that "latest" is not a well-known tag. It might be worth having a comment here explaining that it's handled manually elsewhere :)
Nice point, thanks for bringing it up! I reconsidered TAG_PRIORITY — it's definitely weird to exclude latest from it. Although we filter it out in the UI, it doesn't make sense for a general constant to be affected by specific UI logic.
So I've added latest: 0 and shifted the other priorities by 1.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/pages/package/[[org]]/[name]/versions.vue (1)
70-73: Normalise the latest-tag predicate once.
getTagPriority()already lowercases tags, but the featured-row split still relies on exact'latest'checks. A tiny helper would keep the card selection, exclusion filter, and label rendering aligned if mixed-case tags ever leak through.♻️ Proposed tidy-up
const tagRows = computed(() => buildTaggedVersionRows(distTags.value)) -const latestTagRow = computed(() => tagRows.value.find(r => r.tags.includes('latest')) ?? null) +const isLatestTag = (tag: string) => tag.toLowerCase() === 'latest' + +const latestTagRow = computed(() => tagRows.value.find(r => r.tags.some(isLatestTag)) ?? null) const otherTagRows = computed(() => tagRows.value - .filter(r => !r.tags.includes('latest')) + .filter(r => !r.tags.some(isLatestTag)) .sort((a, b) => { const pa = getTagPriority(a.primaryTag) const pb = getTagPriority(b.primaryTag) if (pa !== pb) return pa - pb const ta = versionTimes.value[a.version] ?? '' const tb = versionTimes.value[b.version] ?? '' return tb.localeCompare(ta) }), )- v-for="tag in latestTagRow!.tags.filter(t => t !== 'latest')" + v-for="tag in latestTagRow!.tags.filter(t => !isLatestTag(t))"Also applies to: 241-241
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 940f3047-49fc-4769-acec-3d22c747c9c2
📒 Files selected for processing (2)
app/pages/package/[[org]]/[name]/versions.vueapp/utils/versions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/utils/versions.ts
| const pa = getTagPriority(a.primaryTag) | ||
| const pb = getTagPriority(b.primaryTag) | ||
| if (pa !== pb) return pa - pb | ||
| const ta = versionTimes.value[a.version] ?? '' | ||
| const tb = versionTimes.value[b.version] ?? '' | ||
| return tb.localeCompare(ta) |
There was a problem hiding this comment.
nit: I would recommend to use readable vars
There was a problem hiding this comment.
nit: I would recommend to use readable vars
Nice point! I will refactor this.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 023d94fd-4ca9-498b-aa2a-3299cf80450b
📒 Files selected for processing (1)
app/pages/package/[[org]]/[name]/versions.vue
ShroXd
force-pushed
the
fix-version-history-latest-tag
branch
from
50473b7 to
3113448
Compare
ShroXd
force-pushed
the
fix-version-history-latest-tag
branch
from
3113448 to
9da287c
Compare
3 participants
🔗 Linked issue
Resolves #2102
🧭 Context
A implementation bug caused the confusing UI.
📚 Description
Fix unstable ordering of the tagged versions array. Tags are now sorted by semantic priority, with the exception of latest which remains pinned at the top.