feat: tools#3079
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 Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Tools area with a dependency statistics page. The feature parses uploaded package manifests, displays categorised dependencies and package statistics, and retrieves vulnerability and deprecation data through a validated server API. ChangesTools dependency statistics
Sequence Diagram(s)sequenceDiagram
participant Visitor
participant DependencyStatsPage
participant PackageJsonParser
participant DependencyList
participant HealthComposable
participant HealthApi
Visitor->>DependencyStatsPage: Upload package.json
DependencyStatsPage->>PackageJsonParser: Parse dependency text
PackageJsonParser-->>DependencyStatsPage: Parsed dependencies
DependencyStatsPage->>DependencyList: Render dependency groups
DependencyList->>HealthComposable: Request visible dependency health
HealthComposable->>HealthApi: POST dependency batch
HealthApi-->>HealthComposable: Vulnerability and deprecation results
HealthComposable-->>DependencyList: Update dependency indicators
DependencyList-->>DependencyStatsPage: Select dependency
DependencyStatsPage-->>Visitor: Render dependency statistics
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
server/api/registry/direct-deps-health.post.ts (1)
1-15: 🧹 Nitpick | 🔵 TrivialConsider rate limiting this endpoint.
Each request can trigger up to 50 npm registry fetches plus an OSV batch/detail query fan-out, with no visible per-caller rate limiting on this route. Since it's a public, unauthenticated endpoint, consider adding request throttling to avoid amplifying load onto the npm registry and OSV API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/registry/direct-deps-health.post.ts` around lines 1 - 15, Add request throttling to the unauthenticated `defineEventHandler` for the direct dependency health endpoint, limiting each caller before invoking `analyzeDirectDependencyHealth`. Reuse the project’s existing rate-limiting mechanism and preserve the current validation and analysis behavior for requests within the limit.app/components/DepsStats/DependencyList.vue (1)
86-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated
getVulnerableInfo(dep)/getDeprecatedInfo(dep)calls per row.Each is invoked up to 4 times in the template (
v-if,:to,:class,:title, plus thesr-onlyspan), each re-doing the object lookup and relying on repeated non-null assertions (!) instead of a single narrowed value.Consider computing
vulnerableInfo/deprecatedInfoonce per row (e.g. via a small wrapper computed keyed bydep.packageName, or destructure once at the top of the row template) to avoid duplicated lookups and non-null assertions.Also applies to: 234-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/DepsStats/DependencyList.vue` around lines 86 - 92, Update the dependency row rendering around getVulnerableInfo and getDeprecatedInfo to resolve each package’s vulnerability and deprecation data once per row, then reuse those narrowed values for v-if, links, classes, titles, and the sr-only content. Remove the repeated helper calls and non-null assertions while preserving the existing row behavior.app/pages/tools/index.vue (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$t()consistently in page script setup.Both pages bypass the project’s established Nuxt i18n global.
app/pages/tools/index.vue#L6-L6: removeuseI18n()and use$t(...).app/pages/tools/deps-stats.vue#L10-L10: removeuseI18n()and use$t(...).Based on learnings: pages must rely on auto-imported
$t()in<script setup>rather than destructuringtfromuseI18n().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/tools/index.vue` at line 6, Replace the destructured useI18n() translation setup with the auto-imported $t() in app/pages/tools/index.vue at lines 6-6 and app/pages/tools/deps-stats.vue at lines 10-10, updating each page’s translation calls accordingly and removing the unused composable import.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/DepsStats/DependencyList.vue`:
- Around line 179-184: Remove the focus-visible utility classes from the
button’s class attribute in the DependencyList selection button, while
preserving all other classes and behavior. Rely on the existing global button
focus-visible styling in main.css.
- Around line 40-62: Update the dependency aggregation around registryDeps,
orderedRegistryNames, and their consumers so duplicate package names retain each
dependency row’s category/alias-specific range instead of overwriting by
packageName. Use a unique key that preserves each parsed dependency identity,
and ensure outdatedDeps, replacementDeps, and health/requestHealth are derived
and ordered from those distinct entries while continuing to exclude nonRegistry
dependencies.
In `@app/components/DepsStats/PackageJsonUpload.vue`:
- Around line 64-80: Update the template in DepsStatsPackageJsonUpload so the
error message guarded by error is rendered regardless of whether fileName is
set. Preserve the selected filename and clear-selection controls, while ensuring
parse errors remain visible for invalid uploads.
In `@server/api/registry/direct-deps-health.post.ts`:
- Around line 12-15: Update the default handler around
DirectDepsHealthBodySchema parsing to catch ValiError from v.parse and throw
createError with statusCode 400 and the first issue message; leave
analyzeDirectDependencyHealth unchanged for valid bodies.
In `@shared/schemas/dependency-analysis.ts`:
- Around line 5-13: Update DirectDepsHealthBodySchema so the v.record value
schema applies a maximum-length constraint to each dependency range string
before analyzeDirectDependencyHealth() processes it. Preserve the existing
dependency-count check and use the established schema-validation approach for
bounding string length.
---
Nitpick comments:
In `@app/components/DepsStats/DependencyList.vue`:
- Around line 86-92: Update the dependency row rendering around
getVulnerableInfo and getDeprecatedInfo to resolve each package’s vulnerability
and deprecation data once per row, then reuse those narrowed values for v-if,
links, classes, titles, and the sr-only content. Remove the repeated helper
calls and non-null assertions while preserving the existing row behavior.
In `@app/pages/tools/index.vue`:
- Line 6: Replace the destructured useI18n() translation setup with the
auto-imported $t() in app/pages/tools/index.vue at lines 6-6 and
app/pages/tools/deps-stats.vue at lines 10-10, updating each page’s translation
calls accordingly and removing the unused composable import.
In `@server/api/registry/direct-deps-health.post.ts`:
- Around line 1-15: Add request throttling to the unauthenticated
`defineEventHandler` for the direct dependency health endpoint, limiting each
caller before invoking `analyzeDirectDependencyHealth`. Reuse the project’s
existing rate-limiting mechanism and preserve the current validation and
analysis behavior for requests within the limit.
🪄 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 Plus
Run ID: 459228d1-5f75-4568-8fa9-06b7a9a9c268
📒 Files selected for processing (23)
app/components/AppFooter.vueapp/components/DepsStats/DependencyList.vueapp/components/DepsStats/DependencyStats.vueapp/components/DepsStats/DependencyStatsPanel.vueapp/components/DepsStats/PackageJsonUpload.vueapp/composables/npm/useDirectDependencyHealth.tsapp/composables/useCommandPaletteGlobalCommands.tsapp/pages/tools/deps-stats.vueapp/pages/tools/index.vueapp/utils/parse-package-json-deps.tsdocs/content/2.guide/3.url-structure.mdi18n/locales/en.jsoni18n/schema.jsonnuxt.config.tsserver/api/registry/direct-deps-health.post.tsserver/middleware/canonical-redirects.global.tsserver/utils/dependency-analysis.tsshared/schemas/dependency-analysis.tsshared/types/dependency-analysis.tsshared/utils/constants.tstest/nuxt/a11y.spec.tstest/nuxt/composables/use-direct-dependency-health.spec.tstest/unit/app/utils/parse-package-json-deps.spec.ts
| export default defineEventHandler(async (event): Promise<DirectDependencyHealthResult> => { | ||
| const body = v.parse(DirectDepsHealthBodySchema, await readBody(event)) | ||
| return await analyzeDirectDependencyHealth(body.dependencies) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other API routes handle valibot parse failures.
rg -n 'v\.parse\(' server/api -B3 -A5
fd 'error' server -t fRepository: npmx-dev/npmx.dev
Length of output: 15774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Error handler outline:"
ast-grep outline server/utils/error-handler.ts || true
echo
echo "Error handler content:"
cat -n server/utils/error-handler.ts
echo
echo "Other ValiError catch blocks:"
rg -n 'catch \(error\)|instanceof v\.ValiError|throw createError\(\{[\s\S]*?statusCode: 400' server/api -B2 -A6
echo
echo "Direct-deps-health relevant section:"
cat -n server/api/registry/direct-deps-health.post.tsRepository: npmx-dev/npmx.dev
Length of output: 17114
Map valibot parse failures to 400 instead of 404.
This route does not catch ValiError; when it propagates, the shared handleApiError maps valibot errors to statusCode: 404. For invalid request bodies, catch the validation error locally and throw a createError({ statusCode: 400, message: error.issues[0].message }) to match the expected bad-request semantics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/registry/direct-deps-health.post.ts` around lines 12 - 15, Update
the default handler around DirectDepsHealthBodySchema parsing to catch ValiError
from v.parse and throw createError with statusCode 400 and the first issue
message; leave analyzeDirectDependencyHealth unchanged for valid bodies.
Source: Coding guidelines
There was a problem hiding this comment.
Share more about the source of the proposals
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== Endpoint =='
cat -n server/api/registry/direct-deps-health.post.ts
echo
echo '== Error-handler outline =='
ast-grep outline server/utils/error-handler.ts --items all
echo
echo '== ValiError handling and validation patterns =='
rg -n -C 5 'ValiError|v\.parse\(|safeParse|statusCode:\s*400' server/utils/error-handler.ts server/apiLength of output: 40793
The Comment images show the npmx dark-theme Tools experience: two dependency-stat screens with a package.json input, dependency list, and package details for gray-matter and module-replacements; plus the /tools landing page with a deps stats card.
@alexdln, the proposal was based on static control-flow analysis in this PR—not an external advisory or a runtime report:
- This endpoint validates the request body with
v.parseatserver/api/registry/direct-deps-health.post.tsLine 13, with no local error handling. - A failed Valibot parse propagates as a
ValiError. The sharedhandleApiErrorimplementation explicitly detects that type inserver/utils/error-handler.tsLines 31–35 and turns it into:The source itself notes the rationale:statusCode: 404TODO: throwing 404 rather than 400 as it's cacheable - I compared that with existing endpoint-level validation behavior. For example,
server/api/atproto/bluesky-author-profiles.get.tscatchesv.ValiErrorand returns 400 for malformed client input.
So the proposal reflects the usual HTTP distinction that an invalid POST body is a 400 Bad Request, while the current shared fallback makes it a 404.
That said, the comment in the shared handler shows this is an intentional project-wide caching trade-off. If the intended policy is for malformed requests to use cacheable 404s, then this endpoint should follow that policy and my suggestion should be withdrawn rather than introducing a route-specific exception.
There was a problem hiding this comment.
This is really awesome :)
I wonder if we could somehow make the charts smaller so everything could fit without having to scroll. Maybe just reducing the charts height config could work, which would require to add a height prop (or a more general options object). Right now a chartHeight computed (which I just noticed is a bit legacy since we don't use TrendsChart.vue inside modals anymore), could be based on this height prop, defaulting to decent values for mobile & desktop. (?)
Also the tools page should be easier to find, it's so good^^
Other things:
- the uploaded package.json is not persisted when navigating (for example show more stats and back)
2 participants
🧭 Context
Added a new section - tools. We've become a service with big data, features and capabilities. I think the logical next step is to start adding development tools for all package-related issues.
One of these is the ability to quickly analyze your project's dependencies - to understand their status, relevance, and alternatives. So I added the deps-stats page.
📚 Description
Additional screenshot