Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/composables/npm/useSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ export function useSearch(
const doSearch = provider === 'algolia' ? searchAlgolia : searchNpm
const response = await doSearch(q, { size, from })

const beforeCount = cache.value?.objects.length ?? 0

if (cache.value && cache.value.query === q && cache.value.provider === provider) {
const existingNames = new Set(cache.value.objects.map(obj => obj.package.name))
const newObjects = response.objects.filter(obj => !existingNames.has(obj.package.name))
Expand All @@ -279,6 +281,12 @@ export function useSearch(
}
}

// Bail if the provider gave us no new unique items
// Without something like this the recursion below never terminates.
if ((cache.value?.objects.length ?? 0) === beforeCount) {
return
}
Comment on lines +284 to +288
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate the no-growth state to hasMore.

The new bail stops the recursive call, but hasMore still stays true when objects.length < total after an Algolia cap response. That leaves callers free to immediately request more and repeat the same empty fetch. Track a local “exhausted/capped” state, reset it on query/provider refreshes, and include it in hasMore.

Suggested shape
   const isLoadingMore = shallowRef(false)
   const isRateLimited = shallowRef(false)
+  const isFetchExhausted = shallowRef(false)

 ...
       cache.value = null
+      isFetchExhausted.value = false

 ...
       if ((cache.value?.objects.length ?? 0) === beforeCount) {
+        isFetchExhausted.value = true
         return
       }
+      isFetchExhausted.value = false

 ...
   const hasMore = computed(() => {
     if (!cache.value) return true
+    if (isFetchExhausted.value) return false
     return cache.value.objects.length < cache.value.total
   })

Also applies to: 337-340

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/composables/npm/useSearch.ts` around lines 284 - 288, When the bail
condition (if ((cache.value?.objects.length ?? 0) === beforeCount) return)
triggers we must mark a local "exhausted" (or "capped") flag so callers see no
more results; set exhausted = true at that bail site and include exhausted in
the computed hasMore so hasMore = !exhausted && (cache.value.objects.length <
total) (or similar). Also reset exhausted to false whenever the query or
provider changes (the places that refresh the search, e.g., the functions that
update query/filters or re-run the provider) so a new search can retry, and
apply the same change to the other identical bail at lines 337-340 (same
cache/beforeCount check). Ensure you reference and update the local exhausted
variable wherever hasMore is computed and where searches are reinitialized.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the bot is pointing out is not introduced by this PR, the hasMore already is always true for this case of search results w/ more than 1k entries. I'll add it to the caveats though, it should be handled in a followup IMO.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonchurch, that's a fair point — hasMore staying true past Algolia's pagination cap is indeed pre-existing behavior independent of this PR's changes. The bail added here prevents the freeze; the stale hasMore state is a separate UX concern. Tracking it as a follow-up makes sense. Thanks for the context!


🧠 Learnings used
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1358
File: app/composables/useSettings.ts:118-131
Timestamp: 2026-02-11T00:34:43.263Z
Learning: In app/composables/useSettings.ts, the search provider cookie getter intentionally checks only for 'npm' explicitly and falls back to settings for all other cases, because 'algolia' is the default search provider. This treats the cookie as an override flag specifically for the non-default option.

Learnt from: Kiwow
Repo: npmx-dev/npmx.dev PR: 2506
File: app/pages/compare.vue:0-0
Timestamp: 2026-04-13T22:26:42.895Z
Learning: In `app/composables/usePackageComparison.ts`, `packagesData` is defined as `computed(() => packages.value.map(name => cache.value.get(name) ?? null))`, which means it is **always** the same length as `packages.value`. Unloaded/missing packages yield `null` entries rather than a shorter array. Therefore, checking `packagesData.value.length >= 1 && packagesData.value.every(data => data !== null)` is sufficient to confirm that all selected packages have loaded. Do not suggest adding a `packagesData.value.length === packages.value.length` guard, as this invariant is structurally guaranteed.

Learnt from: ShroXd
Repo: npmx-dev/npmx.dev PR: 2115
File: app/pages/search.vue:365-368
Timestamp: 2026-03-17T06:53:49.002Z
Learning: In npmx-dev/npmx.dev (app/pages/search.vue), the `showClaimPrompt` computed intentionally returns `true` during a `status === 'pending'` fetch even when `avail.name` doesn't match `committedQuery.value.trim()`. This is by design to prevent the claim prompt from flickering/disappearing while the user is typing (avoiding layout shifts). The claim button is separately guarded with `:disabled="status === 'pending'"` to prevent accidental clicks on stale data. Do not suggest adding an `avail.name === committedQuery` guard to the pending branch.

Learnt from: danielroe
Repo: npmx-dev/npmx.dev PR: 1204
File: app/composables/npm/useUserPackages.ts:40-41
Timestamp: 2026-02-08T13:24:06.104Z
Learning: In Nuxt 3, when using useAsyncData or useLazyAsyncData, allow and rely on reactive keys by typing them as MaybeRefOrGetter<string>. Keys can be a string, a Ref<string>, a ComputedRef<string>, or a getter () => string. Reactive keys should automatically trigger refetches when their value changes. In reviews, verify that data scripts pass a MaybeRefOrGetter<string> (not a plain string-only key if reactivity is intended), and that the logic handles dynamic key changes without stale data.

Learnt from: MatteoGabriele
Repo: npmx-dev/npmx.dev PR: 1922
File: app/composables/usePackageListPreferences.ts:52-53
Timestamp: 2026-03-05T10:14:50.799Z
Learning: In Nuxt projects (here npmx-dev/npmx.dev), exports from shared/types/* (e.g., PAGE_SIZE_OPTIONS, DEFAULT_COLUMNS, DEFAULT_PREFERENCES, PageSize) are auto-imported by Nuxt for composables and components. Do not add explicit import statements for these constants/types when using files under shared/types/, and rely on the auto-imported bindings in files under app/composables (and similarly in components). This pattern applies to all TS files within app/composables that reference these shared/types exports.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create/update an issue for the follow up so we don't forget?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


if (
cache.value &&
cache.value.objects.length < targetSize &&
Expand Down
Loading