Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
dd33ecc to
2873b63
Compare
2873b63 to
5cc0842
Compare
5cc0842 to
5fd65b8
Compare
5fd65b8 to
35bdb74
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
35bdb74 to
03ed3c3
Compare
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
|
03ed3c3 to
bba4380
Compare
📝 WalkthroughWalkthroughIntroduces a new package versions page component implementing grouped, virtualised version history with two-phase data loading, per-group lazy metadata fetch, debounced filtering (including semver ranges) and SSR-friendly initial headers. The main package page now detects the versions route and gates template rendering while data loads. The Versions component header adds a "View all versions" link next to the existing distribution action. i18n keys and schema were extended for the new UI strings. Tests for the versions page and component were added/updated. Possibly related PRs
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 TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/pages/package/[[org]]/[name]/versions.vue (1)
377-387: Consider adding safety check for array access.
item.versions[0]is accessed without a guard. While the grouping logic ensures each group has at least one version, adding a safety check would satisfy strict type-safety guidelines and prevent potential runtime issues if the logic changes.🔧 Suggested fix
<span class="ms-auto flex items-center gap-3 shrink-0"> - <span class="text-xs text-fg-muted" dir="ltr">{{ item.versions[0] }}</span> + <span v-if="item.versions[0]" class="text-xs text-fg-muted" dir="ltr">{{ item.versions[0] }}</span> <DateTime - v-if="getVersionTime(item.versions[0])" - :datetime="getVersionTime(item.versions[0])!" + v-if="item.versions[0] && getVersionTime(item.versions[0])" + :datetime="getVersionTime(item.versions[0])" class="text-xs text-fg-subtle hidden sm:block"As per coding guidelines: "ensure you always check when accessing an array value by index".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9477326f-b47e-43c8-81a2-07b1d0698711
📒 Files selected for processing (7)
app/components/Package/Versions.vueapp/pages/package/[[org]]/[name].vueapp/pages/package/[[org]]/[name]/versions.vuei18n/locales/en.jsoni18n/schema.jsontest/nuxt/components/PackageVersions.spec.tstest/nuxt/pages/PackageVersionsPage.spec.ts
| expandedGroups.has(item.groupKey) | ||
| ? $t('package.versions.collapse', { tag: item.label }) | ||
| : $t('package.versions.expand', { tag: item.label }) |
There was a problem hiding this comment.
Don’t worry about updating these labels. Using aria-expanded is already sufficient for AT users to know what state the button is in and consequently what state it will change to if they activate it. Doing both can be confusing (e.g. screen reader might read Collapse 4.x versions, button expanded or Expand 4.x versions, button collapsed).
This goes for anywhere else in the PR that includes the words “expand” or “collapse” inside of the aria-label.
There was a problem hiding this comment.
Thanks for pointing it out!
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/pages/package/[[org]]/[name]/versions.vue (2)
175-176: TODO: Changelog side panel.This TODO tracks a future enhancement for showing GitHub release notes or parsed CHANGELOG.md content per version.
Would you like me to open a GitHub issue to track this feature, or generate a skeleton implementation for the changelog side panel?
373-381: Minor inconsistency in defensive array access.The main template accesses
item.versions[0]directly (line 373), whilst the SSR fallback usesitem.versions[0] ?? ''defensively (line 479). Although groups are guaranteed to have at least one version by construction, consider aligning the defensive coding style for consistency:Suggested alignment
<span class="ms-auto flex items-center gap-3 shrink-0"> - <span class="text-xs text-fg-muted" dir="ltr">{{ item.versions[0] }}</span> + <span class="text-xs text-fg-muted" dir="ltr">{{ item.versions[0] ?? '' }}</span> <DateTime - v-if="getVersionTime(item.versions[0])" - :datetime="getVersionTime(item.versions[0])!" + v-if="item.versions[0] && getVersionTime(item.versions[0])" + :datetime="getVersionTime(item.versions[0])!"Also applies to: 477-480
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c74b64d5-2ea0-41a6-b9c3-e9963f9f4458
📒 Files selected for processing (2)
app/pages/package/[[org]]/[name].vueapp/pages/package/[[org]]/[name]/versions.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- app/pages/package/[[org]]/[name].vue
| .findAll('a') | ||
| .filter(a => !a.attributes('href')?.startsWith('#') && a.attributes('target') !== '_blank') | ||
| .filter( | ||
| a => | ||
| !a.attributes('href')?.startsWith('#') && | ||
| a.attributes('target') !== '_blank' && | ||
| !a.attributes('data-testid')?.includes('view-all-versions'), | ||
| ) |
There was a problem hiding this comment.
This whole findAll + filter is repeated throughout the test, and might be dried out in a function
🔗 Linked issue
Resolves #1846
🧭 Context
Like the original issues mentioned, it would be useful to have a full version history page with filtering functionality.
📚 Description
Since it's common for a package to have hundreds of versions, some package like typescript even have 3,700 versions. To provide good performance and user experience, I did some optimization and trade off during developing.
1. Two phase data fetching
Phase 1 (on page load) calls
getVersionsto fetch only a lightweight summary — version strings, dist-tags, and publish times. This is enough to immediately render the "Current Tags" section and all group headers without waiting for full metadata.Phase 2 (lazy, on first group expand) calls
fetchAllPackageVersionsto retrieve full metadata (deprecated status, provenance). The result is cached infullVersionMapas ashallowRef— once loaded it's reused across all subsequent group expansions, never re-fetched.The tradeoff: a short one-time loading delay on first expand, in exchange for a significantly faster initial page load.
2. Virtual scrolling + SSR fallback
Since
WindowVirtualizeris client-only, it's wrapped in<ClientOnly>, with a#fallbackslot providing a static SSR substitute — just the first 20 group headers, no interactivity.The SSR fallback iterates
versionGroupsdirectly, a flat list of group objects, each containing an array of versionsThe client-side
virtualizerconsumesflatItems— a flattened array that interleaves group headers and their expanded version rows into a single indexed list.This follows existing pattern in package list page.
3. Debounced version filter
Since the filtering work is done entirely on the frontend, it can put significant performance pressure on packages with a large number of versions, such as typescript. And to support virtual scrolling, version groups are flattened into a
flatItemsarray — a single indexed list of group headers and version rows thatWindowVirtualizercan render efficiently. Therefore, a debounce on the filter input is necessary to avoid triggering expensive recomputation on every keystroke.Some further optimization options include chunked
flatItemsconstruction withrequestIdleCallbackand moving the filtering work into a Web Worker. However, both require non-trivial design and implementation to handle potential race conditions. We also plan to add changelog support to this page, which will introduce additional matching overhead. For now, the current implementation is "good enough" — once the full feature set is complete, we can do thorough performance profiling to find the best optimization approach.4. Disable prefetching for package page of each version
Per-version links in the history list have prefetching disabled, as expanding a group could generate a lot of simultaneous prefetch requests and unnecessarily strain the network and significantly impact page performance. Although this makes package page navigation slightly slower, considering most user behavior on this page is browsing and filtering, the tradeoff is acceptable.
5. Some interesting cases
📸 Screenshots