feat: progressive loading for org packages#1953
feat: progressive loading for org packages#1953Adebesin-Cell wants to merge 20 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 batched Algolia lookups and a new slice API, implements progressive/org-aware package loading with staleness guards and in‑flight deduplication (exposing loadAll), updates visible-items to support async expand hooks, adapts the org page to progressive loading, and adds Vite+ skill references and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrgPage as "Org Page (Client UI)"
participant useOrg as "useOrgPackages"
participant Algolia
participant Server as "Server (package-meta)"
Client->>OrgPage: navigate to org page (orgName)
OrgPage->>useOrg: useOrgPackages(orgName)
Note over useOrg: capture org, fetch initial slice via getPackagesByNameSlice
useOrg->>Algolia: getPackagesByNameSlice(batch 1)
Algolia-->>useOrg: slice results
useOrg-->>OrgPage: return initial asyncData (initialObjects, totalPackages)
OrgPage->>useVisible: create visible list (onExpand -> useOrg.loadAll)
alt user expands / onExpand triggered
OrgPage->>useOrg: loadAll()
Note over useOrg: partition remainingNames into ALGOLIA_BATCH_SIZE slices
par concurrent slice fetches
useOrg->>Algolia: getPackagesByNameSlice(batch N)
Algolia-->>useOrg: results
and
useOrg->>Algolia: getPackagesByNameSlice(batch M)
Algolia-->>useOrg: results
end
useOrg->>useOrg: dedupe & append loadedObjects
useOrg->>useOrg: re-check org staleness
alt some metadata missing
useOrg->>Server: fetch package-meta fallback for remaining names
Server-->>useOrg: fallback results
end
useOrg-->>OrgPage: update asyncData with appended objects
end
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3cf16238-f272-4a67-8b0c-bc06ec8bf13a
📒 Files selected for processing (3)
app/composables/npm/useAlgoliaSearch.tsapp/composables/npm/useOrgPackages.tsapp/pages/org/[org].vue
Instead of capping at 1000 packages and blocking on all metadata, load the first 250 immediately and fetch the rest on demand. Filtering/sorting triggers loadAll() so client-side search still works on the full set.
Adebesin-Cell
changed the title
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
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/composables/npm/useOrgPackages.ts (1)
249-256: Consider cancelling in-flight operations when the provider changes.The watch resets the cache and promise lock but doesn't cancel any in-flight
loadAll()operations. If a user toggles the search provider while loading is in progress, the old operation may complete and update the cache after the reset, causing inconsistent state.The existing staleness guard on
orgName(line 196) doesn't protect against provider changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eeb7210c-a4af-4e08-ad83-232fa11b004d
📒 Files selected for processing (5)
app/composables/npm/useAlgoliaSearch.tsapp/composables/npm/useOrgPackages.tsapp/pages/org/[org].vuei18n/locales/en.jsoni18n/schema.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/composables/npm/useAlgoliaSearch.ts (1)
217-242: Consider adding a defensive guard for the documented 1000-item limit.The JSDoc states "max 1000" but the function doesn't enforce this constraint. While callers (e.g.,
useOrgPackages.ts) correctly batch inputs to 1000 items before calling, a defensive check would make the contract self-enforcing and catch accidental misuse.🛡️ Optional defensive check
/** Fetch metadata for a single batch of packages (max 1000) by exact name. */ async function getPackagesByNameSlice(names: string[]): Promise<NpmSearchResult[]> { if (names.length === 0) return [] + if (names.length > 1000) { + throw new Error(`getPackagesByNameSlice: batch size ${names.length} exceeds Algolia limit of 1000`) + } const response = await $fetch<{ results: (AlgoliaHit | null)[] }>(
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc7cae55-8a67-432f-a21d-e8abd23f16d9
📒 Files selected for processing (3)
app/composables/npm/useAlgoliaSearch.tsi18n/locales/en.jsoni18n/schema.json
✅ Files skipped from review due to trivial changes (2)
- i18n/locales/en.json
- i18n/schema.json
ghostdevv
removed
the
needs review
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 509b1c68-959d-48d1-aafe-959b9c10edfb
📒 Files selected for processing (4)
app/composables/npm/useOrgPackages.tsapp/pages/org/[org].vuei18n/locales/en.jsoni18n/schema.json
✅ Files skipped from review due to trivial changes (1)
- i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- i18n/schema.json
- Extend useVisibleItems with onExpand callback + partial load support - useOrgPackages: fetch first 50 on SSR, loadMore(1000) per page, loadAll() only when filters need the full dataset - Track remaining by name (not position) for retry safety - Pagination shows real total (11,397) while loading incrementally Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| import { mapWithConcurrency } from '#shared/utils/async' | ||
|
|
||
| /** Number of packages to fetch metadata for in the initial load */ | ||
| const INITIAL_BATCH_SIZE = 50 |
There was a problem hiding this comment.
Why only 50? This is a very fast query. Are you limiting the initial data fetch to get a neat page of 50 packages displayed in the UI? That's convenient for the UI but inefficient for the network call budget.
There was a problem hiding this comment.
Ran some benchmarks against the Algolia getObjects endpoint for @types (11,397 packages):
| Initial batch size | Algolia response time (avg of 3 runs) |
|---|---|
| 50 | ~680ms |
| 100 | ~1,030ms |
| 150 | ~1,120ms |
| 200 | ~1,065ms |
| 250 | ~1,170ms |
| 500 | ~1,470ms |
| 1,000 | ~2,000ms |
This is a single Algolia getObjects POST during SSR. It blocks the HTML response. The registry fetch for the package names list is ~20ms (cached), so Algolia is the bottleneck.
50 to 100 adds ~350ms but doubles the initial content. 100 to 250 adds ~140ms more, but the response is already over a second. The default page size is 25, so 100 gives users 4 pages of content on first paint without needing a second fetch.
I can bump it to 100. It is a good balance between content density and SSR speed. The remaining packages load incrementally (1,000 per batch) as the user scrolls or paginates, or all at once when they filter or sort.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
That’s weird. I did test locally, and loading more consistently triggers when you scroll to the end. If you switch to table view, you can see it paginates correctly with the total package count showing (screenshot attached). 
I’ll look into why the infinite scroll load-more isn’t triggering on the preview deployment and fix it. |
shallowRef state doesn't transfer from SSR to client, so loadMore() had no package names to work with after hydration. Fix by including allPackageNames in the async data response (transferred via Nuxt payload) and reading from it instead of a separate shallowRef. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
It doesnt make much sense to split this view into pages this way while this view is a sorted list of unsorted data. That is, we get a list of all packages, and then the view we present is always sorted. Yet we are not currently sorting the data in the query. So what good is showing me the top downloaded package from a random subset of 50 packages out of 100, 500, 1k+ or whatever? I think the solution here is to query all data before rendering the page. That will still be significantly faster than the existing behavior, and is a much simpler fix. My feedback to you here is to first solve the problem much more simply. It is an extremely low bar to improve the perf here, and the only reason the perf is impacted is because it is hitting the much slower fallback codepath due to the 400 that is hit on the fast path, see #2507 The actual needed fix is batching the network calls to get around the 1k package limit, not turning this view into a paged view. |
There was a problem hiding this comment.
The issue with not fetching the next "page" on scrolling to the bottom is fixed 👍
But if you test it now you'll see what I mean in #1953 (comment)
Lets say I want to see the top downloaded package of sindresorhus org. On first load I will just see whatever comes from the first 50 results. I'll think "hmm, i didnt expect that package to be first? oh well" because as a user I am seeing a sorted list but its unclear to me that its only a subset of 50. Sure it says its showing 50 of 1600, but surely its showing me the top 50 based on my sort choice? (it is not)
But imagine I'm a very Smart user, and I know exactly how the implementatino of this sorted view works! I will simply scroll to the bottom of the list of 50! Currently, it fetches the next 1k, re-sorts the list right? I scroll back up to see what the new top one is. Now I know what the top downloaded package of the first 1050 packages are. Now I want to see the list with all of them sorted! So I scroll down, through 1050 items, to trigger it to fetch the next 1k... I rinse and repeat for however many thousand I need to get through.
My point is that the way this page is presented to the user as pre-sorted makes dynamic pagination here feel really bad as UX
I did approach the issue with batching at first. My commit fa6c5908 added Algolia batching to handle the 1k getObjects limit (the root cause of the 400 from #2507), though it was sequential and capped at 1000 packages. The 6e49401a removed the cap and made batches parallel, which is the actual performance fix. No more falling into the slow npm fallback path that was causing the hang. The progressive loading layer (first 50, load more on scroll, load all on filter) came later based on #1953 (review), suggesting we adopt the useVisibleItems composable from #2395. I agree that sorting on a partial dataset is misleading and that the batched Algolia path is fast enough to load everything upfront. I am happy to simplify back to "batch-fetch everything, let PackageList handle display." But since the progressive approach came from review feedback, it would be good to get @ghostdevv's take before I strip it out. |
|
ok i've mostly skimmed the diff so let me know if i misunderstood anything. i think @ghostdevv was suggesting However, going back to the root problem, i'm not sure we need all of this. As far as I understand, the root problem is that we send too many packages to Algolia in one request so we get an error back. The solution to that is to batch the requests, ideally with a concurrency limit (i.e. so we don't just If we do that server-side, the UI receives the full batch as far as i understand and renders it in its entirety (no paging). unrelated to the Algolia limit, it may be nicer UX to page those client-side one day. But that feels like something which needs a discussion first. So my two suggested actions are:
|
|
@43081j @jonchurch Thanks for the clear breakdown. That makes sense — Willow's suggestion was the right call given the PR had paging at the time, but the paging itself was solving a symptom rather than the root cause. I'll simplify this PR down to just the Algolia batching fix (with concurrency limit) and leave the UI as-is. I've opened #2529 to discuss client-side paging for org pages separately as James suggested. |
The root cause of npmx-dev#1946 and npmx-dev#2507 is that Algolia's getObjects API has a 1000-item limit. Sending >1000 names returns a 400, which triggers the slow npm fallback path (1 request per package). Fix: batch getPackagesByName into chunks of 1000 with a concurrency limit of 3. The org page and useOrgPackages stay unchanged — all packages are fetched upfront and sorted client-side. Client-side paging is tracked separately in npmx-dev#2529. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use nested for...of instead of push(...results.flat()) to avoid exceeding max function arguments for large orgs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 participants
Summary
Fixes #1946 and #2507. Org pages were hanging for large orgs like
@types(11,000+ packages) and@sindresorhus(1,690 packages).Root cause
Algolia's
getObjectsAPI has a 1,000-item limit per request. The existing code sent all package names in a single request, which returned a 400 for orgs with >1,000 packages. This triggered the npm fallback path — one API call per package — causing the page to hang.Fix
Batch
getPackagesByNameinto chunks of 1,000 with a concurrency limit of 3. All packages are fetched upfront so sorting and filtering operate on the complete dataset.What changed
getPackagesByNameSliceas an internal helper for single-batch AlgoliagetObjectscalls (max 1,000)getPackagesByNamenow batches automatically with a concurrency limit of 3for...ofinstead of spread to avoid exceeding max function arguments for large result setsuseOrgPackagesfor org name changes during fetchPackageListhandles display as beforeContext
This PR originally included progressive loading and
useVisibleItemsintegration. After discussion in the PR, we simplified to just the batching fix — sorting a partial dataset is misleading UX, and the batched Algolia path is fast enough to load everything upfront. Client-side paging is tracked separately in #2529.