feat: strip version info from the search query#1626
feat: strip version info from the search query#1626Codefoxdev wants to merge 6 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a parser and composable to extract package, scope, version and trailing text from search input. The search page now uses a version‑stripped query and parsed scope for fetches, exact‑match logic, no‑results payload and rendered list. Unit tests validate the parser. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchPage as "Search Page"
participant Composable as "useParsedSearchQuery"
participant Parser as "parseSearchQuery"
participant SearchHook as "useSearch"
participant PackageList
User->>SearchPage: enter raw query
SearchPage->>Composable: pass raw query
Composable->>Parser: parse raw query
Parser-->>Composable: parsed fields (name, scope, version, trailing)
Composable-->>SearchPage: versionStrippedQuery & packageScope
SearchPage->>SearchHook: fetch with versionStrippedQuery
SearchHook-->>SearchPage: search results
SearchPage->>PackageList: render with search-query=versionStrippedQuery
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Review rate limit: 6/8 reviews remaining, refill in 7 minutes and 47 seconds.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/pages/search.vue (2)
36-55: Tighten scope parsing and normalise trailing whitespace.The current regex allows whitespace inside the captured scope and
strippedQuerycan insert double spaces whentrailingalready starts with one. Tightening the character classes and trimming the trailing chunk keeps scopes valid and avoids noisy search strings.Suggested update
- const match = q.match(/^(?:@([^/]+)\/)?([^/@ ]+)(?:@([^ ]*))?(.*)/) + const match = q.match(/^(?:@([^/\s]+)\/)?([^/@\s]+)(?:@([^\s]*))?(.*)/) ... - const strippedQuery = `${name} ${trailing ?? ''}`.trim() + const strippedQuery = `${name}${trailing ? ` ${trailing.trimStart()}` : ''}`.trim()
216-216: Keep exact‑match handling aligned with the stripped search term.Now that search uses
strippedQuery, versioned inputs like[email protected]will fetch results fornuxt, but exact‑match boosting/highlighting and claim/availability checks still compare against the rawquery. Consider switching those comparisons toparsedQuery.value.name(orstrippedQuery) so the UI behaviour matches the search term.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| const parsedQuery = computed(() => { | ||
| const q = query.value.trim() | ||
| // Regex matches a (un)scoped package and optionally extracts versioning info using the following syntax: @scope/specifier@version | ||
| // It makes use of 4 capture groups to extract this info. | ||
| const match = q.match( | ||
| /^(?:@(?<scope>[^/]+)\/)?(?<specifier>[^/@ ]+)(?:@(?<version>[^ ]*))?(?<trailing>.*)/, | ||
| ) | ||
| if (!match) return { scope: null, name: q, version: null, strippedQuery: q } | ||
|
|
||
| const { scope, specifier, version, trailing } = match.groups ?? {} | ||
| // Reconstruct the query without the version info, essentially stripping the version data: | ||
| // anything directly after the @ for the version specifier is stripped. | ||
| const name = scope ? `@${scope}/${specifier}` : (specifier ?? '') | ||
| const strippedQuery = `${name} ${trailing ?? ''}`.trim() | ||
|
|
||
| return { scope: scope ?? null, name: name, version: version || null, strippedQuery } | ||
| }) |
There was a problem hiding this comment.
Might be a good idea to extract this into a util and add a unit test for it
There was a problem hiding this comment.
I have extracted this into a util file and added some basic unit tests in the new commits
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/search.vue`:
- Around line 42-44: The computed versionStrippedQuery is adding an extra space
because it inserts a literal space before queryTrailing even though
queryTrailing already contains its leading whitespace; update the template in
versionStrippedQuery to concatenate packageName.value and queryTrailing.value
without the extra literal space (e.g. `${packageName.value}${queryTrailing.value
?? ''}`) so you don't produce double spaces, and keep the .trim() only if you
still need to guard against accidental surrounding whitespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2cc7a54c-3f24-4bda-aa85-92ad8f0fd45e
📒 Files selected for processing (4)
app/composables/useParsedSearchQuery.tsapp/pages/search.vueapp/utils/search.tstest/unit/app/utils/search.spec.ts
|
There are maybe some places where this util can be used instead of existing implementations. |
| } = useParsedSearchQuery(query) | ||
|
|
||
| const versionStrippedQuery = computed(() => | ||
| `${packageName.value} ${queryTrailing.value ?? ''}`.trim(), |
There was a problem hiding this comment.
Looking at the tests, this probably doesn't need an extra space since the queryTrailing seems to have it
| `${packageName.value} ${queryTrailing.value ?? ''}`.trim(), | |
| `${packageName.value}${queryTrailing.value ?? ''}`.trim(), |
|
@gameroman Should I also update the similar existing implementations to use this new method? |
🔗 Linked issue
Partially addresses #1416
🧭 Context
Currently, when searching for a package along with version info (e.g. when copying a package name from the output of a package manager) it just returns random results, instead of useful ones. (e.g. https://npmx.dev/[email protected])
This pr strips the version info from the query to improve search results.
📚 Description
This pr implements a new regex query to extract essential date from the search query on the search page, such as package scope (which replaces an existing method), package specifier and (optionally) version info. The query is then reconstructed without version info to support the existing search api. It can currently parse a package name in the following format:
An object containing the separate parts of the query is exposed as the
parsedQueryref and it is able to be used for other features. (Such as implementing the other part of the linked issue)