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
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughAdds a new exported SearchProvider type and a SearchResponse (now including totalUnlimited) to shared types; propagates totalUnlimited through search composables and refactors the search cache and response normalization in useSearch (introducing SEARCH_ENGINE_HITS_LIMIT and DEFAULT_INITIAL_SEARCH_LIMIT). Replaces hard‑coded pagination threshold with ALL_PAGES_VISIBLE_TRESHOLD and disables distant page buttons when threshold exceeded; localises displayed item numbers. Updates Algolia type aliasing in useAlgoliaSearch, removes an ARIA role from the package card keywords list, and adds i18n keys/messages for “more results available” hints. Possibly related PRs
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 Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/components/PaginationControls.vue (1)
5-5: Rename the threshold constant before the typo spreads further.
ALL_PAGES_VISIBLE_TRESHOLDis misspelt, which makes the intent look accidental and makes future searches/reuse harder. Please rename it toALL_PAGES_VISIBLE_THRESHOLDwhile it is still local to this component.As per coding guidelines,
**/*.{ts,tsx,vue}should "Use clear, descriptive variable and function names".Also applies to: 68-68, 104-104
app/composables/useSettings.ts (1)
5-5: Prefer the shared-types auto-import here.
app/composables/*already getsshared/types/*exports auto-imported, so this extra type import is unnecessary noise.Based on learnings, exports from
shared/types/*are auto-imported by Nuxt for composables and components, so explicit imports for those types are unnecessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d48761f8-b319-4497-8e1e-c7c6b829a4d1
📒 Files selected for processing (13)
app/components/Package/Card.vueapp/components/PaginationControls.vueapp/composables/npm/search-utils.tsapp/composables/npm/useAlgoliaSearch.tsapp/composables/npm/useSearch.tsapp/composables/npm/useUserPackages.tsapp/composables/useGlobalSearch.tsapp/composables/useSettings.tsapp/pages/search.vuei18n/locales/en.jsoni18n/schema.jsonshared/types/npm-registry.tsshared/types/preferences.ts
💤 Files with no reviewable changes (1)
- app/components/Package/Card.vue
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
i18n/locales/en.json (1)
75-76: Don't bake the cap values into the English copy.Line 76 hard-codes
1,000and5,000, but the real caps already live in the search logic. If those limits change again, the UI text will drift even though the behaviour is correct. Passing the caps as i18n params would keep the message aligned with enforcement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 924a2186-bd03-4d75-93cb-6460fa8e22e2
📒 Files selected for processing (3)
app/pages/search.vuei18n/locales/en.jsoni18n/schema.json
🚧 Files skipped from review as they are similar to previous changes (1)
- i18n/schema.json
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: dfc2700e-c643-45ae-8514-6e6c703ed936
📒 Files selected for processing (6)
app/composables/npm/search-utils.tsapp/composables/npm/useAlgoliaSearch.tsapp/composables/npm/useSearch.tsi18n/locales/en.jsoni18n/schema.jsonshared/types/npm-registry.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- i18n/locales/en.json
- i18n/schema.json
- shared/types/npm-registry.ts
- app/composables/npm/useAlgoliaSearch.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/pages/search.vue (1)
455-474: Verify destructuring in watch callback behaves as intended.The pattern
watch(displayResults, ([firstResult]) => { ... })destructures the first element from the watched array. This works because Vue's watch callback receives the new value (the array) as its first argument, and array destructuring extracts the first element.However, this pattern may be unfamiliar to some developers. Consider adding a brief comment or using the more explicit form for clarity:
Optional: More explicit alternative
-watch(displayResults, ([firstResult]) => { +watch(displayResults, (results) => { + const firstResult = results[0] if (!pendingEnterQuery.value) returnThe current implementation is functionally correct and type-safe (thanks to optional chaining at line 470), so this is purely a readability suggestion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c97bcca-a9e1-4f79-9cfc-ddd40356b415
📒 Files selected for processing (2)
app/composables/npm/useSearch.tsapp/pages/search.vue
1 participant
🔗 Linked issue
Resolves #1923, #1921
🧭 Context
Search results pagination worked with errors for outputs with many items. There are issues with results for 1001+ items.
After investigation I've found the following limits:
searchendpoint) is strictly limited to 1000 hits(see jsDelivr that uses Algolia has only 100 pages of 10 items while 180k results available). See in Algolia docs. This means that we should limit our output on Algolia engine to 1000 hits. Possible solutions: usebrowseendpoint (see docs), but it is not available in current Algolia index used by npmx📚 Description
The following fixes implemented: