Skip to content
Merged
Show file tree
Hide file tree
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
173 changes: 94 additions & 79 deletions src/components/npm-stats/PackageSearch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@ import { useDebouncedValue } from '@tanstack/react-pacer'
import { Search } from 'lucide-react'
import { keepPreviousData, useQuery } from '@tanstack/react-query'
import { Command } from 'cmdk'
import { twMerge } from 'tailwind-merge'
import { Spinner } from '~/components/Spinner'

type NpmSearchResult = {
name: string
description?: string
version?: string
label?: string
publisher?: { username?: string }
}

const CREATE_ITEM_VALUE = '__create__'

export type PackageSearchProps = {
onSelect: (packageName: string) => void
placeholder?: string
Expand Down Expand Up @@ -48,110 +50,123 @@ export function PackageSearch({
}
}, [])

const hasUsableQuery = debouncedInputValue.length > 2

const searchQuery = useQuery({
queryKey: ['npm-search', debouncedInputValue],
queryFn: async () => {
if (!debouncedInputValue || debouncedInputValue.length <= 2)
return [] as Array<NpmSearchResult>

const response = await fetch(
`https://api.npms.io/v2/search?q=${encodeURIComponent(
`https://registry.npmjs.org/-/v1/search?text=${encodeURIComponent(
debouncedInputValue,
)}&size=10`,
)
const data = (await response.json()) as {
results: Array<{ package: NpmSearchResult }>
objects: Array<{ package: NpmSearchResult }>
}
return data.results.map((r) => r.package)
return data.objects.map((r) => r.package)
},
Comment on lines 57 to 67
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle registry failures explicitly.

Line 66 assumes a successful npm payload, but 4xx/5xx responses will put the query into an error state. The list then falls through to the empty-results path, which turns outages or rate limits into “No packages found”. Check response.ok and render an error state when searchQuery.isError is true.

🛡️ Suggested fix
   const searchQuery = useQuery({
     queryKey: ['npm-search', debouncedInputValue],
     queryFn: async () => {
       const response = await fetch(
         `https://registry.npmjs.org/-/v1/search?text=${encodeURIComponent(
           debouncedInputValue,
         )}&size=10`,
       )
+      if (!response.ok) {
+        throw new Error(`npm search failed: ${response.status}`)
+      }
       const data = (await response.json()) as {
         objects: Array<{ package: NpmSearchResult }>
       }
-      return data.objects.map((r) => r.package)
+      return data.objects?.map((r) => r.package) ?? []
     },
@@
-          ) : !searchResults.length && !showCreateItem ? (
+          ) : searchQuery.isError ? (
+            <div className="px-3 py-2 text-sm text-red-600 dark:text-red-400">
+              Search failed. Try again.
+            </div>
+          ) : !searchResults.length && !showCreateItem ? (
             <div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
               No packages found
             </div>

Also applies to: 129-137

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

In `@src/components/npm-stats/PackageSearch.tsx` around lines 57 - 67, The queryFn
currently assumes a successful npm registry response and maps response.json()
directly, so HTTP 4xx/5xx responses are treated as empty results; modify the
queryFn used in PackageSearch (the async fetch block that builds the registry
URL) to check response.ok and, if false, throw an Error including
response.status and response.statusText (or response.text()) so the react-query
hook sets searchQuery.isError; then update the component render logic to detect
searchQuery.isError and render an explicit error state/message instead of
falling back to the empty-results path; apply the same change to the other
queryFn instance referenced around lines 129–137.

enabled: debouncedInputValue.length > 2,
enabled: hasUsableQuery,
placeholderData: keepPreviousData,
})

const results = React.useMemo(() => {
const hasInputValue = searchQuery.data?.find(
(d) => d.name === debouncedInputValue,
)

return [
...(hasInputValue
? []
: [
{
name: debouncedInputValue,
label: `Use "${debouncedInputValue}"`,
},
]),
...(searchQuery.data ?? []),
]
}, [searchQuery.data, debouncedInputValue])

const handleInputChange = (value: string) => {
setInputValue(value)
}
const searchResults = hasUsableQuery ? (searchQuery.data ?? []) : []
const trimmedInput = debouncedInputValue.trim()
const showCreateItem =
hasUsableQuery &&
trimmedInput.length > 0 &&
!searchResults.some((d) => d.name === trimmedInput)
Comment on lines +53 to +77
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the live input for the create item and list state.

trimmedInput comes from the debounced value, so Line 80 can submit the previous term if the user presses Enter before the debounce settles. The same split state also makes Line 133 briefly show “No packages found” right after the third keystroke. Keep the debounce on the request only; derive create/empty-state UI from the current inputValue.

💡 Suggested fix
-  const hasUsableQuery = debouncedInputValue.length > 2
+  const trimmedInput = inputValue.trim()
+  const debouncedTrimmedInput = debouncedInputValue.trim()
+  const hasUsableQuery = debouncedTrimmedInput.length > 2

   const searchQuery = useQuery({
-    queryKey: ['npm-search', debouncedInputValue],
+    queryKey: ['npm-search', debouncedTrimmedInput],
     queryFn: async () => {
       const response = await fetch(
         `https://registry.npmjs.org/-/v1/search?text=${encodeURIComponent(
-          debouncedInputValue,
+          debouncedTrimmedInput,
         )}&size=10`,
       )
@@
-  const trimmedInput = debouncedInputValue.trim()
+  const isDebouncing = trimmedInput !== debouncedTrimmedInput
   const showCreateItem =
-    hasUsableQuery &&
+    trimmedInput.length > 2 &&
     trimmedInput.length > 0 &&
     !searchResults.some((d) => d.name === trimmedInput)
@@
-          {inputValue.length < 3 ? (
+          {trimmedInput.length < 3 ? (
             <div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
               Keep typing to search...
             </div>
-          ) : searchQuery.isLoading ? (
+          ) : isDebouncing || searchQuery.isLoading ? (
             <div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
               Searching...
             </div>

Also applies to: 80-82, 125-145

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

In `@src/components/npm-stats/PackageSearch.tsx` around lines 53 - 77, The UI is
using debouncedInputValue for create/empty-state logic which can show stale
info; keep the debounce only for the network request (use debouncedInputValue
for useQuery and its enabled flag) but derive UI state from the live inputValue:
replace trimmedInput = debouncedInputValue.trim() with trimmedInput =
inputValue.trim(), compute showCreateItem using trimmedInput and searchResults
(which still come from searchQuery), and compute the empty-list/“no packages
found” condition from inputValue.trim() as well so the create/empty UI reflects
the immediate user input while queries remain debounced.


const handleSelect = (value: string) => {
const selectedItem = results?.find((item) => item.name === value)
if (!selectedItem) return

onSelect(selectedItem.name)
if (value === CREATE_ITEM_VALUE) {
if (!trimmedInput) return
onSelect(trimmedInput)
} else {
const match = searchResults.find((item) => item.name === value)
if (!match) return
onSelect(match.name)
}
setInputValue('')
setOpen(false)
}

const showList = open && inputValue.length > 0

return (
<div className="flex-1" ref={containerRef}>
<div className="relative">
<Command className="w-full" shouldFilter={false}>
<div className="flex items-center gap-1">
<Search className="text-lg" />
<Command.Input
placeholder={placeholder}
className="w-full bg-gray-500/10 rounded-md px-2 py-1 min-w-[200px] text-sm"
value={inputValue}
onValueChange={handleInputChange}
onFocus={() => setOpen(true)}
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={autoFocus}
/>
</div>
<div className="flex-1 relative" ref={containerRef}>
<Command
className="w-full"
shouldFilter={false}
loop
label="Search npm packages"
>
<div className="flex items-center gap-1">
<Search className="text-lg" />
<Command.Input
placeholder={placeholder}
className="w-full bg-gray-500/10 rounded-md px-2 py-1 min-w-[200px] text-sm"
value={inputValue}
onValueChange={setInputValue}
onFocus={() => setOpen(true)}
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={autoFocus}
/>
{searchQuery.isFetching && (
<div className="absolute right-2 top-0 bottom-0 flex items-center justify-center">
<div className="absolute right-2 top-0 bottom-0 flex items-center justify-center pointer-events-none">
<Spinner className="text-sm" />
</div>
)}
{inputValue.length && open ? (
<Command.List className="absolute z-10 w-full mt-1 bg-white dark:bg-gray-800 rounded-md shadow-lg max-h-60 overflow-auto divide-y divide-gray-500/10">
{inputValue.length < 3 ? (
<div className="px-3 py-2">Keep typing to search...</div>
) : searchQuery.isLoading ? (
<div className="px-3 py-2 flex items-center gap-2">
Searching...
</div>
<Command.List
className={twMerge(
'absolute z-10 w-full mt-1 bg-white dark:bg-gray-800 rounded-md shadow-lg max-h-60 overflow-auto divide-y divide-gray-500/10',
!showList && 'hidden',
)}
>
{inputValue.length < 3 ? (
<div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
Keep typing to search...
</div>
) : searchQuery.isLoading ? (
<div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
Searching...
</div>
) : !searchResults.length && !showCreateItem ? (
<div className="px-3 py-2 text-sm text-gray-500 dark:text-gray-400">
No packages found
</div>
) : null}
{showCreateItem && (
<Command.Item
key={CREATE_ITEM_VALUE}
value={CREATE_ITEM_VALUE}
onSelect={handleSelect}
className="px-3 py-2 cursor-pointer hover:bg-gray-500/20 data-[selected=true]:bg-gray-500/20"
>
<div className="font-medium">Use "{trimmedInput}"</div>
</Command.Item>
)}
{searchResults.map((item) => (
<Command.Item
key={item.name}
value={item.name}
onSelect={handleSelect}
className="px-3 py-2 cursor-pointer hover:bg-gray-500/20 data-[selected=true]:bg-gray-500/20"
>
<div className="font-medium">{item.name}</div>
{item.description ? (
<div className="text-sm text-gray-500 dark:text-gray-400">
{item.description}
</div>
) : !results?.length ? (
<div className="px-3 py-2">No packages found</div>
) : null}
{results?.map((item) => (
<Command.Item
key={item.name}
value={item.name}
onSelect={handleSelect}
className="px-3 py-2 cursor-pointer hover:bg-gray-500/20 data-[selected=true]:bg-gray-500/20"
>
<div className="font-medium">{item.label || item.name}</div>
<div className="text-sm text-gray-500 dark:text-gray-400">
{item.description}
</div>
<div className="text-xs text-gray-400 dark:text-gray-500">
{item.version ? `v${item.version}• ` : ''}
{item.publisher?.username}
</div>
</Command.Item>
))}
</Command.List>
) : null}
</Command>
</div>
<div className="text-xs text-gray-400 dark:text-gray-500">
{item.version ? `v${item.version}` : ''}
{item.version && item.publisher?.username ? ' • ' : ''}
{item.publisher?.username}
</div>
</Command.Item>
))}
</Command.List>
</Command>
</div>
)
}
50 changes: 30 additions & 20 deletions src/routes/stats/npm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react'
import { Link, createFileRoute } from '@tanstack/react-router'
import * as v from 'valibot'
import { useThrottledCallback } from '@tanstack/react-pacer'
import * as DialogPrimitive from '@radix-ui/react-dialog'
import { X } from 'lucide-react'
import { useQuery } from '@tanstack/react-query'
import { Card } from '~/components/Card'
Expand Down Expand Up @@ -601,29 +602,38 @@ function RouteComponent() {
/>

{/* Combine Package Dialog */}
{combiningPackage && (
<div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50 p-2 sm:p-4">
<div className="bg-white dark:bg-gray-800 rounded-lg p-2 sm:p-4 w-full max-w-md">
<div className="flex justify-between items-center mb-2 sm:mb-4">
<h3 className="text-base sm:text-lg font-medium">
<DialogPrimitive.Root
open={combiningPackage !== null}
onOpenChange={(open) => {
if (!open) setCombiningPackage(null)
}}
>
<DialogPrimitive.Portal>
<DialogPrimitive.Overlay className="fixed inset-0 z-[999] bg-black/60 backdrop-blur-sm" />
<DialogPrimitive.Content className="fixed left-1/2 top-1/2 z-[1000] w-[calc(100%-1rem)] max-w-md -translate-x-1/2 -translate-y-1/2 rounded-xl bg-white dark:bg-gray-900 p-4 shadow-xl outline-none">
<div className="flex justify-between items-center mb-4">
<DialogPrimitive.Title className="text-base sm:text-lg font-medium text-gray-900 dark:text-gray-100">
Add packages to {combiningPackage}
</h3>
<button
onClick={() => setCombiningPackage(null)}
className="p-0.5 sm:p-1 hover:text-red-500"
>
</DialogPrimitive.Title>
<DialogPrimitive.Close className="rounded-full p-1 hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-500">
<X className="w-4 h-4 sm:w-5 sm:h-5" />
</button>
</DialogPrimitive.Close>
Comment on lines +618 to +620
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add an accessible name to the close button.

This is an icon-only control right now, so assistive tech does not get a reliable label. Add aria-label="Close dialog" or visually hidden text.

♿ Suggested fix
-                  <DialogPrimitive.Close className="rounded-full p-1 hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-500">
+                  <DialogPrimitive.Close
+                    aria-label="Close dialog"
+                    className="rounded-full p-1 hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-500"
+                  >
                     <X className="w-4 h-4 sm:w-5 sm:h-5" />
                   </DialogPrimitive.Close>
📝 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.

Suggested change
<DialogPrimitive.Close className="rounded-full p-1 hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-500">
<X className="w-4 h-4 sm:w-5 sm:h-5" />
</button>
</DialogPrimitive.Close>
<DialogPrimitive.Close
aria-label="Close dialog"
className="rounded-full p-1 hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-500"
>
<X className="w-4 h-4 sm:w-5 sm:h-5" />
</DialogPrimitive.Close>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/stats/npm/index.tsx` around lines 618 - 620, The close control
using DialogPrimitive.Close that renders only the X icon is missing an
accessible name; update the DialogPrimitive.Close element to include an
accessible label (e.g., add aria-label="Close dialog") or include visually
hidden text next to the X icon so screen readers get a reliable name—modify the
DialogPrimitive.Close usage that wraps the X component to include the aria-label
or hidden label text.

</div>
<PackageSearch
onSelect={handleAddToGroup}
placeholder="Search for packages to add..."
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={true}
/>
</div>
</div>
)}
<DialogPrimitive.Description className="sr-only">
Search for additional npm packages to combine with{' '}
{combiningPackage}.
</DialogPrimitive.Description>
{combiningPackage && (
<PackageSearch
onSelect={handleAddToGroup}
placeholder="Search for packages to add..."
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={true}
/>
)}
</DialogPrimitive.Content>
</DialogPrimitive.Portal>
</DialogPrimitive.Root>

{/* Color Picker Popover */}
{colorPickerPackage && colorPickerPosition && (
Expand Down
Loading