Skip to content

Comments

fix: fix fallback not triggered for 0 measuredWidth#1584

Merged
danielroe merged 1 commit intomainfrom
improved-fallback
Feb 22, 2026
Merged

fix: fix fallback not triggered for 0 measuredWidth#1584
danielroe merged 1 commit intomainfrom
improved-fallback

Conversation

@wojtekmaj
Copy link
Contributor

@wojtekmaj wojtekmaj commented Feb 22, 2026

Currently fallback does not work: https://npmx.dev/api/registry/badge/version/nuxt?label=thisisatest

with this PR, fallback still sucks but at least does not appear very broken:

https://npmxdev-git-improved-fallback-npmx.vercel.app/api/registry/badge/version/nuxt?label=thisisatest

Copilot AI review requested due to automatic review settings February 22, 2026 20:23
@vercel
Copy link

vercel bot commented Feb 22, 2026

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 22, 2026 8:25pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 22, 2026 8:25pm
npmx-lunaria Ignored Ignored Feb 22, 2026 8:25pm

Request Review

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates badge SVG text measurement logic so the character-count fallback is triggered when the canvas text measurement returns 0, preventing badges from rendering with an incorrectly small width in that scenario.

Changes:

  • Treat measuredWidth === 0 as an invalid measurement by requiring Number.isFinite(measuredWidth) && measuredWidth > 0 before using it.

Comment on lines +72 to 73
if (Number.isFinite(measuredWidth) && measuredWidth > 0) {
return Math.ceil(measuredWidth)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

This change adjusts the fallback behavior when measureText() returns 0, but there’s no regression test covering the measuredWidth === 0 case (which is the bug being fixed). Consider adding a Vitest test that mocks @napi-rs/canvas (or getCanvasContext()) to return width: 0 for a non-empty string and asserts that the computed badge/text width uses the character-count fallback (i.e., grows beyond the minimum width for longer strings).

Copilot generated this review using guidance from repository custom instructions.
@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

This PR modifies the validation logic in the measureTextWidth function within the badge handler. The conditional check is tightened to verify that measuredWidth is both finite and positive (Number.isFinite(measuredWidth) && measuredWidth > 0) before returning the ceiling value. Previously, the function would return ceil(measuredWidth) as long as it was not NaN. The updated logic now explicitly excludes Infinity, negative Infinity, and non-positive values, with potential null return otherwise.

Possibly related PRs

Suggested reviewers

  • danielroe
  • ghostdevv
🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description is empty except for template content; the actual changes are not described by the author, only by the AI summary. Provide a meaningful description explaining the bug fix, why measuredWidth of 0 requires a fallback, and any relevant test coverage or edge cases addressed.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improved-fallback

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

@danielroe danielroe added this pull request to the merge queue Feb 22, 2026
Merged via the queue into main with commit d78ca75 Feb 22, 2026
25 checks passed
@danielroe danielroe deleted the improved-fallback branch February 22, 2026 22:14
Sign up for free to 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.

2 participants