fix(ui): replace "yolo" mode with npm API max of 250 results#1922
fix(ui): replace "yolo" mode with npm API max of 250 results#1922serhalp merged 10 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (4)
📝 WalkthroughWalkthroughThis PR removes the special-case "all" page-size mode across the app and replaces it with a numeric 250 option. Pagination logic (totalPages, startItem, endItem, page-size handling) now always treats pageSize as numeric. PAGE_SIZE_OPTIONS was updated to include 250 instead of the string "all". Locale keys and schema entries referencing the "all" label (all_yolo) were deleted. A one-time migration converts any persisted 'all' preference to the new maximum numeric option and persists the change. 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)
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
🧹 Nitpick comments (1)
app/components/PaginationControls.vue (1)
25-26: Update the stale'all'mode comment.Line 25 still describes an
'all'mode that this PR removes. Please update/remove it to avoid misleading future edits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5bb1760-6dc3-427d-a758-ac30c34d1bef
📒 Files selected for processing (2)
app/components/PaginationControls.vueshared/types/preferences.ts
| export const PAGE_SIZE_OPTIONS = [10, 25, 50, 100, 250] as const | ||
| export type PageSize = (typeof PAGE_SIZE_OPTIONS)[number] |
There was a problem hiding this comment.
Add a migration/normalisation path for legacy 'all' page sizes.
Line 282 removes 'all' from PageSize, but persisted preferences can still hydrate 'all' from storage. That stale value can flow into pagination arithmetic and yield NaN ranges/pages for returning users.
Proposed patch in this file
export const PAGE_SIZE_OPTIONS = [10, 25, 50, 100, 250] as const
export type PageSize = (typeof PAGE_SIZE_OPTIONS)[number]
+
+export function isPageSize(value: unknown): value is PageSize {
+ return typeof value === 'number' && PAGE_SIZE_OPTIONS.includes(value as PageSize)
+}
+
+export function normalisePageSize(value: unknown, fallback: PageSize = 25): PageSize {
+ return isPageSize(value) ? value : fallback
+}Then use normalisePageSize(stored.pageSize) when hydrating preferences in app/composables/usePreferencesProvider.ts.
As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
There was a problem hiding this comment.
^ this is actually a good point 🐰. We persist preferences to localStorage. We should add logic where we read this from local storage to default it to our default (250).
There was a problem hiding this comment.
🧩 Analysis chain
Great point — let me locate the preferences hydration code first.
🏁 Script executed:
#!/bin/bash
# Find the preferences provider composable
fd -t f 'usePreferences' --exec echo {}
# Also search for localStorage usage related to preferences/pageSize
rg -n "pageSize|PAGE_SIZE|localStorage|useStorage|hydrat" --type vue --type ts -C 2 -g "!node_modules" -g "!dist"
Length of output: 71
🏁 Script executed:
#!/bin/bash
# Read the preferences provider composable
cat app/composables/usePreferencesProvider.ts
# Search for pageSize/localStorage/useStorage in ts files
rg -n "pageSize\|PAGE_SIZE\|localStorage\|useStorage\|normalise\|hydrat" --type ts -C 3 -g "!node_modules"
# Search in .vue files separately
rg -n "pageSize\|PAGE_SIZE\|localStorage\|useStorage\|normalise\|hydrat" -C 3 -g "*.vue" -g "!node_modules"
Length of output: 2548
🏁 Script executed:
#!/bin/bash
# Find where usePreferencesProvider is consumed
rg -n "usePreferencesProvider\|PackageListPreferences\|DEFAULT_PREFERENCES" --type ts -C 3 -g "!node_modules"
# Also find any composable that references pageSize + preferences together
rg -n "pageSize" --type ts -C 3 -g "!node_modules"
Length of output: 6924
🏁 Script executed:
#!/bin/bash
cat app/composables/usePackageListPreferences.ts
Length of output: 2556
@serhalp — here's the full picture from the code and the plan:
-
usePreferencesProvider.tsusesdefu(stored, defaultValue)on mount.defuonly fills inundefined/nullkeys, so a stored'all'survives the merge unchanged and flows into pagination arithmetic. -
usePackageListPreferences.tsis the right place to apply the fix — it owns both the hydration flag (isHydrated) and thesave()call, so we can normalise the stale value and immediately re-persist the corrected value to localStorage in one step. -
Add
isPageSize+normalisePageSizeguards toshared/types/preferences.ts -
Watch
isHydratedinusePackageListPreferences.tsto normalise'all'→250on first hydration and re-persist
✏️ Learnings added
Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 1922
File: shared/types/preferences.ts:282-283
Timestamp: 2026-03-05T00:49:15.091Z
Learning: In npmx-dev/npmx.dev, the preferences hydration in app/composables/usePreferencesProvider.ts uses defu(stored, defaultValue) on onMounted. defu only fills in null/undefined keys, so a stale persisted value (e.g. legacy 'all' pageSize) survives the merge unchanged. Normalisation/migration of stale stored values must be done explicitly after hydration, not via defu defaults.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: MatteoGabriele
Repo: npmx-dev/npmx.dev PR: 1922
File: app/pages/org/[org].vue:68-69
Timestamp: 2026-03-04T13:25:19.779Z
Learning: In npmx-dev/npmx.dev, PAGE_SIZE_OPTIONS in shared/types/preferences.ts contains only strictly numeric values: 10, 25, 50, 100, and 250. The previous 'all' string option was removed in PR `#1922`. The derived PageSize type is therefore 10 | 25 | 50 | 100 | 250 — no non-numeric values exist. Code that reads pageSize.value (e.g., in app/pages/org/[org].vue) can safely use ?? 25 without needing Number() coercion, as TypeScript enforces numeric-only values.
Failed to handle agent chat message. Please try again.
There was a problem hiding this comment.
@serhalp, what if I drop something like this into the usePackageListPreferences file instead of checking for "all"?
const pageSize = computed({
get: () => {
const size = preferences.value.pageSize as unknown
// Migrate legacy "all" value to the highest number
if (size === 'all') {
const max = Math.max(...PAGE_SIZE_OPTIONS) as PageSize
preferences.value.pageSize = max
save()
return max
}
return preferences.value.pageSize
},
set: (value: PageSize) => {
preferences.value.pageSize = value
save()
},
})There was a problem hiding this comment.
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!
There was a problem hiding this comment.
@MatteoGabriele — happy to join! 🐰
The concept is solid and self-contained, but there's one Vue-specific concern: mutating reactive state and calling save() inside a computed getter is a side effect, which Vue discourages. The reactivity system can invoke getters multiple times during a render cycle, and while in practice preferences.value.pageSize would flip to a numeric value on the first invocation (preventing repeated save() calls), Vue may log a warning.
A small refinement that keeps the same inline spirit but moves the side effect out of the getter:
// One-time migration: replace legacy 'all' with the current maximum page size
watch(isHydrated, (hydrated) => {
if (!hydrated) return
if ((preferences.value.pageSize as unknown) === 'all') {
preferences.value.pageSize = Math.max(...PAGE_SIZE_OPTIONS) as PageSize
save()
}
}, { immediate: true })
const pageSize = computed({
get: () => preferences.value.pageSize,
set: (value: PageSize) => {
preferences.value.pageSize = value
save()
},
})This runs exactly once after hydration, keeps the getter pure, and achieves the same outcome — stale 'all' gets replaced with 250 and persisted back to localStorage. The Math.max(...PAGE_SIZE_OPTIONS) trick from your snippet is a nice touch to avoid hardcoding 250.
🧠 Learnings used
Learnt from: MatteoGabriele
Repo: npmx-dev/npmx.dev PR: 1922
File: app/pages/org/[org].vue:68-69
Timestamp: 2026-03-04T13:25:19.779Z
Learning: In npmx-dev/npmx.dev, PAGE_SIZE_OPTIONS in shared/types/preferences.ts contains only strictly numeric values: 10, 25, 50, 100, and 250. The previous 'all' string option was removed in PR `#1922`. The derived PageSize type is therefore 10 | 25 | 50 | 100 | 250 — no non-numeric values exist. Code that reads pageSize.value (e.g., in app/pages/org/[org].vue) can safely use ?? 25 without needing Number() coercion, as TypeScript enforces numeric-only values.
Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 1922
File: shared/types/preferences.ts:282-283
Timestamp: 2026-03-05T00:49:15.091Z
Learning: In npmx-dev/npmx.dev, the preferences hydration in app/composables/usePreferencesProvider.ts uses defu(stored, defaultValue) on onMounted. defu only fills in null/undefined keys, so a stale persisted value (e.g. legacy 'all' pageSize) survives the merge unchanged. Normalisation/migration of stale stored values must be done explicitly after hydration, not via defu defaults.
MatteoGabriele
changed the title
…eoGabriele/npmx.dev into fix/search-yolo-pagination-
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da5a888d-72a0-4753-8230-406ab0e56a52
📒 Files selected for processing (4)
app/components/Package/List.vueapp/components/Package/ListToolbar.vueapp/pages/org/[org].vueapp/pages/search.vue
MatteoGabriele
changed the title
| @@ -23,22 +23,17 @@ const effectiveMode = computed<PaginationMode>(() => | |||
| ) | |||
|
|
|||
| // When 'all' is selected, there's only 1 page with everything | |||
There was a problem hiding this comment.
| // When 'all' is selected, there's only 1 page with everything |
There was a problem hiding this comment.
| const newSize = Number(value) as PageSize |
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/components/PaginationControls.vue (1)
25-25: Drop the redundant numeric assertion onpageSize.Line 25 uses
pageSize.value as number, butPageSizeis already numeric-only in this PR, so the assertion is unnecessary noise.♻️ Proposed refactor
-const totalPages = computed(() => Math.ceil(props.totalItems / (pageSize.value as number))) +const totalPages = computed(() => Math.ceil(props.totalItems / pageSize.value))Based on learnings:
PageSizenow contains only numeric values (10 | 25 | 50 | 100 | 250) after removing'all'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74e6d902-7c7e-4afe-9afd-f53484f21592
📒 Files selected for processing (28)
app/components/PaginationControls.vuei18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-DE.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsoni18n/schema.json
💤 Files with no reviewable changes (27)
- i18n/locales/bn-IN.json
- i18n/locales/fr-FR.json
- i18n/locales/ta-IN.json
- i18n/locales/cs-CZ.json
- i18n/locales/it-IT.json
- i18n/locales/tr-TR.json
- i18n/locales/nb-NO.json
- i18n/locales/uk-UA.json
- i18n/locales/zh-TW.json
- i18n/locales/ar.json
- i18n/locales/pl-PL.json
- i18n/locales/ne-NP.json
- i18n/locales/ja-JP.json
- i18n/locales/kn-IN.json
- i18n/locales/te-IN.json
- i18n/locales/pt-BR.json
- i18n/locales/de-DE.json
- i18n/locales/es.json
- i18n/locales/zh-CN.json
- i18n/locales/ru-RU.json
- i18n/locales/id-ID.json
- i18n/locales/az-AZ.json
- i18n/locales/en.json
- i18n/schema.json
- i18n/locales/bg-BG.json
- i18n/locales/hi-IN.json
- i18n/locales/hu-HU.json
| const newSize = Number(value) as PageSize | ||
| pageSize.value = newSize |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "PaginationControls.vue" | head -20Repository: npmx-dev/npmx.dev
Length of output: 231
🏁 Script executed:
fd -type f -path "*shared/types/preferences.ts" | head -5Repository: npmx-dev/npmx.dev
Length of output: 231
🏁 Script executed:
fd "PaginationControls.vue"Repository: npmx-dev/npmx.dev
Length of output: 98
🏁 Script executed:
fd "preferences.ts"Repository: npmx-dev/npmx.dev
Length of output: 133
🏁 Script executed:
cat -n app/components/PaginationControls.vue | sed -n '90,115p'Repository: npmx-dev/npmx.dev
Length of output: 831
🏁 Script executed:
cat -n shared/types/preferences.tsRepository: npmx-dev/npmx.dev
Length of output: 10569
🏁 Script executed:
cat -n app/components/PaginationControls.vue | sed -n '111,180p'Repository: npmx-dev/npmx.dev
Length of output: 3187
Consider explicit validation of page-size value.
Line 104 uses an unchecked cast: Number(value) as PageSize. Although the select element is constrained to valid options (10, 25, 50, 100, 250), an explicit guard makes the runtime constraint clear at the code level rather than relying solely on template construction.
Suggested approach
- const newSize = Number(value) as PageSize
- pageSize.value = newSize
+ const newSize = Number(value)
+ if (!PAGE_SIZE_OPTIONS.includes(newSize as PageSize)) return
+ pageSize.value = newSize as PageSizeThis follows strict type-safety practices by explicitly validating before state update, rather than trusting the type cast alone.
📝 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.
| const newSize = Number(value) as PageSize | |
| pageSize.value = newSize | |
| const newSize = Number(value) | |
| if (!PAGE_SIZE_OPTIONS.includes(newSize as PageSize)) return | |
| pageSize.value = newSize as PageSize |
serhalp
left a comment
serhalp
left a comment
There was a problem hiding this comment.
LGTM, just the "migration" issue
| export const PAGE_SIZE_OPTIONS = [10, 25, 50, 100, 250] as const | ||
| export type PageSize = (typeof PAGE_SIZE_OPTIONS)[number] |
There was a problem hiding this comment.
^ this is actually a good point 🐰. We persist preferences to localStorage. We should add logic where we read this from local storage to default it to our default (250).
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: d35d2a2c-120b-4384-8a08-c4473816ae83
📒 Files selected for processing (1)
app/composables/usePackageListPreferences.ts
serhalp
left a comment
serhalp
left a comment
There was a problem hiding this comment.
Thank you!
Goodbye, my sweet yolo mode 🥹.
3 participants
🔗 Linked issue
closes #1916
🧭 Context
As stated in the ticket, npm APIs don't support a "yolo" mode, and you can only receive up to 250 results per page.
This change removes the "yolo" mode, which was not working initially, and simply adds the maximum number of items per page.
📚 Description
Removes the "all" options from the size array and uses 250, which is the maximum allowed by npm.