Skip to content

Commit c4b5015

Browse files
committed
fix: harden and polish /compare package search
- use the existing package search composable - improve loading states in result grid: facet-level and column-level, and don't hide existing data for some columns/rows while new/others are loading
1 parent 65412ec commit c4b5015

11 files changed

Lines changed: 108 additions & 212 deletions

File tree

app/components/AppHeader.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ onKeyStroke(',', e => {
8181
to="/compare"
8282
class="hidden sm:inline-flex link-subtle font-mono text-sm items-center gap-1.5 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent/50 rounded"
8383
>
84-
<span class="i-carbon-compare w-4 h-4" aria-hidden="true" />
84+
<span class="i-carbon:compare w-4 h-4" aria-hidden="true" />
8585
{{ $t('nav.compare') }}
8686
</NuxtLink>
8787

app/components/compare/FacetRow.vue

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ const props = defineProps<{
88
description?: string
99
/** Values for each column */
1010
values: (FacetValue | null | undefined)[]
11-
/** Whether this row is loading */
12-
loading?: boolean
11+
/** Whether this facet is loading (e.g., install size) */
12+
facetLoading?: boolean
13+
/** Whether each column is loading (array matching values) */
14+
columnLoading?: boolean[]
1315
/** Whether to show the proportional bar (defaults to true for numeric values) */
1416
bar?: boolean
1517
}>()
@@ -50,6 +52,11 @@ function getStatusClass(status?: FacetValue['status']): string {
5052
return 'text-fg'
5153
}
5254
}
55+
56+
// Check if a specific cell is loading
57+
function isCellLoading(index: number): boolean {
58+
return props.facetLoading || (props.columnLoading?.[index] ?? false)
59+
}
5360
</script>
5461

5562
<template>
@@ -82,7 +89,7 @@ function getStatusClass(status?: FacetValue['status']): string {
8289
/>
8390

8491
<!-- Loading state -->
85-
<template v-if="loading">
92+
<template v-if="isCellLoading(index)">
8693
<span
8794
class="i-carbon:circle-dash w-4 h-4 text-fg-subtle motion-safe:animate-spin"
8895
aria-hidden="true"

app/components/compare/PackageSelector.vue

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
<script setup lang="ts">
2-
import { debounce } from 'perfect-debounce'
3-
42
const packages = defineModel<string[]>({ required: true })
53
64
const props = defineProps<{
@@ -14,42 +12,20 @@ const maxPackages = computed(() => props.max ?? 4)
1412
const inputValue = ref('')
1513
const isInputFocused = ref(false)
1614
17-
// Search state
18-
const searchResults = ref<Array<{ name: string; description?: string }>>([])
19-
const isSearching = ref(false)
15+
// Use the shared npm search composable
16+
const { data: searchData, status } = useNpmSearch(inputValue, { size: 15 })
2017
21-
// Debounced search
22-
const performSearch = debounce(async (query: string) => {
23-
if (!query.trim()) {
24-
searchResults.value = []
25-
return
26-
}
18+
const isSearching = computed(() => status.value === 'pending')
2719
28-
isSearching.value = true
29-
try {
30-
const response = await $fetch<{
31-
objects: Array<{ package: { name: string; description?: string } }>
32-
}>(`https://registry.npmjs.org/-/v1/search`, {
33-
query: { text: query, size: 15 },
34-
})
35-
searchResults.value = response.objects.map(o => ({
20+
// Filter out already selected packages
21+
const filteredResults = computed(() => {
22+
if (!searchData.value?.objects) return []
23+
return searchData.value.objects
24+
.map(o => ({
3625
name: o.package.name,
3726
description: o.package.description,
3827
}))
39-
} catch {
40-
searchResults.value = []
41-
} finally {
42-
isSearching.value = false
43-
}
44-
}, 200)
45-
46-
watch(inputValue, value => {
47-
performSearch(value)
48-
})
49-
50-
// Filter out already selected packages
51-
const filteredResults = computed(() => {
52-
return searchResults.value.filter(r => !packages.value.includes(r.name))
28+
.filter(r => !packages.value.includes(r.name))
5329
})
5430
5531
function addPackage(name: string) {
@@ -58,7 +34,6 @@ function addPackage(name: string) {
5834
5935
packages.value = [...packages.value, name]
6036
inputValue.value = ''
61-
searchResults.value = []
6237
}
6338
6439
function removePackage(name: string) {
@@ -108,13 +83,17 @@ function handleBlur() {
10883
<!-- Add package input -->
10984
<div v-if="packages.length < maxPackages" class="relative">
11085
<div class="relative">
86+
<label for="package-search" class="sr-only">
87+
{{ $t('compare.selector.search_label') }}
88+
</label>
11189
<span
11290
class="absolute inset-is-3 top-1/2 -translate-y-1/2 text-fg-subtle"
11391
aria-hidden="true"
11492
>
11593
<span class="i-carbon:search w-4 h-4" />
11694
</span>
11795
<input
96+
id="package-search"
11897
v-model="inputValue"
11998
type="text"
12099
:placeholder="

app/composables/usePackageComparison.ts

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,42 +33,46 @@ export interface PackageComparisonData {
3333
export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
3434
const packages = computed(() => toValue(packageNames))
3535

36-
// Reactive state
37-
const packagesData = ref<(PackageComparisonData | null)[]>([])
36+
// Cache of fetched data by package name (source of truth)
37+
const cache = shallowRef(new Map<string, PackageComparisonData>())
38+
39+
// Derived array in current package order
40+
const packagesData = computed(() => packages.value.map(name => cache.value.get(name) ?? null))
41+
3842
const status = ref<'idle' | 'pending' | 'success' | 'error'>('idle')
3943
const error = ref<Error | null>(null)
4044

45+
// Track which packages are currently being fetched
46+
const loadingPackages = shallowRef(new Set<string>())
47+
4148
// Track install size loading separately (it's slower)
4249
const installSizeLoading = ref(false)
4350

44-
// Track last fetched packages to avoid redundant fetches
45-
let lastFetchedPackages: string[] = []
46-
47-
// Fetch function
51+
// Fetch function - only fetches packages not already in cache
4852
async function fetchPackages(names: string[]) {
4953
if (names.length === 0) {
50-
packagesData.value = []
5154
status.value = 'idle'
52-
lastFetchedPackages = []
5355
return
5456
}
5557

56-
// Skip fetch if packages haven't actually changed
57-
if (
58-
names.length === lastFetchedPackages.length &&
59-
names.every((n, i) => n === lastFetchedPackages[i])
60-
) {
58+
// Only fetch packages not already cached
59+
const namesToFetch = names.filter(name => !cache.value.has(name))
60+
61+
if (namesToFetch.length === 0) {
62+
status.value = 'success'
6163
return
6264
}
6365

64-
lastFetchedPackages = [...names]
6566
status.value = 'pending'
6667
error.value = null
6768

69+
// Mark packages as loading
70+
loadingPackages.value = new Set(namesToFetch)
71+
6872
try {
6973
// First pass: fetch fast data (package info, downloads, analysis, vulns)
7074
const results = await Promise.all(
71-
names.map(async (name): Promise<PackageComparisonData | null> => {
75+
namesToFetch.map(async (name): Promise<PackageComparisonData | null> => {
7276
try {
7377
// Fetch basic package info first (required)
7478
const pkgData = await $fetch<{
@@ -131,26 +135,35 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
131135
}),
132136
)
133137

134-
packagesData.value = results
138+
// Add results to cache
139+
const newCache = new Map(cache.value)
140+
for (const [i, name] of namesToFetch.entries()) {
141+
const data = results[i]
142+
if (data) {
143+
newCache.set(name, data)
144+
}
145+
}
146+
cache.value = newCache
147+
loadingPackages.value = new Set()
135148
status.value = 'success'
136149

137-
// Second pass: fetch slow install size data in background
150+
// Second pass: fetch slow install size data in background for new packages
138151
installSizeLoading.value = true
139152
Promise.all(
140-
names.map(async (name, index) => {
153+
namesToFetch.map(async name => {
141154
try {
142155
const installSize = await $fetch<{
143156
selfSize: number
144157
totalSize: number
145158
dependencyCount: number
146159
}>(`/api/registry/install-size/${name}`)
147160

148-
// Update the specific package's install size
149-
if (packagesData.value[index]) {
150-
packagesData.value[index] = {
151-
...packagesData.value[index]!,
152-
installSize,
153-
}
161+
// Update cache with install size
162+
const existing = cache.value.get(name)
163+
if (existing) {
164+
const updated = new Map(cache.value)
165+
updated.set(name, { ...existing, installSize })
166+
cache.value = updated
154167
}
155168
} catch {
156169
// Install size fetch failed, leave as undefined
@@ -160,6 +173,7 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
160173
installSizeLoading.value = false
161174
})
162175
} catch (e) {
176+
loadingPackages.value = new Set()
163177
error.value = e as Error
164178
status.value = 'error'
165179
}
@@ -193,12 +207,19 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
193207
return facet === 'installSize' || facet === 'dependencies'
194208
}
195209

210+
// Check if a specific column (package) is loading
211+
function isColumnLoading(index: number): boolean {
212+
const name = packages.value[index]
213+
return name ? loadingPackages.value.has(name) : false
214+
}
215+
196216
return {
197217
packagesData: readonly(packagesData),
198218
status: readonly(status),
199219
error: readonly(error),
200220
getFacetValues,
201221
isFacetLoading,
222+
isColumnLoading,
202223
}
203224
}
204225

app/pages/compare.vue

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ const { selectedFacets, selectAll, deselectAll, isAllSelected, isNoneSelected }
2929
useFacetSelection()
3030
3131
// Fetch comparison data
32-
const { packagesData, status, getFacetValues, isFacetLoading } = usePackageComparison(packages)
32+
const { packagesData, status, getFacetValues, isFacetLoading, isColumnLoading } =
33+
usePackageComparison(packages)
34+
35+
// Get loading state for each column
36+
const columnLoading = computed(() => packages.value.map((_, i) => isColumnLoading(i)))
3337
3438
// Check if we have enough packages to compare
3539
const canCompare = computed(() => packages.value.length >= 2)
@@ -110,19 +114,23 @@ useSeoMeta({
110114
{{ $t('compare.packages.section_comparison') }}
111115
</h2>
112116

113-
<div v-if="status === 'pending'" class="flex items-center justify-center py-12">
117+
<div
118+
v-if="status === 'pending' && (!packagesData || packagesData.every(p => p === null))"
119+
class="flex items-center justify-center py-12"
120+
>
114121
<LoadingSpinner :text="$t('compare.packages.loading')" />
115122
</div>
116123

117-
<div v-else-if="packagesData && packagesData.length > 0">
124+
<div v-else-if="packagesData && packagesData.some(p => p !== null)">
118125
<CompareComparisonGrid :columns="packages.length" :headers="gridHeaders">
119126
<CompareFacetRow
120127
v-for="facet in selectedFacets"
121128
:key="facet"
122129
:label="FACET_INFO[facet].label"
123130
:description="FACET_INFO[facet].description"
124131
:values="getFacetValues(facet)"
125-
:loading="isFacetLoading(facet)"
132+
:facet-loading="isFacetLoading(facet)"
133+
:column-loading="columnLoading"
126134
:bar="facet !== 'lastUpdated'"
127135
/>
128136
</CompareComparisonGrid>

i18n/locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,7 @@
768768
"empty_description": "Search and add at least 2 packages above to see a side-by-side comparison of their metrics."
769769
},
770770
"selector": {
771+
"search_label": "Search for packages",
771772
"search_first": "Search for a package...",
772773
"search_add": "Add another package...",
773774
"searching": "Searching...",

i18n/locales/fr-FR.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@
753753
"empty_description": "Recherchez et ajoutez au moins 2 paquets ci-dessus pour voir une comparaison côte à côte de leurs facettes."
754754
},
755755
"selector": {
756+
"search_label": "Rechercher des paquets",
756757
"search_first": "Rechercher un paquet...",
757758
"search_add": "Ajouter un autre paquet...",
758759
"searching": "Recherche...",

lunaria/files/en-US.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,7 @@
768768
"empty_description": "Search and add at least 2 packages above to see a side-by-side comparison of their metrics."
769769
},
770770
"selector": {
771+
"search_label": "Search for packages",
771772
"search_first": "Search for a package...",
772773
"search_add": "Add another package...",
773774
"searching": "Searching...",

lunaria/files/fr-FR.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@
753753
"empty_description": "Recherchez et ajoutez au moins 2 paquets ci-dessus pour voir une comparaison côte à côte de leurs facettes."
754754
},
755755
"selector": {
756+
"search_label": "Rechercher des paquets",
756757
"search_first": "Rechercher un paquet...",
757758
"search_add": "Ajouter un autre paquet...",
758759
"searching": "Recherche...",

test/nuxt/components/compare/FacetRow.spec.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,29 @@ describe('FacetRow', () => {
7171
expect(component.text()).toContain('2K')
7272
})
7373

74-
it('renders loading state', async () => {
74+
it('renders loading state for facet loading', async () => {
7575
const component = await mountSuspended(FacetRow, {
7676
props: {
7777
...baseProps,
78-
values: [null],
79-
loading: true,
78+
values: [null, null],
79+
facetLoading: true,
80+
},
81+
})
82+
// All cells should show loading spinner
83+
expect(component.findAll('.i-carbon\\:circle-dash').length).toBe(2)
84+
})
85+
86+
it('renders loading state for specific column loading', async () => {
87+
const component = await mountSuspended(FacetRow, {
88+
props: {
89+
...baseProps,
90+
values: [{ raw: 1000, display: '1K', status: 'neutral' }, null],
91+
columnLoading: [false, true],
8092
},
8193
})
82-
expect(component.find('.i-carbon\\:circle-dash').exists()).toBe(true)
94+
// Only second cell should show loading spinner
95+
const spinners = component.findAll('.i-carbon\\:circle-dash')
96+
expect(spinners.length).toBe(1)
8397
})
8498
})
8599

0 commit comments

Comments
 (0)