fix: skip emoji conversion inside code blocks#2373
fix: skip emoji conversion inside code blocks#2373ffedex wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Hello! Thank you for opening your first PR to npmx, @ffedex! 🚀 Here’s what will happen next:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| const key = shortcode.slice(1, -1) as keyof typeof emojis | ||
| return emojis[key] || shortcode |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify TypeScript strictness settings and locate assertion-based lookup in this utility.
fd -i 'tsconfig*.json' --exec sh -c 'echo "=== {} ==="; cat "{}"' \;
echo
rg -n --type=ts -C2 'as keyof typeof emojis|const emojis\s*:\s*Record<string,\s*string>|emojis\[key\]' shared/utils/emoji.tsRepository: npmx-dev/npmx.dev
Length of output: 1366
🏁 Script executed:
# Check file size first
wc -l shared/utils/emoji.ts
# Read the area around the disputed lines and the emojis definition
head -n 100 shared/utils/emoji.ts | tail -n 50
# Also check the beginning to find the emojis object definition
head -n 50 shared/utils/emoji.tsRepository: npmx-dev/npmx.dev
Length of output: 2543
🏁 Script executed:
# Read the area around the disputed lines
sed -n '1900,1920p' shared/utils/emoji.ts
# Also check the end of the file to see if there's an export
tail -n 30 shared/utils/emoji.ts
# Check for how many entries are in the emojis object
grep -n "^const emojis\|^}" shared/utils/emoji.ts | head -n 5Repository: npmx-dev/npmx.dev
Length of output: 1512
Remove assertion-based key narrowing in emoji lookup to improve type safety.
Line 1914 uses as keyof typeof emojis, which unsafely asserts the key is valid. Instead, annotate the emojis object as Record<string, string> to allow string-indexed lookup without assertion. This approach is more permissive in the right way—string keys are valid on string-indexed maps—and aligns with strict TypeScript requirements.
Suggested refactor
-const emojis = {
+const emojis: Record<string, string> = {
'100': '💯',
'1234': '🔢',
...
}
@@
- const key = shortcode.slice(1, -1) as keyof typeof emojis
- return emojis[key] || shortcode
+ const key = shortcode.slice(1, -1)
+ return emojis[key] ?? shortcode
| const key = match.slice(1, -1) as keyof typeof emojis | ||
| return emojis[key] || match | ||
| }) | ||
| export function convertToEmoji(html: string): string { |
There was a problem hiding this comment.
this previously took in text, but now html - is this definitely the case everywhere? and if not I guess it doesn't matter?
2 participants
🔗 Linked issue
Fixes #2364
🧭 Context
Emoji shortcodes like
:1234:were being converted even inside code blocks in package READMEs.📚 Description
convertToEmojiwas running against the full HTML string, so:1234:in IPv6 addresses inside<code>and<pre>tags got turned into 🔢. Updated the function to match code/pre blocks first and skip them, only converting shortcodes in the rest of the content. Removed the now-unusedemojisKeysRegexvariable.