fix: resolve page freeze for packages with many versions #122
Conversation
close #111 Replace Cascader with virtualized Select for version selector to prevent rendering thousands of DOM nodes. Add pagination (50 per page) to versions list and memoize filtering/sorting operations to avoid expensive recomputation on every render. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds client-side pagination and memoized filtering to the versions page and replaces a hierarchical Cascader with a flat Select for version selection; includes comprehensive tests for both components covering rendering, sorting, tag behavior, prereleases, invalid semver, and large-list performance. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant TagFilter as Tag/Filter Logic
participant Pagination as Pagination Component
participant Display as Version Items Display
User->>TagFilter: select tag / change filter
activate TagFilter
TagFilter->>TagFilter: compute filteredVersions (memoized)
TagFilter->>Pagination: reset to page 1
deactivate TagFilter
rect rgb(230,245,255)
Pagination->>Pagination: compute pages (PAGE_SIZE = 50)
Pagination->>Display: provide pageItems (≤50)
end
Display->>Display: render pageItems (publisher, deprecated styling)
Display->>User: show updated list
User->>Pagination: click page N
activate Pagination
Pagination->>Display: update current pageItems
Display->>User: render new page
deactivate Pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical performance bottlenecks encountered when displaying npm packages with an extensive history of versions. By upgrading the version selection UI to a virtualized component and introducing pagination and memoization for version and tag lists, the changes ensure a smoother, more responsive user experience, effectively preventing UI freezes in high-data scenarios. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the page freeze issue for packages with many versions by replacing the Cascader component with a virtualized Select and introducing pagination for the versions list. The use of useMemo to cache expensive filtering and sorting operations is a great performance enhancement. The code is well-structured and the changes are clear. I've identified a high-severity issue with an unstable sorting function and have also included a few suggestions to improve user experience by sorting tag lists, which are currently unordered.
Add tests for NPMVersionSelect component covering: - Rendering with various version lists - Version sorting by semver - Handling of prerelease and invalid versions - Large version list performance (1000+ versions) Add tests for ReadOnlyVersions page covering: - Empty state rendering - Version list with correct sorting - Pagination with 50 versions per page - Tags list display - Deprecated versions styling - Publisher info display - Performance with 1000+ versions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/slugs/versions/index.tsx (2)
54-59: Consider sorting the filtered tags for consistent display order.The tags are filtered but displayed in
Object.keys()order, which depends on insertion order and can be unpredictable. Sorting semantically (e.g., by semver descending) would provide a consistent user experience.This was flagged in a previous review. Consider applying semver sorting:
Suggested fix
const filteredTags = React.useMemo(() => { return Object.keys(tagsInfo || {}).filter( (item) => !onlyProd || !semver.parse(item)?.prerelease.length - ); + ).sort((a, b) => { + const aValid = semver.valid(a); + const bValid = semver.valid(b); + if (aValid && bValid) return semver.rcompare(a, b); + return a.localeCompare(b); + }); }, [tagsInfo, onlyProd]);
120-125: Sort comparator returns inconsistent values for equal timestamps.The current sort
(a, b) => (dayjs(a.publish_time).isAfter(b.publish_time) ? -1 : 1)returns1when timestamps are equal, leading to unstable sort behavior. Subtracting Unix timestamps correctly handles equality.Suggested fix
const filteredVersions = React.useMemo(() => { return versions .filter((item) => !onlyProd || !semver.parse(item.version)?.prerelease.length) - .sort((a, b) => (dayjs(a.publish_time).isAfter(b.publish_time) ? -1 : 1)); + .sort((a, b) => dayjs(b.publish_time).unix() - dayjs(a.publish_time).unix()); }, [versions, onlyProd]);src/components/NPMVersionSelect.tsx (1)
49-52: Tags are not sorted, resulting in unpredictable order.The tag options maintain
Object.keys()order, which depends on insertion order. Sorting alphabetically would improve usability.Suggested fix
const tagOptions = React.useMemo(() => { - return Object.keys(tags || {}).map((t) => ({ label: t, value: tags[t] })); + return Object.keys(tags || {}).sort().map((t) => ({ label: t, value: tags[t] })); }, [tags]);
🧹 Nitpick comments (4)
src/slugs/versions/index.test.tsx (1)
224-243: Performance test provides reasonable coverage but threshold may be environment-dependent.The 1000ms threshold is generous for pagination benefits, but CI environments may have variable performance. Consider documenting that this is a smoke test rather than a strict performance benchmark.
- // Should render in under 1 second (actual should be much faster with pagination) + // Smoke test: should render quickly with pagination (threshold is generous for CI variance) expect(renderTime).toBeLessThan(1000);src/components/NPMVersionSelect.tsx (1)
84-94: Tag Select lacks search filtering.The version Select has
filterOptionfor searching, but the tag Select uses default behavior. For consistency and usability with many tags, consider adding the same filter.Suggested fix
<Select size="small" options={tagOptions} placeholder="选择 Tag" value={hasTag ? targetVersion : undefined} popupMatchSelectWidth={180} onChange={(val) => { setVersionStr(val); }} showSearch + filterOption={(input, option) => + (option?.label ?? '').toLowerCase().includes(input.toLowerCase()) + } />src/components/NPMVersionSelect.test.tsx (2)
69-84: Sorting tests don't verify actual sort order.The test "should sort versions in descending semver order" only checks that the component renders and displays
1.0.0. It doesn't verify that versions appear in the expected order (10.0.0, 2.1.0, 2.0.0, 1.5.0, 1.0.0).Consider expanding the test to open the dropdown and verify option order, or test the
sortVersionsfunction directly.// Option 1: Export and test sortVersions directly import { sortVersions } from './NPMVersionSelect'; it('should sort versions in descending semver order', () => { const sorted = sortVersions(['1.0.0', '2.0.0', '1.5.0', '10.0.0', '2.1.0']); expect(sorted).toEqual(['10.0.0', '2.1.0', '2.0.0', '1.5.0', '1.0.0']); });
154-174: Performance test validates render success but not render time.Unlike the ReadOnlyVersions test which measures
performance.now(), this test only checks that the component renders without crashing. Consider adding timing assertions for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/NPMVersionSelect.test.tsxsrc/components/NPMVersionSelect.tsxsrc/slugs/versions/index.test.tsxsrc/slugs/versions/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/NPMVersionSelect.test.tsx (1)
src/components/NPMVersionSelect.tsx (1)
NPMVersionSelect(38-97)
src/slugs/versions/index.test.tsx (3)
src/hooks/useManifest.ts (2)
NpmPackageVersion(13-30)PackageManifest(32-47)src/slugs/versions/index.tsx (1)
ReadOnlyVersions(221-245)src/components/ReadmeContent.tsx (1)
link(26-32)
src/slugs/versions/index.tsx (3)
src/components/VersionTags.tsx (1)
VersionTags(36-68)src/hooks/useManifest.ts (2)
NpmPackageVersion(13-30)PackageManifest(32-47)src/components/Gravatar.tsx (1)
Gravatar(11-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (9)
src/slugs/versions/index.tsx (2)
112-136: Pagination implementation looks solid.The memoization strategy correctly separates concerns:
filteredVersionshandles filtering and sortingpageItemsslices for the current page- Effect resets page when filter changes
This ensures expensive operations don't re-run on page navigation.
205-216: Pagination UI implementation is correct.The conditional rendering
filteredVersions.length > PAGE_SIZEappropriately hides pagination when unnecessary, and theshowTotalprop provides good UX feedback.src/slugs/versions/index.test.tsx (4)
1-11: MissingbeforeEachimport from vitest.Line 3 imports
describe, it, expect, vibutbeforeEachis used on line 9 without being imported. This may work due to global test configuration, but explicit imports are preferred for clarity.Suggested fix
-import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest';Actually, I see
beforeEachis in the import on line 3. Disregard - the import is correct.
39-70: Good test helper factories with sensible defaults.The
createMockVersionandcreateMockManifesthelpers reduce boilerplate and make tests more readable. The spread ofoptionsallows flexible overrides.
154-194: Comprehensive pagination boundary tests.Good coverage of the 50-version boundary:
- No pagination at exactly 50 versions
- Pagination appears at 60 versions
- Only 50 items rendered per page at 100 versions
245-265: Deprecated version test correctly validates DOM structure.The test properly checks that the version link is wrapped in a
<del>element, which corresponds toTypography.Text delete={!!item.deprecated}.src/components/NPMVersionSelect.tsx (2)
8-23: Robust version sorting with graceful fallbacks.The
sortVersionsfunction handles edge cases well:
- First tries
semver.cleanfor strict parsing- Falls back to
semver.coercefor loose parsing- Returns 0 for completely invalid versions, preserving their relative order
67-83: Good UX improvements for version selection.
virtualandlistHeight={300}enable efficient rendering of large listspopupMatchSelectWidth={false}allows flexible dropdown widthfilterOptionenables case-insensitive searchstyle={{ width: 'auto', minWidth: 100 }}adapts to contentThis addresses the core performance issue from issue #111.
src/components/NPMVersionSelect.test.tsx (1)
112-152: Good coverage of tag scenarios.Tests appropriately cover:
- Tag value displayed when version matches
- Placeholder shown when no match
- Empty tags object handling
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical performance issue where packages with many versions (e.g., 1000+ versions) would freeze the page during rendering. The solution replaces the hierarchical Cascader component with a virtualized Select component and adds pagination to the version list display.
Key changes:
- Replaced Cascader with virtualized Select in NPMVersionSelect component to efficiently handle large version lists
- Added pagination (50 versions per page) to VersionsList to limit DOM node rendering
- Implemented memoization for filtering and sorting operations to prevent unnecessary recomputations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/components/NPMVersionSelect.tsx |
Replaced Cascader with virtualized Select component, simplified version sorting to flat list structure, and added virtual scrolling support |
src/components/NPMVersionSelect.test.tsx |
Added comprehensive test coverage including edge cases for large version lists, sorting, tags, and rendering scenarios |
src/slugs/versions/index.tsx |
Added pagination to VersionsList with page reset on filter changes, and memoized filtered/sorted data for both TagsList and VersionsList |
src/slugs/versions/index.test.tsx |
Added comprehensive test suite covering pagination behavior, sorting, large datasets, deprecated versions, and user info display |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'use client'; | ||
| import { isEmpty } from 'lodash'; | ||
| import { Cascader, Select, Space } from 'antd'; | ||
| import { Select } from 'antd'; |
There was a problem hiding this comment.
Unused import Space.
| } from '@/hooks/useManifest'; | ||
|
|
||
| import { Card, Result, Segmented, Space, Tag, Tooltip, Typography } from 'antd'; | ||
| import { Card, Pagination, Result, Segmented, Space, Tooltip, Typography } from 'antd'; |
There was a problem hiding this comment.
Unused import Tag.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/components/NPMVersionSelect.tsx (1)
49-52: Sort tags alphabetically for better usability.The tags are currently displayed in insertion order, which can make it difficult for users to locate a specific tag. Sorting them alphabetically would improve the user experience.
🔎 Suggested fix
const tagOptions = React.useMemo(() => { - return Object.keys(tags || {}).map((t) => ({ label: t, value: tags[t] })); + return Object.keys(tags || {}).sort().map((t) => ({ label: t, value: tags[t] })); }, [tags]);src/slugs/versions/index.tsx (2)
54-59: Sort tags semantically for predictable display order.The current implementation displays tags in the order returned by
Object.keys(), which is unpredictable. Sorting tags usingsemver.rcomparewould provide a consistent, descending semantic version order that improves user experience.🔎 Suggested fix
const filteredTags = React.useMemo(() => { - return Object.keys(tagsInfo || {}).filter( - (item) => !onlyProd || !semver.parse(item)?.prerelease.length - ); + return Object.keys(tagsInfo || {}) + .filter((item) => !onlyProd || !semver.parse(item)?.prerelease.length) + .sort(semver.rcompare); }, [tagsInfo, onlyProd]);
120-125: Fix unstable sort for equal timestamps.The current sort function
(a, b) => (dayjs(a.publish_time).isAfter(b.publish_time) ? -1 : 1)returns1when timestamps are equal, making the sort unstable. Use timestamp subtraction for a stable, reliable sort.🔎 Suggested fix
const filteredVersions = React.useMemo(() => { return versions .filter((item) => !onlyProd || !semver.parse(item.version)?.prerelease.length) - .sort((a, b) => (dayjs(a.publish_time).isAfter(b.publish_time) ? -1 : 1)); + .sort((a, b) => dayjs(b.publish_time).unix() - dayjs(a.publish_time).unix()); }, [versions, onlyProd]);
🧹 Nitpick comments (1)
src/components/NPMVersionSelect.tsx (1)
8-23: Add lexicographic fallback for unparseable versions.When both
semver.cleanandsemver.coercefail to parse a version string, returning0leaves the sort order undefined. For deterministic and predictable ordering, fall back to lexicographic string comparison.🔎 Suggested fix
function sortVersions(versions: string[]): string[] { return [...versions].sort((a, b) => { const aClean = semver.clean(a); const bClean = semver.clean(b); if (aClean && bClean) { return semver.rcompare(aClean, bClean); } const aVersion = semver.coerce(a); const bVersion = semver.coerce(b); if (aVersion && bVersion) { return semver.rcompare(aVersion, bVersion); } - return 0; + // Fallback to lexicographic comparison for unparseable versions + return b.localeCompare(a); }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/NPMVersionSelect.test.tsxsrc/components/NPMVersionSelect.tsxsrc/slugs/versions/index.test.tsxsrc/slugs/versions/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/NPMVersionSelect.test.tsx
- src/slugs/versions/index.test.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/slugs/versions/index.tsx (3)
src/components/VersionTags.tsx (1)
VersionTags(36-68)src/hooks/useManifest.ts (2)
NpmPackageVersion(13-30)PackageManifest(32-47)src/components/Gravatar.tsx (1)
Gravatar(11-34)
🔇 Additional comments (4)
src/components/NPMVersionSelect.tsx (2)
25-29: LGTM!The function correctly builds flat options for the Select component, enabling virtual scrolling to handle large version lists efficiently.
67-83: Excellent use of virtual scrolling to prevent page freeze.The combination of
virtual,listHeight, andpopupMatchSelectWidthsettings effectively addresses the issue of rendering thousands of versions by only rendering visible items in the viewport. The customfilterOptionprovides a good user experience for searching through large version lists.src/slugs/versions/index.tsx (2)
127-136: Well-implemented pagination logic.The page reset when filters change provides good UX, and the memoized
pageItemscomputation efficiently slices the filtered versions for the current page. The pagination correctly limits rendering to 50 items at a time, directly addressing the page freeze issue.
205-216: LGTM! Pagination controls properly configured.The pagination is conditionally rendered, correctly wired to state, and provides clear feedback with the total count. This implementation effectively limits the number of rendered versions, preventing the page freeze issue described in issue #111.
close #111
Replace Cascader with virtualized Select for version selector to prevent rendering thousands of DOM nodes. Add pagination (50 per page) to versions list and memoize filtering/sorting operations to avoid expensive recomputation on every render.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.