Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
52 changes: 46 additions & 6 deletions app/components/Compare/PackageSelector.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script setup lang="ts">
import { NO_DEPENDENCY_ID } from '~/composables/usePackageComparison'
import { checkPackageExists } from '~/utils/package-name'

const packages = defineModel<string[]>({ required: true })

Expand All @@ -13,6 +14,12 @@ const maxPackages = computed(() => props.max ?? 4)
// Input state
const inputValue = shallowRef('')
const isInputFocused = shallowRef(false)
const isCheckingPackage = shallowRef(false)
const packageError = shallowRef('')

watch(inputValue, () => {
packageError.value = ''
})

// Use the shared npm search composable
const { data: searchData, status } = useNpmSearch(inputValue, { size: 15 })
Expand Down Expand Up @@ -76,10 +83,36 @@ function removePackage(name: string) {
packages.value = packages.value.filter(p => p !== name)
}

function handleKeydown(e: KeyboardEvent) {
if (e.key === 'Enter' && inputValue.value.trim()) {
e.preventDefault()
addPackage(inputValue.value.trim())
async function handleKeydown(e: KeyboardEvent) {
if (e.key !== 'Enter' || !inputValue.value.trim() || isCheckingPackage.value) return
e.preventDefault()

const name = inputValue.value.trim()
if (packages.value.length >= maxPackages.value) return
if (packages.value.includes(name)) return

// If it matches a dropdown result, add immediately (already confirmed to exist)
const exactMatch = filteredResults.value.find(r => r.name === name)
if (exactMatch) {
addPackage(exactMatch.name)
return
}

// Otherwise, verify it exists on npm
isCheckingPackage.value = true
packageError.value = ''
try {
const exists = await checkPackageExists(name)
if (name !== inputValue.value.trim()) return // stale guard
if (exists) {
addPackage(name)
} else {
packageError.value = `Package "${name}" was not found on npm.`
}
} catch {
packageError.value = 'Could not verify package. Please try again.'
} finally {
isCheckingPackage.value = false
}
Comment on lines +86 to 116
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid misleading “not found” on network failures.
checkPackageExists returns false for any fetch error, so the “not found” message will also show on registry/network issues and the catch branch will effectively never run. Consider returning a richer status from the util, or make the message neutral.

💡 Possible message adjustment
-      packageError.value = `Package "${name}" was not found on npm.`
+      packageError.value = `Package "${name}" was not found or could not be verified.`
📝 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
async function handleKeydown(e: KeyboardEvent) {
if (e.key !== 'Enter' || !inputValue.value.trim() || isCheckingPackage.value) return
e.preventDefault()
const name = inputValue.value.trim()
if (packages.value.length >= maxPackages.value) return
if (packages.value.includes(name)) return
// If it matches a dropdown result, add immediately (already confirmed to exist)
const exactMatch = filteredResults.value.find(r => r.name === name)
if (exactMatch) {
addPackage(exactMatch.name)
return
}
// Otherwise, verify it exists on npm
isCheckingPackage.value = true
packageError.value = ''
try {
const exists = await checkPackageExists(name)
if (name !== inputValue.value.trim()) return // stale guard
if (exists) {
addPackage(name)
} else {
packageError.value = `Package "${name}" was not found on npm.`
}
} catch {
packageError.value = 'Could not verify package. Please try again.'
} finally {
isCheckingPackage.value = false
}
async function handleKeydown(e: KeyboardEvent) {
if (e.key !== 'Enter' || !inputValue.value.trim() || isCheckingPackage.value) return
e.preventDefault()
const name = inputValue.value.trim()
if (packages.value.length >= maxPackages.value) return
if (packages.value.includes(name)) return
// If it matches a dropdown result, add immediately (already confirmed to exist)
const exactMatch = filteredResults.value.find(r => r.name === name)
if (exactMatch) {
addPackage(exactMatch.name)
return
}
// Otherwise, verify it exists on npm
isCheckingPackage.value = true
packageError.value = ''
try {
const exists = await checkPackageExists(name)
if (name !== inputValue.value.trim()) return // stale guard
if (exists) {
addPackage(name)
} else {
packageError.value = `Package "${name}" was not found or could not be verified.`
}
} catch {
packageError.value = 'Could not verify package. Please try again.'
} finally {
isCheckingPackage.value = false
}
}

}

Expand Down Expand Up @@ -138,7 +171,8 @@ function handleBlur() {
class="absolute inset-y-0 start-3 flex items-center text-fg-subtle pointer-events-none group-focus-within:text-accent"
aria-hidden="true"
>
<span class="i-carbon:search w-4 h-4" />
<span v-if="isCheckingPackage" class="i-carbon:renew w-4 h-4 animate-spin" />
<span v-else class="i-carbon:search w-4 h-4" />
</span>
<input
id="package-search"
Expand All @@ -149,7 +183,8 @@ function handleBlur() {
? $t('compare.selector.search_first')
: $t('compare.selector.search_add')
"
class="w-full bg-bg-subtle border border-border rounded-lg ps-10 pe-4 py-2.5 font-mono text-sm text-fg placeholder:text-fg-subtle motion-reduce:transition-none duration-200 focus:border-accent focus-visible:(outline-2 outline-accent/70)"
:disabled="isCheckingPackage"
class="w-full bg-bg-subtle border border-border rounded-lg ps-10 pe-4 py-2.5 font-mono text-sm text-fg placeholder:text-fg-subtle motion-reduce:transition-none duration-200 focus:border-accent focus-visible:(outline-2 outline-accent/70) disabled:opacity-60 disabled:cursor-wait"
aria-autocomplete="list"
@focus="isInputFocused = true"
@blur="handleBlur"
Expand Down Expand Up @@ -205,6 +240,11 @@ function handleBlur() {
</button>
</div>
</Transition>

<!-- Package not found error -->
<p v-if="packageError" class="text-xs text-red-400 mt-1" role="alert">
{{ packageError }}
</p>
</div>

<!-- Hint -->
Expand Down
38 changes: 37 additions & 1 deletion test/nuxt/components/compare/PackageSelector.spec.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ref } from 'vue'
import { flushPromises } from '@vue/test-utils'
import { mountSuspended } from '@nuxt/test-utils/runtime'
import PackageSelector from '~/components/Compare/PackageSelector.vue'

// Mock checkPackageExists
vi.mock('~/utils/package-name', () => ({
checkPackageExists: vi.fn(),
}))

import { checkPackageExists } from '~/utils/package-name'
const mockCheckPackageExists = vi.mocked(checkPackageExists)

// Mock $fetch for useNpmSearch
const mockFetch = vi.fn()
vi.stubGlobal('$fetch', mockFetch)

describe('PackageSelector', () => {
beforeEach(() => {
mockFetch.mockReset()
mockCheckPackageExists.mockReset()
mockFetch.mockResolvedValue({
objects: [
{ package: { name: 'lodash', description: 'Lodash modular utilities' } },
Expand All @@ -18,6 +28,7 @@ describe('PackageSelector', () => {
total: 2,
time: new Date().toISOString(),
})
mockCheckPackageExists.mockResolvedValue(true)
})

describe('selected packages display', () => {
Expand Down Expand Up @@ -132,7 +143,9 @@ describe('PackageSelector', () => {
})

describe('adding packages', () => {
it('adds package on Enter key', async () => {
it('adds package on Enter key when package exists', async () => {
mockCheckPackageExists.mockResolvedValue(true)

const component = await mountSuspended(PackageSelector, {
props: {
modelValue: [],
Expand All @@ -142,13 +155,16 @@ describe('PackageSelector', () => {
const input = component.find('input')
await input.setValue('my-package')
await input.trigger('keydown', { key: 'Enter' })
await flushPromises()

const emitted = component.emitted('update:modelValue')
expect(emitted).toBeTruthy()
expect(emitted![0]![0]).toEqual(['my-package'])
})

it('clears input after adding package', async () => {
mockCheckPackageExists.mockResolvedValue(true)

const component = await mountSuspended(PackageSelector, {
props: {
modelValue: [],
Expand All @@ -158,11 +174,31 @@ describe('PackageSelector', () => {
const input = component.find('input')
await input.setValue('my-package')
await input.trigger('keydown', { key: 'Enter' })
await flushPromises()

// Input should be cleared
expect((input.element as HTMLInputElement).value).toBe('')
})

it('does not add non-existent packages', async () => {
mockCheckPackageExists.mockResolvedValue(false)

const component = await mountSuspended(PackageSelector, {
props: {
modelValue: [],
},
})

const input = component.find('input')
await input.setValue('nonexistent-pkg')
await input.trigger('keydown', { key: 'Enter' })
await flushPromises()

const emitted = component.emitted('update:modelValue')
expect(emitted).toBeFalsy()
expect(component.find('[role="alert"]').exists()).toBe(true)
})

it('does not add duplicate packages', async () => {
const component = await mountSuspended(PackageSelector, {
props: {
Expand Down
Loading