style: revise the badge char width#908
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughThis change enhances the badge type route handler to support configurable character widths for text measurement. It introduces constants for default character width (7) and strategy-specific widths via a CHARS_WIDTH mapping, with an engines strategy using a width of 5.5. The measureTextWidth function is updated to accept an optional charWidth parameter that defaults to the standard width. The right text width calculation is modified to apply the appropriate strategy-specific width when available. 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)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| const leftWidth = measureTextWidth(finalLabel) | ||
| const rightWidth = measureTextWidth(finalValue) | ||
| const rightWidth = measureTextWidth( | ||
| finalValue, | ||
| CHARS_WIDTH[strategyKey as keyof typeof CHARS_WIDTH], | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n server/api/registry/badge/\[type\]/\[...\pkg\].get.ts | head -320 | tail -40Repository: npmx-dev/npmx.dev
Length of output: 2135
🏁 Script executed:
find . -type f -name "*.get.ts" | grep -E "badge.*pkg"Repository: npmx-dev/npmx.dev
Length of output: 111
🏁 Script executed:
fd -t f "pkg.*get.ts" -o -path "*badge*" -type f -name "*.get.ts"Repository: npmx-dev/npmx.dev
Length of output: 177
🏁 Script executed:
fd -t f ".get.ts" | grep badgeRepository: npmx-dev/npmx.dev
Length of output: 109
🏁 Script executed:
grep -n "CHARS_WIDTH" server/api/registry/badge/\[type\]/\[...\pkg\].get.tsRepository: npmx-dev/npmx.dev
Length of output: 151
🏁 Script executed:
grep -n "strategyKey" server/api/registry/badge/\[type\]/\[...\pkg\].get.ts | head -20Repository: npmx-dev/npmx.dev
Length of output: 305
🏁 Script executed:
sed -n '35,50p' server/api/registry/badge/\[type\]/\[...\pkg\].get.tsRepository: npmx-dev/npmx.dev
Length of output: 559
🏁 Script executed:
sed -n '1,50p' server/api/registry/badge/\[type\]/\[...\pkg\].get.ts | grep -E "import|const|type"Repository: npmx-dev/npmx.dev
Length of output: 1019
🏁 Script executed:
sed -n '270,285p' server/api/registry/badge/\[type\]/\[...\pkg\].get.tsRepository: npmx-dev/npmx.dev
Length of output: 871
🏁 Script executed:
grep -n "BadgeTypeSchema\|badgeStrategies" server/api/registry/badge/\[type\]/\[...\pkg\].get.ts | head -20Repository: npmx-dev/npmx.dev
Length of output: 346
🏁 Script executed:
sed -n '1,200p' server/api/registry/badge/\[type\]/\[...\pkg\].get.ts | grep -A 30 "badgeStrategies"Repository: npmx-dev/npmx.dev
Length of output: 1409
🏁 Script executed:
sed -n '121,200p' server/api/registry/badge/\[type\]/\[...\pkg\].get.tsRepository: npmx-dev/npmx.dev
Length of output: 3555
🏁 Script executed:
sed -n '35,45p' server/api/registry/badge/\[type\]/\[...\pkg\].get.tsRepository: npmx-dev/npmx.dev
Length of output: 350
Remove the unsafe type assertion when indexing CHARS_WIDTH.
strategyKey can be any badge strategy type ('version', 'license', 'size', etc.), but CHARS_WIDTH only defines 'engines'. The type assertion hides this mismatch; a type guard ensures safe access.
♻️ Suggested type-safe lookup
+function hasCharWidth(key: string): key is keyof typeof CHARS_WIDTH {
+ return Object.hasOwn(CHARS_WIDTH, key)
+}
+
- const rightWidth = measureTextWidth(
- finalValue,
- CHARS_WIDTH[strategyKey as keyof typeof CHARS_WIDTH],
- )
+ const charWidth = hasCharWidth(strategyKey) ? CHARS_WIDTH[strategyKey] : undefined
+ const rightWidth = measureTextWidth(finalValue, charWidth)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const leftWidth = measureTextWidth(finalLabel) | |
| const rightWidth = measureTextWidth(finalValue) | |
| const rightWidth = measureTextWidth( | |
| finalValue, | |
| CHARS_WIDTH[strategyKey as keyof typeof CHARS_WIDTH], | |
| ) | |
| function hasCharWidth(key: string): key is keyof typeof CHARS_WIDTH { | |
| return Object.hasOwn(CHARS_WIDTH, key) | |
| } | |
| const leftWidth = measureTextWidth(finalLabel) | |
| const charWidth = hasCharWidth(strategyKey) ? CHARS_WIDTH[strategyKey] : undefined | |
| const rightWidth = measureTextWidth(finalValue, charWidth) |
|
We could also measure actual text width: https://github.com/wojtekmaj/update-input-width/blob/main/src/index.ts#L3-L60 |
|
I've considered this, but SSR doesn't seem to support rendering canvas. |
https://npmx.dev/api/registry/badge/engines/vite
When configuring the engines badges via the API, I found that the calculated character width was too large and didn't look appropriate. Therefore, I adjusted the character width for the badge type (the characters for this type of badge are predictable and take up relatively little space).