-
-
Notifications
You must be signed in to change notification settings - Fork 333
fix(npm-stats): use official npm registry search and fix search UX #845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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) | ||
| }, | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the live input for the create item and list state.
💡 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 |
||
|
|
||
| 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> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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' | ||||||||||||||||||||||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 ♿ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| </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 && ( | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.okand render an error state whensearchQuery.isErroris 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