fix: use lookup table for badges char widths when canvas not available#2487
fix: use lookup table for badges char widths when canvas not available#2487t128n wants to merge 2 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughReplaced heuristic character-width estimation using character sets and fallback width buckets with static per-character lookup tables ( Changes
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Text is usually measured using createCanvasContext. On Vercel, createCanvasContext can’t be created (returns null, likely due to missing native node.js addons in the serveless runtime), so it falls back to estimateTextWidth. estimateTextWidth makes a best guess for characters’ widths and has edge cases with wider characters like So Canvas not available was referring to the createCanvasContext (or what its name is - currently on mobile and it's hard to look outside the PRs boundary to find the imports). |
ah but that's always been true right? like that fact hasn't changed in this PR? also that's why we have the napi-rs canvas? or no? |
|
Yeah, my assumptions were flawed - I thought it would be about the whole canvas not being available, but I made some further tests real quick. So, function measureTextWidth(text: string, font: string): number | null {
const context = getCanvasContext()
if (context) {
context.font = font
const measuredWidth = context.measureText(text).width
if (Number.isFinite(measuredWidth) && measuredWidth > 0) { // Returns false as font not available and nothing can be measured
return Math.ceil(measuredWidth)
}
}
return null // yields null for canvas context and then falls back to estimateTextWidth
}So it's rather: |
🔗 Linked issue
Resolves #1601
🧭 Context
In production, measuring text with canvas is crashing, resulting in inaccurate fallback measurements:
Introduced a character lookup table of "manually" measured character widths for more
accurate lookups.
📚 Description
Tweaked the
estimateTextWidthfunction to use a on-character level lookup table for more accurate results when the Canvas API from@napi-rs/canvasis unavailable.Lookup table derived from running
locally, where fonts and the API are available.
Using a generous fallback for characters that aren't in the map, such as emojis.
📷 Comparison
Overflow / Long Labels
Shields.io Style (
?style=shieldsio)Emojis
CJK Characters
Special & Punctuation Characters
iiiii)WWWWW)Badge Types Smoke Test
versionlicensedownloadssizetypesdeprecated