feat: "I'm feeling lucky" search redirection with bang (!) ending#1552
feat: "I'm feeling lucky" search redirection with bang (!) ending#1552shuuji3 wants to merge 3 commits intonpmx-dev:mainfrom
!) ending#1552Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThe PR adds "I'm Feeling Lucky" behaviour to search: queries are trimmed and a trailing exclamation mark is removed for matching. A watcher on displayResults (now immediate) and Enter key handling were updated to detect queries ending with "!" (feeling-lucky) and, when there is an exact first-result match, navigate directly to that package; otherwise normal search results are shown. A pendingEnterQuery flow ensures consistent trimmed/cleaned values for future navigation. An end-to-end test file verifies redirect-on- Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (1)
test/e2e/search-feeling-lucky.spec.ts (1)
3-13: Add a test case for the bang query with no exact match (e.g.,nuxt-no!).The PR description explicitly lists
https://npmx.dev/search?q=nuxt-no!as expected to show normal search results (no redirect). This is the key edge-case that distinguishes the feature from a naive "strip!and always redirect" implementation, and it's currently untested.Suggested additional test
test('normal search query (without "!") should not redirect', async ({ page, goto }) => { await goto('/search?q=nuxt', { waitUntil: 'hydration' }) await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({ timeout: 15000 }) await expect(page).toHaveURL(/\/search\?q=nuxt/) }) + + test('query ending with "!" but no exact match should show search results', async ({ page, goto }) => { + await goto('/search?q=nuxt-no!', { waitUntil: 'hydration' }) + await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({ timeout: 15000 }) + await expect(page).toHaveURL(/\/search\?q=nuxt-no!/) + }) })
| // Watch for results to navigate when Enter was pressed before results arrived, | ||
| // or for "I'm feeling lucky" redirection when the query ends with "!" and there is the exact match. | ||
| watch( | ||
| displayResults, | ||
| results => { | ||
| const rawQuery = normalizeSearchParam(route.query.q) | ||
| const isFeelingLucky = rawQuery.endsWith('!') | ||
|
|
||
| // Check if input is still focused (user hasn't started navigating or clicked elsewhere) | ||
| if (document.activeElement?.tagName !== 'INPUT') { | ||
| pendingEnterQuery.value = null | ||
| return | ||
| } | ||
| if (!pendingEnterQuery.value && !isFeelingLucky) return | ||
|
|
||
| // Navigate if first result matches the query that was entered | ||
| const firstResult = results[0] | ||
| // eslint-disable-next-line no-console | ||
| console.log('[search] watcher fired', { | ||
| pending: pendingEnterQuery.value, | ||
| firstResult: firstResult?.package.name, | ||
| }) | ||
| if (firstResult?.package.name === pendingEnterQuery.value) { | ||
| pendingEnterQuery.value = null | ||
| navigateToPackage(firstResult.package.name) | ||
| } | ||
| }) | ||
| // For manual Enter, check if input is still focused (user hasn't started navigating or clicked elsewhere) | ||
| if (pendingEnterQuery.value && document.activeElement?.tagName !== 'INPUT') { | ||
| pendingEnterQuery.value = null | ||
| return | ||
| } | ||
|
|
||
| const target = pendingEnterQuery.value || rawQuery.replace(/!$/, '') | ||
| if (!target) return | ||
|
|
||
| // Navigate if first result matches the target query | ||
| const firstResult = results[0] | ||
| if (firstResult?.package.name === target) { | ||
| pendingEnterQuery.value = null | ||
| navigateToPackage(firstResult.package.name) | ||
| } | ||
| }, | ||
| { immediate: true }, | ||
| ) |
There was a problem hiding this comment.
Case-sensitive comparison may prevent redirect for mixed-case input.
Line 448 performs a strict === comparison between firstResult?.package.name and target. However, npm package names are case-insensitive, and the visibleResults reordering logic (line 89) already uses .toLowerCase() when finding exact matches. If a user types Nuxt!, target becomes "Nuxt" while the actual package name is "nuxt", so the redirect silently fails.
The same issue exists in the Enter-key handler at line 467.
Suggested fix
- const target = pendingEnterQuery.value || rawQuery.replace(/!$/, '')
+ const target = (pendingEnterQuery.value || rawQuery.replace(/!$/, '')).toLowerCase()
if (!target) return
// Navigate if first result matches the target query
const firstResult = results[0]
- if (firstResult?.package.name === target) {
+ if (firstResult?.package.name.toLowerCase() === target) {And in the Enter-key handler:
- const cleanedInputValue = inputValue.replace(/!$/, '')
+ const cleanedInputValue = inputValue.replace(/!$/, '').toLowerCase()
// Check if first result matches the input value exactly
const firstResult = displayResults.value[0]
- if (firstResult?.package.name === cleanedInputValue) {
+ if (firstResult?.package.name.toLowerCase() === cleanedInputValue) {
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
resolve #996