feat: add markdown output support for package pages #151
Conversation
|
@BYK is attempting to deploy a commit to the danielroe Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
ultimately it might make sense to share some logic with the client (see #169) so we can click 'copy as markdown' ...although that button could also fetch the markdown from the server endpoint, which might be better from a JS bundle size so... just linking so you're aware |
|
@atinux @okineadev are you waiting on me for something? (just making sure I'm not the blocker here) |
|
Sorry I was quite busy. I suggest to not use a middleware here but instead having a server route with Then like on https://github.com/unjs/undocs/blob/main/app/modules/md-rewrite.ts, we can add the rewrite in the generated Would you be up to update your PR with this approach @BYK ? |
Add support for getting package information in Markdown format via: - URL suffix: /package-name.md - Accept header: text/markdown Includes security hardening: - URL validation for homepage/bugs links (prevents javascript: injection) - README size limit (500KB) to prevent DoS - Path exclusion alignment between Vercel rewrites and middleware
Address review feedback from @atinux: - Replace middleware with server route at /raw/[...slug].md - Update vercel.json rewrites to point to /raw/:path.md - Add curl user-agent rewrite support for CLI tools - Enable CDN-level caching for markdown responses Also fix maintainer URLs to use npmx.dev instead of npmjs.com
|
@atinux no worries. Updated :) |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds server-side support to serve package information as Markdown via a new /raw/** route and a server route handler that parses slugs, resolves package and version data, fetches README with a jsDelivr fallback, obtains weekly and 12‑week download stats and repository info, and returns generated Markdown with Content-Type text/markdown and ISR-aligned caching (60s). Adds server-side markdown generation utilities (sparkline, formatting, URL normalisation, README handling), an ISR route rule for /raw/** in nuxt.config, a markdown alternate link on the package page, Vercel rewrites for markdown/curl requests, unit tests for markdown utilities, and a codecov ignore update. Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 3
🧹 Nitpick comments (3)
server/routes/raw/[...slug].md.get.ts (1)
130-235: Consider splitting the handler into smaller helpers.This handler is well over the 50-line guideline; extracting readme, downloads, and version resolution into helpers would improve readability and testability.
As per coding guidelines: Keep functions focused and manageable (generally under 50 lines).
server/utils/markdown.ts (2)
8-27: Guard the sparkline index access.Clamping or fallbacking the array access keeps the indexing explicitly safe.
As per coding guidelines: Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index.🛠️ Safer index access
- const index = Math.round(normalized * (SPARKLINE_CHARS.length - 1)) - return SPARKLINE_CHARS[index] + const index = Math.round(normalized * (SPARKLINE_CHARS.length - 1)) + return SPARKLINE_CHARS[index] ?? SPARKLINE_CHARS[SPARKLINE_CHARS.length - 1]
115-279: Consider extracting sections to reduce function length.
generatePackageMarkdownis very long; splitting into section builders (stats, links, readme) would improve readability and maintenance.As per coding guidelines: Keep functions focused and manageable (generally under 50 lines).
|
Addressed the sparkline feedback - now showing both numeric trend data and visual sparkline: This provides both LLM-friendly numeric data and human-readable visual trend. |
- Fix Cache-Control mismatch: change from 3600s to 60s to match ISR TTL - Add URL validation after normalizeGitUrl() to skip non-HTTP URLs - Fix installSize check: use !== undefined instead of truthiness - Enhance sparkline with numeric trend values for LLM readability - Add comprehensive unit tests for markdown utils (97.97% coverage)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/routes/raw/[...slug].md.get.ts (1)
31-44: Guard jsDelivr README fetches with a size cap.The 500KB limit is applied after the body is fully downloaded, so very large READMEs still consume memory and bandwidth. Consider checking
content-length(when present) and truncating/short‑circuiting early.🛠️ Example improvement
for (const filename of readmeFilenames) { try { const url = `https://cdn.jsdelivr.net/npm/${packageName}${versionSuffix}/${filename}` + const maxReadmeBytes = 500 * 1024 const response = await fetch(url) if (response.ok) { - return await response.text() + const length = Number(response.headers.get('content-length') ?? 0) + if (length && length > maxReadmeBytes) { + continue + } + const text = await response.text() + return text.length > maxReadmeBytes ? text.slice(0, maxReadmeBytes) : text } } catch { // Try next filename } }
- Add index clamping in generateSparkline() to prevent floating-point edge cases - Update getRepositoryUrl() to handle string repository values (common in npm metadata) - Add unit tests for string repository handling
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
server/utils/markdown.ts (5)
31-45: Consider sharing formatting utilities with the client.These functions duplicate logic from
app/utils/formatters.ts. Per the PR discussion, danielroe suggested sharing logic to enable features like "copy as markdown" on the client. Consider moving these to a shared location (e.g.,shared/utils/formatters.ts) to maintain consistency and reduce duplication.Note: The implementations differ slightly (
formatCompactNumberhere usesIntl.NumberFormatwhilst the client version uses manual thresholds), so unification would require choosing one approach.
47-49: Consider expanding the escaped character set.The function escapes common Markdown formatting characters but omits others that could cause rendering issues in certain contexts (e.g.,
#at line start,>,|in tables). Since descriptions are placed in blockquotes, the current set may suffice, but be aware that multi-line descriptions containing these characters could render unexpectedly.
268-277: Consider a type guard for cleaner maintainer handling.The type assertion on line 270 works but could be replaced with a type guard for better type safety and readability.
♻️ Optional refactor with type guard
+interface NpmMaintainer { + name?: string + email?: string + username?: string +} + +function hasUsername(m: unknown): m is NpmMaintainer { + return typeof m === 'object' && m !== null && 'username' in m +} + // In the maintainers loop: - // npm API returns username but `@npm/types` Contact doesn't include it - const username = (maintainer as { username?: string }).username - const name = maintainer.name || username || 'Unknown' + const username = hasUsername(maintainer) ? maintainer.username : undefined + const name = maintainer.name || username || 'Unknown'
132-299: Consider breaking downgeneratePackageMarkdowninto smaller helpers.At ~167 lines, this function exceeds the 50-line guideline. The function is logically organised with clear section comments, but extracting helpers (e.g.,
buildMetaSection,buildStatsTable,buildLinksSection) would improve testability and maintainability.As per coding guidelines, Keep functions focused and manageable (generally under 50 lines).
51-58: Consider refactoring to use or align with the sharednormalizeGitUrlutility.The codebase has a more comprehensive
normalizeGitUrlfunction inshared/utils/git-providers.ts(lines 295-319) that handles a broader range of Git URL formats:
- Proper URL parsing for
ssh://andgit://protocols using the URL constructor- Generic SCP-style URL handling (
git@host:path) for any host, not just GitHub- Error handling with try-catch
- Input validation (empty string checks)
The current implementation in
server/utils/markdown.ts(lines 51-58) is GitHub-specific and lacks error handling. It cannot normalise non-GitHub SSH or SCP-style URLs correctly.Note: The shared utility returns
string | nullrather thanstring. If refactoring to use it, the calling code at line 82 must handle the nullable return value appropriately.
- Remove unused repoInfo parameter from PackageMarkdownOptions interface - Exclude app/pages and app/layouts from codecov patch coverage (these are tested by Playwright browser tests, not unit tests)
|
@atinux ready for another look? |
| "rewrites": [ | ||
| { | ||
| "source": "/:path((?!api|_nuxt|_v|__nuxt|search|code|raw/).*)", | ||
| "has": [{ "type": "header", "key": "accept", "value": "(.*?)text/markdown(.*)" }], | ||
| "destination": "/raw/:path.md" | ||
| }, | ||
| { | ||
| "source": "/:path((?!api|_nuxt|_v|__nuxt|search|code|raw/).*)", | ||
| "has": [{ "type": "header", "key": "user-agent", "value": "curl/.*" }], | ||
| "destination": "/raw/:path.md" | ||
| } | ||
| ], |
There was a problem hiding this comment.
We also need support for package/* paths
$ curl -L https://npmxdev-git-fork-byk-byk-featmd-poetry.vercel.app/package/vite
A server error has occurred
FUNCTION_INVOCATION_FAILED
arn1::r7r5t-1770279708442-3169599c59e0
$ curl -L https://npmxdev-git-fork-byk-byk-featmd-poetry.vercel.app/vite
# vite
> Native-ESM powered web dev build tool
...
|
ah! we do now have a copy as markdown button on the frontend #1020 - if you rebase you could make any change you need to use your new logic? It would be nice if that button also fetched it dynamically |
| const username = (maintainer as { username?: string }).username | ||
| const name = maintainer.name || username || 'Unknown' | ||
| if (username) { | ||
| lines.push(`- [${name}](https://npmx.dev/~${username})`) |
There was a problem hiding this comment.
this may not be correct as maintainers is a user supplied field
Co-authored-by: Okinea Dev <[email protected]>
|
I think we should also take a look at #1382 |
5 participants
Summary
Add support for getting package information in Markdown format, useful for AI/LLM consumption and command-line tools.
Access Methods
/<package>.md(e.g.,/vue.md,/@nuxt/kit.md)Accept: text/markdownOutput Includes
Security Hardening
javascript:protocol injection)