feat: endpoint to embed downloads chart static svg#2833
Open
graphieros wants to merge 24 commits into
Open
Conversation
3 tasks
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
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 a cached server SVG endpoint for downloads trends, extracts shared trends-chart builders and download-range utilities, refactors the client TrendsChart to use shared builders (adds embed UI), introduces date/i18n updates and a dependency bump, and adds comprehensive unit tests. ChangesEmbed downloads chart endpoint
Sequence DiagramsequenceDiagram
participant Client
participant Embed as /embed/downloads.svg
participant Evolution as fetchDownloadsEvolution
participant ChartData as buildTrendsChartData
participant ChartConfig as buildTrendsChartConfig
participant Render as createStaticVueUiXy
Client->>Embed: GET ?packages=tinyglobby&granularity=week&dark=true
Embed->>Embed: Parse & validate query params
Embed->>Evolution: fetchDownloadsEvolution(packages, options)
Evolution->>Evolution: Resolve date range per granularity
Evolution->>Evolution: Fetch NPM downloads (chunked if needed)
Evolution-->>Embed: EvolutionData[]
Embed->>ChartData: buildTrendsChartData(evolution, options)
ChartData-->>Embed: VueUiXyDatasetItem[] + dates
Embed->>ChartConfig: buildTrendsChartConfig(dataset, options)
ChartConfig-->>Embed: VueUiXyConfig
Embed->>Render: createStaticVueUiXy(config, data)
Render-->>Embed: SVG string
Embed->>Embed: Add value labels + watermark
Embed-->>Client: SVG (image/svg+xml, cached)
Possibly related PRs
Suggested reviewers
🚥 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)
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! |
e18e dependency analysisNo dependency warnings found. |
…ub.com/npmx-dev/npmx.dev into 2811-endpoint-to-embed-downloads-chart
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
server/utils/download-evolution.ts (1)
100-110: ⚡ Quick winNo timeout or error handling on the upstream npm fetch.
$fetchhere has no timeout and no error handling. A slow or failing upstream (or a 404 for a non-existent package) will reject, and since the endpoint awaits these viaPromise.all, one bad package fails the entire embed request as an unhandled 500. Consider a per-request timeout and graceful degradation (e.g. treat a failed package as empty series rather than failing the whole chart).🤖 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/utils/download-evolution.ts` around lines 100 - 110, fetchNpmDownloadsRangeServer currently calls $fetch without timeout or error handling, so a slow/failed/404 upstream will reject and break callers; wrap the $fetch call in a try/catch inside fetchNpmDownloadsRangeServer and apply a per-request timeout (e.g., AbortController or $fetch timeout option) so the request is aborted if slow, and on any error/404 return a safe fallback (an empty NpmDownloadsRangeResponse or zeroed series) instead of throwing so callers (e.g., Promise.all consumers) can continue gracefully. Ensure you update the implementation referenced by fetchNpmDownloadsRangeServer and keep the returned shape consistent with NpmDownloadsRangeResponse when returning the fallback.test/unit/shared/utils/trends-chart.spec.ts (1)
261-261: ⚡ Quick winAvoid
as nevercasts in tests; they bypass the strict typing this repo asks for.These casts silence meaningful type checks and can hide API regressions in shared utilities. Prefer
satisfies, explicit union fixtures, or narrowly typed helpers instead.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".
Also applies to: 468-468, 542-542
🤖 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 `@test/unit/shared/utils/trends-chart.spec.ts` at line 261, Replace the unsafe "fgSubtle: undefined as never" cast on the test fixture object (the colors object and any similar occurrences) with a strictly typed approach: either declare the fixture as a Partial or exact narrowed type (e.g., const colorsFixture: Partial<ColorPalette> = { fgSubtle: undefined }), use the "satisfies" operator to ensure the object conforms to the expected type, or provide an explicit union/optional type for fgSubtle; update all occurrences (including the other noted lines) so no "as never" casts are used and TypeScript will enforce correct types for fgSubtle and related properties.
🤖 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/Package/TrendsChart.vue`:
- Around line 20-25: The component calls getTrendsDatetimeFormatterOptions but
it isn't imported; add an explicit import for getTrendsDatetimeFormatterOptions
from '`#shared/utils/trends-chart`' alongside buildNormalisedTrendsDataset,
buildTrendsChartConfig, buildTrendsChartData, and isWeeklyDataset so the SFC
compiles and the call to getTrendsDatetimeFormatterOptions (used around lines
~163-165) resolves correctly.
- Around line 1958-1978: The toggle button should be a proper disclosure
control: add aria-expanded bound to showEmbedFields and aria-controls pointing
to the region id "trends-embed-chart" on the button (the same element that
currently uses `@click` and showEmbedFields), and ensure the controlled region
(`#trends-embed-chart`) keeps aria-hidden bound to !showEmbedFields; this exposes
the expanded state and relationship to assistive tech while keeping the existing
showEmbedFields logic intact.
- Around line 1995-1998: The aria-label on the copy button uses the readme
translation key ($t('package.readme.copy_as_markdown')) which is misleading for
the embed URL action; update the aria-label logic around copiedEmbedUrl to use
embed-specific translation keys (e.g., a "copy embed URL" and "copied" key)
instead of the README copy key, and add corresponding i18n strings so the
accessible label correctly reflects the "copy embed URL" action while keeping
:aria-pressed="copiedEmbedUrl".
In `@test/unit/server/utils/download-evolution.spec.ts`:
- Around line 23-43: Add a teardown afterEach to restore fake timers and global
stubs to avoid leaking state from beforeEach; specifically, after the existing
beforeEach add an afterEach that calls vi.useRealTimers() and
vi.unstubAllGlobals() (and optionally vi.restoreAllMocks() or vi.clearAllMocks()
if you want to reset mocks), referencing the existing
beforeEach/vi.useFakeTimers and vi.stubGlobal('$fetch') so the tests revert
timers and the $fetch stub between tests.
---
Nitpick comments:
In `@server/utils/download-evolution.ts`:
- Around line 100-110: fetchNpmDownloadsRangeServer currently calls $fetch
without timeout or error handling, so a slow/failed/404 upstream will reject and
break callers; wrap the $fetch call in a try/catch inside
fetchNpmDownloadsRangeServer and apply a per-request timeout (e.g.,
AbortController or $fetch timeout option) so the request is aborted if slow, and
on any error/404 return a safe fallback (an empty NpmDownloadsRangeResponse or
zeroed series) instead of throwing so callers (e.g., Promise.all consumers) can
continue gracefully. Ensure you update the implementation referenced by
fetchNpmDownloadsRangeServer and keep the returned shape consistent with
NpmDownloadsRangeResponse when returning the fallback.
In `@test/unit/shared/utils/trends-chart.spec.ts`:
- Line 261: Replace the unsafe "fgSubtle: undefined as never" cast on the test
fixture object (the colors object and any similar occurrences) with a strictly
typed approach: either declare the fixture as a Partial or exact narrowed type
(e.g., const colorsFixture: Partial<ColorPalette> = { fgSubtle: undefined }),
use the "satisfies" operator to ensure the object conforms to the expected type,
or provide an explicit union/optional type for fgSubtle; update all occurrences
(including the other noted lines) so no "as never" casts are used and TypeScript
will enforce correct types for fgSubtle and related properties.
🪄 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: 13b5a881-235a-4f7f-8c83-2aada6b422f0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
app/components/Package/TrendsChart.vueapp/composables/useChartWatermark.tsapp/composables/useCharts.tsapp/utils/date.tsi18n/locales/en.jsoni18n/locales/fr-FR.jsoni18n/schema.jsonpackage.jsonserver/api/embed/downloads.svg/index.get.tsserver/utils/download-evolution.tsshared/utils/download-ranges.tsshared/utils/embed-chart-colors.tsshared/utils/trends-chart.tstest/unit/app/utils/date.spec.tstest/unit/server/api/embed/downloads.svg/index.get.spec.tstest/unit/server/utils/download-evolution.spec.tstest/unit/shared/utils/download-ranges.spec.tstest/unit/shared/utils/trends-chart.spec.ts
Contributor
Author
|
TS fails are related to |
ghostdevv
requested changes
Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #2811 for the embedding aspect of the issue.
This creates an endpoint that generates a static svg for download charts, from:
Important note regarding the data:
Refactoring:
Rendering:
Other: