Skip to content

chore: add generic type annotations to chart components - #1615

Merged
graphieros merged 7 commits into
npmx-dev:mainfrom
graphieros:main
Feb 24, 2026
Merged

chore: add generic type annotations to chart components#1615
graphieros merged 7 commits into
npmx-dev:mainfrom
graphieros:main

Conversation

@graphieros

@graphierosgraphieros commented Feb 23, 2026

Copy link
Copy Markdown
Member

Resolves#1606

  • Bump vue-data-ui to 3.15.6
  • Use vue-data-ui generic type annotations on all charts
  • Remove some now useless type defs & casts that compensated missing generic types

This also fixes a remaining CLS when opening the versions modal, with other improvements:

  • remove translateY transition
  • add overlay during loading state

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
npmx.devReadyReadyPreview, CommentFeb 23, 2026 10:24pm
2 Skipped Deployments
ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devIgnoredIgnoredPreviewFeb 23, 2026 10:24pm
npmx-lunariaIgnoredIgnoredFeb 23, 2026 10:24pm

Request Review

@coderabbitai

coderabbitaiBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed0e384 and c669cc8.

📒 Files selected for processing (1)
  • app/components/Package/TrendsChart.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Package/TrendsChart.vue

📝 Walkthrough

Walkthrough

Adds generic type annotations to chart configuration computed properties (VueUiXyConfig / VueUiSparklineConfig) across TrendsChart, VersionDistribution and WeeklyDownloadStats; tightens and adjusts imports from vue-data-ui; adds runtime guards for export callbacks (img/csv/svg); changes default theme handling and chart sizing/overlay; removes chart-container entry animation; adjusts a modal aspect-ratio; and bumps vue-data-ui from 3.15.5 to 3.15.6.

Possibly related PRs

Suggested labels

front

Suggested reviewers

  • danielroe
  • shuuji3
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description check✅ PassedThe PR description accurately describes the changeset, including the vue-data-ui bump, generic type annotations, removal of compensating type definitions, and CLS fixes.
Linked Issues check✅ PassedAll objectives from #1606 are met: vue-data-ui bumped to 3.15.6, generic type annotations added to chart configs, type defs removed, and guards added to export callbacks.
Out of Scope Changes check✅ PassedAll changes are within scope of #1606 objectives. The aspect-ratio adjustment in Versions.vue and runtime coercions in WeeklyDownloadStats.vue are necessary accompanying changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Package/WeeklyDownloadStats.vue (1)

234-239: ⚠️ Potential issue | 🟡 Minor

Avoid passing an undefined pulse colour.

Line 238 can still resolve to undefined before CSS variables are ready, so the chart may receive an invalid colour. A fallback keeps this safe.

💡 Suggested fix
- color: pulseColor.value!,+ color: pulseColor.value ?? OKLCH_NEUTRAL_FALLBACK,
🧹 Nitpick comments (1)
package.json (1)

113-113: Lockfile already reflects the 3.15.6 bump correctly.

The pnpm-lock.yaml is up to date with vue-data-ui@3.15.6. If needed, verify the release notes directly on the npm package page or GitHub repository for any breaking changes.


ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ce52be and 8a72913.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • app/components/Package/TrendsChart.vue
  • app/components/Package/VersionDistribution.vue
  • app/components/Package/Versions.vue
  • app/components/Package/WeeklyDownloadStats.vue
  • package.json

Comment threadapp/components/Package/TrendsChart.vue Outdated
Comment threadapp/components/Package/VersionDistribution.vue Outdated
graphierosand others added 2 commits February 23, 2026 23:05
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Package/TrendsChart.vue (1)

1518-1522: ⚠️ Potential issue | 🟡 Minor

Unsafe index access on non-array datapoint.

Line 1521 accesses datapoint[0] without verifying datapoint is an array, yielding undefined for plain objects and blanking the tooltip. This violates the coding guideline requiring strictly type-safe code.

Suggested fix
- const items = Array.isArray(datapoint) ? datapoint : [datapoint[0]]+ const items = Array.isArray(datapoint) ? datapoint : [datapoint]

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c731f28 and ed0e384.

📒 Files selected for processing (2)
  • app/components/Package/TrendsChart.vue
  • app/components/Package/VersionDistribution.vue

@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 15.38462% with 22 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing linesPatch %Lines
app/components/Package/VersionDistribution.vue0.00%9 Missing and 3 partials ⚠️
app/components/Package/TrendsChart.vue16.66%6 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@serhalpserhalp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@graphieros
graphieros added this pull request to the merge queueFeb 24, 2026
@serhalp

Copy link
Copy Markdown
Member

do the CLS fixes resolve #1342 or is that separate?

Merged via the queue into npmx-dev:main with commit f4943b9Feb 24, 2026
17 checks passed
@graphieros

Copy link
Copy Markdown
MemberAuthor

do the CLS fixes resolve #1342 or is that separate?

No, unfortunately I forgot this one

alex-key pushed a commit to alex-key/npmx.dev that referenced this pull request Feb 25, 2026
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add generic type annotations to chart components configs

2 participants

@graphieros@serhalp