feat: support algolia for package search + org/user package listing #1204
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
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
|
📝 WalkthroughWalkthroughThis change introduces Algolia as an alternative search provider for npm package discovery alongside the existing npm registry implementation. A new SearchProvider setting allows users to toggle between 'npm' and 'algolia' sources via a UI toggle component. The Algolia integration includes a dedicated search composable (useAlgoliaSearch), runtime configuration, and provider-aware logic throughout the search flow. Existing composables have been extended to support dual-path queries with fallback mechanisms. New user packages and org package composables leverage the provider setting to determine fetch strategies. Supporting translations and UI elements have been added to expose the data source selection to users. Possibly related PRs
🚥 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)
No actionable comments were generated in the recent review. 🎉 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/composables/npm/useNpmSearch.ts (1)
173-255:⚠️ Potential issue | 🟠 MajorAvoid stale
fetchMorewrites after provider/query switches.If the provider or query changes mid-request, the in-flight response can still overwrite the cache. Add a guard after each await before updating state.
🛡️ Suggested guard
if (provider === 'algolia') { // Algolia incremental fetch const response = await searchAlgolia(q, { size, from }) + if (q !== toValue(query).trim() || provider !== searchProvider.value) { + return + } 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)) cache.value = { @@ } else { // npm registry incremental fetch const params = new URLSearchParams() params.set('text', q) params.set('size', String(size)) params.set('from', String(from)) const { data: response } = await $npmRegistry<NpmSearchResponse>( `/-/v1/search?${params.toString()}`, {}, 60, ) + if (q !== toValue(query).trim() || provider !== searchProvider.value) { + return + } 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)) cache.value = {
🧹 Nitpick comments (5)
app/pages/settings.vue (1)
165-180: Remove inlinefocus-visibleutility class from select element.The
focus-visible:outline-accent/70class on line 168 should be removed. The project applies focus-visible styling for buttons and selects globally viamain.css. Rely on the global rule for consistency.Note: The same issue exists on the language-select at line 231, which may have been introduced in a prior change.
♻️ Proposed fix
<select id="search-provider-select" :value="settings.searchProvider" - class="w-full sm:w-auto min-w-48 bg-bg border border-border rounded-md px-3 py-2 text-sm text-fg focus-visible:outline-accent/70 cursor-pointer duration-200 transition-colors hover:border-fg-subtle" + class="w-full sm:w-auto min-w-48 bg-bg border border-border rounded-md px-3 py-2 text-sm text-fg cursor-pointer duration-200 transition-colors hover:border-fg-subtle"Based on learnings: "In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via main.css… individual buttons or selects in Vue components should not rely on inline focus-visible utility classes."
app/composables/npm/useOrgPackages.ts (1)
98-105: Consider adding observability for Algolia failures.The silent fallback is good for user experience, but failures may go unnoticed during debugging or monitoring. Consider adding a
console.warnor structured log when Algolia fails and the fallback path is taken.🔧 Optional: Add warning log
if (searchProvider.value === 'algolia') { try { return await searchAlgoliaByOwner(org) - } catch { + } catch (err) { + console.warn(`[useOrgPackages] Algolia search failed for org "${org}", falling back to npm registry`, err) // Fall through to npm registry path on Algolia failure } }app/composables/npm/useUserPackages.ts (1)
25-205:useUserPackagesis getting large—consider splitting into helpers.Extracting provider-specific fetchers and cache updates would keep the composable easier to follow and test.
As per coding guidelines: Keep functions focused and manageable (generally under 50 lines).
app/composables/npm/useNpmSearch.ts (1)
55-58:useNpmSearchexceeds the recommended function size.Consider extracting provider-specific fetchers and cache merge helpers to keep the composable manageable.
As per coding guidelines: Keep functions focused and manageable (generally under 50 lines).
app/pages/~[username]/index.vue (1)
35-41: GuardloadAll()to avoid repeated full fetches while typingThe watcher fires on every filter/sort change, so
loadAll()can run per keystroke unless it’s already idempotent. Consider gating byhasMore/isLoadingMoreor debouncing the call to avoid redundant full-list requests.♻️ Example guard to prevent redundant full loads
watch([filterText, sortOption], ([filter, sort]) => { - if (filter !== '' || sort !== 'downloads') { - loadAll() - } + if ((filter !== '' || sort !== 'downloads') && hasMore.value && !isLoadingMore.value) { + loadAll() + } debouncedUpdateUrl(filter, sort) })
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/pages/settings.vue (1)
166-181: Remove inlinefocus-visibleutility class from select element.The
focus-visible:outline-accent/70class on line 169 should be removed. Per project conventions, focus-visible styling for<select>elements is applied globally viamain.csswith a consistent rule. The theme selector (line 80) correctly relies on the global rule, but this select and the language select (line 232) have inline utilities.♻️ Suggested fix
<select id="search-provider-select" :value="settings.searchProvider" - class="w-full sm:w-auto min-w-48 bg-bg border border-border rounded-md px-3 py-2 text-sm text-fg focus-visible:outline-accent/70 cursor-pointer duration-200 transition-colors hover:border-fg-subtle" + class="w-full sm:w-auto min-w-48 bg-bg border border-border rounded-md px-3 py-2 text-sm text-fg cursor-pointer duration-200 transition-colors hover:border-fg-subtle" `@change`="Based on learnings: "In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via main.css... individual buttons or selects in Vue components should not rely on inline focus-visible utility classes."
app/composables/npm/useUserPackages.ts (1)
24-221: Consider splittinguseUserPackagesinto smaller helpers.The composable now mixes Algolia fetching, npm pagination, caching and UI helpers in one long function; extracting per-path helpers would improve readability and testability. As per coding guidelines, “Keep functions focused and manageable (generally under 50 lines)”.
| public: { | ||
| // Algolia npm-search index (maintained by Algolia & jsDelivr, used by yarnpkg.com et al.) | ||
| algolia: { | ||
| appId: 'OFCNCOG2CU', | ||
| apiKey: 'f54e21fa3a2a0160595bb058179bfb1e', | ||
| indexName: 'npm-search', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Algolia npm-search index public API key OFCNCOG2CU yarnpkg
💡 Result:
For Yarn’s Algolia-backed npm package search (“npm-search” index), the public (search-only) credentials embedded in Yarn’s interactive tools plugin are:
- Algolia Application ID:
OFCNCOG2CU[1] - Index name:
npm-search[1] - Search API key (public):
6fe4476ee5a1832882e326b506d14126[1]
Algolia explicitly designs Search API keys to be public (safe for frontend / end-user code), unlike Admin keys. [2][3]
Sources:
[1] Yarn plugin interactive tools docs (algolia.ts constants) (yarnpkg.cn)
[2] Algolia: “Can the search API key be public?” (support.algolia.com)
[3] Algolia: “Which API key should I use on my front end?” (support.algolia.com)
Citations:
- 1: https://www.yarnpkg.cn/api/modules/plugin_interactive_tools.html?utm_source=openai
- 2: https://support.algolia.com/hc/en-us/articles/18966776061329-Can-the-search-API-key-be-public?utm_source=openai
- 3: https://support.algolia.com/hc/en-us/articles/10153408378129-Which-API-key-should-I-use-on-my-front-end?utm_source=openai
Correct the Algolia API key — it does not match the documented public key for npm-search.
The appId (OFCNCOG2CU) and indexName (npm-search) are correct, but the apiKey (f54e21fa3a2a0160595bb058179bfb1e) does not match the official public search-only key for this index. According to Algolia's documentation, the public search-only key for npm-search is 6fe4476ee5a1832882e326b506d14126.
Either use the correct documented key, or clarify why a different key is required. If this is a personal or private key, it must not be committed to the repository.
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
There was a problem hiding this comment.
API key is correct to use, but I made one specifically for npmx.dev now: e0359cd30aa2c30b80044a0a1fe47282
There was a problem hiding this comment.
@Haroenv, thank you for the clarification and for creating a dedicated API key for npmx.dev! That's excellent practice for better tracking and key management.
You'll want to update line 40 in nuxt.config.ts to use the new key:
apiKey: 'e0359cd30aa2c30b80044a0a1fe47282',🐰✨
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/pages/search.vue (2)
317-387:⚠️ Potential issue | 🟡 MinorAvoid false negatives when an existence check is still in-flight.
When a cache entry is
'pending', the function returnsfalse, so concurrent callers can report “does not exist” even if the pending request will resolvetrue. This can happen when provider changes trigger immediate revalidation while a debounced call is still running. Consider caching the pending promise and awaiting it instead.
Line 317, Line 329, Line 372.💡 Suggested fix (cache the promise and await it)
-const existenceCache = ref<Record<string, boolean | 'pending'>>({}) +const existenceCache = ref<Record<string, boolean | Promise<boolean>>>({}) async function checkOrgExists(name: string): Promise<boolean> { const cacheKey = `org:${name.toLowerCase()}` - if (cacheKey in existenceCache.value) { - const cached = existenceCache.value[cacheKey] - return cached === true - } - existenceCache.value[cacheKey] = 'pending' - try { - const scopePrefix = `@${name.toLowerCase()}/` - if (isAlgolia.value) { - const response = await algoliaSearch(`@${name}`, { size: 5 }) - const exists = response.objects.some(obj => - obj.package.name.toLowerCase().startsWith(scopePrefix), - ) - existenceCache.value[cacheKey] = exists - return exists - } - const response = await $fetch<{ total: number; objects: Array<{ package: { name: string } }> }>( - `${NPM_REGISTRY}/-/v1/search`, - { query: { text: `@${name}`, size: 5 } }, - ) - const exists = response.objects.some(obj => - obj.package.name.toLowerCase().startsWith(scopePrefix), - ) - existenceCache.value[cacheKey] = exists - return exists - } catch { - existenceCache.value[cacheKey] = false - return false - } + const cached = existenceCache.value[cacheKey] + if (typeof cached === 'boolean') return cached + if (cached) return await cached + + const pending = (async () => { + try { + const scopePrefix = `@${name.toLowerCase()}/` + if (isAlgolia.value) { + const response = await algoliaSearch(`@${name}`, { size: 5 }) + return response.objects.some(obj => + obj.package.name.toLowerCase().startsWith(scopePrefix), + ) + } + const response = await $fetch<{ total: number; objects: Array<{ package: { name: string } }> }>( + `${NPM_REGISTRY}/-/v1/search`, + { query: { text: `@${name}`, size: 5 } }, + ) + return response.objects.some(obj => + obj.package.name.toLowerCase().startsWith(scopePrefix), + ) + } catch { + return false + } + })() + existenceCache.value[cacheKey] = pending + const exists = await pending + existenceCache.value[cacheKey] = exists + return exists }Apply the same pattern to
checkUserExists(Line 370-387).
735-742:⚠️ Potential issue | 🟡 MinorRemove inline focus-visible utilities on buttons (use global rule).
The
focus-visible:outline-accent/70utility on buttons conflicts with the global focus-visible styling policy.
Line 737, Line 840.✅ Suggested change
- class="shrink-0 px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md motion-safe:transition-colors motion-safe:duration-200 hover:bg-fg/90 focus-visible:outline-accent/70" + class="shrink-0 px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md motion-safe:transition-colors motion-safe:duration-200 hover:bg-fg/90"- class="px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md transition-colors duration-200 hover:bg-fg/90 focus-visible:outline-accent/70" + class="px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md transition-colors duration-200 hover:bg-fg/90"Based on learnings: In the npmx.dev project, focus-visible styling for button and select elements is implemented globally in app/assets/main.css; do not apply per-element inline focus-visible utilities like focus-visible:outline-accent/70 on buttons or selects.
Also applies to: 838-844
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/composables/npm/useAlgoliaSearch.ts (1)
198-198: Consider quoting the filter value for robustness.While npm usernames have restrictions that generally prevent problematic characters, quoting the filter value would guard against edge cases with special characters (e.g., names containing hyphens that might be misinterpreted in complex filter expressions).
🔧 Suggested change
- filters: `owner.name:${ownerName}`, + filters: `owner.name:"${ownerName}"`,
| <button | ||
| type="button" | ||
| role="menuitem" | ||
| class="w-full flex items-start gap-3 px-3 py-2.5 rounded-md text-start transition-colors hover:bg-bg-muted" |
There was a problem hiding this comment.
Now they are glued and it is noticeable when hovering.
| class="w-full flex items-start gap-3 px-3 py-2.5 rounded-md text-start transition-colors hover:bg-bg-muted" | |
| class="w-full flex items-start gap-3 px-3 py-2.5 rounded-md text-start transition-colors hover:bg-bg-muted mt-1" |
|
|
||
| const response = results[0] as SearchResponse<AlgoliaHit> | undefined | ||
| if (!response) { | ||
| return { isStale: false, objects: [], total: 0, time: new Date().toISOString() } |
There was a problem hiding this comment.
Shouldn't we do some additional processing in this case?
It seems this is a case where we won't figure out whether nothing was actually found or a server error
|
|
||
| // Algolia supports up to 1000 results per query with offset/length pagination | ||
| while (offset < max) { | ||
| const length = Math.min(batchSize, max - offset) |
There was a problem hiding this comment.
It seems the logic is broken here a bit. It should probably be sth like Math.max(max, totalPackages - downloadPackages)
Because right now, the maximum downloaded is 200, and the total could end up in the minus
| // Algolia fetches everything in one go | ||
| if (searchProvider.value !== 'npm') return false | ||
| if (!cache.value) return true | ||
| return cache.value.objects.length < Math.min(cache.value.total, MAX_RESULTS) |
There was a problem hiding this comment.
if cache.value.objects.length is 1700, cache.value.total is 1800, MAX_RESULTS - 1000 - this condition will return false. Is it planned?
app/pages/~[username]/index.vue
Outdated
| <!-- Loading state --> | ||
| <LoadingSpinner | ||
| v-if="status === 'pending' && currentPage === 1" | ||
| v-if="status === 'pending' && packages.length === 0" |
There was a problem hiding this comment.
It seems there is a chance for an eternal spinner?
danielroe
force-pushed
the
feat/algolia-search
branch
from
967c975 to
cd38980
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/composables/npm/useUserPackages.ts (1)
24-39: Consider extracting helper functions to keep this composable lean.
useUserPackagesis now far beyond the 50‑line guideline; splitting Algolia fetch, npm fetch, and pagination helpers would improve readability and testability.As per coding guidelines, "Keep functions focused and manageable (generally under 50 lines)".
…e against stale username
| // --- Algolia fast path: use Algolia to get metadata for known packages --- | ||
| if (searchProvider.value === 'algolia') { | ||
| try { | ||
| const response = await searchByOwner(org) | ||
| if (response.objects.length > 0) { | ||
| // Filter Algolia results to only include packages that are | ||
| // actually in the org (per the npm registry's authoritative list) | ||
| const orgPackageSet = new Set(packageNames.map(n => n.toLowerCase())) | ||
| const filtered = response.objects.filter(obj => | ||
| orgPackageSet.has(obj.package.name.toLowerCase()), | ||
| ) | ||
|
|
||
| if (filtered.length > 0) { | ||
| return { | ||
| ...response, | ||
| objects: filtered, | ||
| total: filtered.length, | ||
| } | ||
| } | ||
| } | ||
| } catch { | ||
| // Fall through to npm registry path | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle potential truncation when Algolia returns incomplete results.
The searchByOwner function now correctly exposes truncation via response.total (server-side nbHits) vs response.objects.length (fetched count). However, this code doesn't check for truncation before returning. If an organisation has more packages than Algolia's pagination limit (1000), some packages will be silently missing from the result.
Consider falling back to the npm registry path when truncation is detected:
🛡️ Proposed fix to detect and handle truncation
if (searchProvider.value === 'algolia') {
try {
const response = await searchByOwner(org)
+ const isTruncated = response.objects.length < response.total
if (response.objects.length > 0) {
// Filter Algolia results to only include packages that are
// actually in the org (per the npm registry's authoritative list)
const orgPackageSet = new Set(packageNames.map(n => n.toLowerCase()))
const filtered = response.objects.filter(obj =>
orgPackageSet.has(obj.package.name.toLowerCase()),
)
- if (filtered.length > 0) {
+ // Only use Algolia results if we got all of them (no truncation)
+ // and the filtered set covers a meaningful portion of the org
+ if (filtered.length > 0 && !isTruncated) {
return {
...response,
objects: filtered,
total: filtered.length,
}
}
}
} catch {
// Fall through to npm registry path
}
}
| "data_source": { | ||
| "label": "Data source", | ||
| "description": "Choose where npmx gets search data. Individual package pages always use the npm registry directly.", | ||
| "npm": "npm Registry", | ||
| "npm_description": "Fetches search, org and user listings directly from the official npm registry. Authoritative, but can be slower.", | ||
| "algolia": "Algolia", | ||
| "algolia_description": "Uses Algolia for faster search, org and user pages." | ||
| }, |
There was a problem hiding this comment.
Tighten copy to reflect org/user listings and npm’s Algolia index.
Line 72 currently mentions only “search data”, but the provider affects org/user listing pages too; the Algolia description can be more precise.
✏️ Suggested copy tweak
- "description": "Choose where npmx gets search data. Individual package pages always use the npm registry directly.",
+ "description": "Choose where npmx gets search, org, and user listing data. Individual package pages always use the npm registry directly.",
...
- "algolia_description": "Uses Algolia for faster search, org and user pages."
+ "algolia_description": "Uses npm’s Algolia index for faster search, org and user pages."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "data_source": { | |
| "label": "Data source", | |
| "description": "Choose where npmx gets search data. Individual package pages always use the npm registry directly.", | |
| "npm": "npm Registry", | |
| "npm_description": "Fetches search, org and user listings directly from the official npm registry. Authoritative, but can be slower.", | |
| "algolia": "Algolia", | |
| "algolia_description": "Uses Algolia for faster search, org and user pages." | |
| }, | |
| "data_source": { | |
| "label": "Data source", | |
| "description": "Choose where npmx gets search, org, and user listing data. Individual package pages always use the npm registry directly.", | |
| "npm": "npm Registry", | |
| "npm_description": "Fetches search, org and user listings directly from the official npm registry. Authoritative, but can be slower.", | |
| "algolia": "Algolia", | |
| "algolia_description": "Uses npm's Algolia index for faster search, org and user pages." | |
| }, |
|
thanks for taking over @danielroe :D |
|
However, could we add the acknowledgment again? |
|
the acknowledgement is in two places I believe - let's chat in discord and you can tell me where would be better 🙏
|
4 participants
remake of #174 as I didn't have permission to push
this builds on @Haroenv's work to add algolia as a provider of data for org/user package listings, as well as the package search page
it also makes it the default, as it is much faster
need to tweak the wording about algoliacloses #174
resolves #32