fix: prevent adding non-existent packages#1208
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe PackageSelector component now validates package existence before adding on Enter: if the typed name exactly matches a dropdown item it is added immediately, otherwise an async check (checkPackageExists) runs. New UI state properties track verification progress ( Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| 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 | ||
| } |
There was a problem hiding this comment.
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.
| 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 | |
| } | |
| } |
|
looks like this was just a second behind #1207 is there anything in this pr that you'd recommend keeping? if so, would you resolve conflicts? 🙏 |
| const dir = glob(argvFiles.length > 0 ? argvFiles : '**/*.vue', { | ||
| withFileTypes: true, | ||
| cwd: APP_DIRECTORY, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Examine the full unocss-checker.ts to understand argument handling
echo "=== Full unocss-checker.ts content ==="
cat scripts/unocss-checker.ts
echo -e "\n=== Checking for APP_DIRECTORY definition ==="
rg "APP_DIRECTORY" scripts/ --type ts -B2 -A2Repository: npmx-dev/npmx.dev
Length of output: 3519
🏁 Script executed:
#!/bin/bash
# Check lint-staged configuration and how the script is invoked
echo "=== lint-staged config in package.json ==="
cat package.json | grep -A10 "lint-staged"
echo -e "\n=== Scripts section in package.json ==="
cat package.json | grep -A20 '"scripts"'
echo -e "\n=== Check how lint:css is defined ==="
grep -r "lint:css" package.jsonRepository: npmx-dev/npmx.dev
Length of output: 1916
Fix the file path handling in unocss-checker.ts when invoked via lint-staged.
When lint-staged runs pnpm lint:css, it passes staged file paths as arguments relative to the repository root (e.g., app/components/Foo.vue). However, the glob is executed with cwd: APP_DIRECTORY (an absolute path pointing to the app directory), causing a mismatch. The script attempts to locate files at paths like <APP_DIRECTORY>/app/components/Foo.vue instead of <APP_DIRECTORY>/components/Foo.vue, which will fail.
Either strip the app/ prefix from paths passed by lint-staged, or adjust the cwd and path handling logic.
|
@danielroe , pls see this. This is fresh pr #1218 |
4 participants
resolves #1198