Refactor mrkdwn converters: extract shared utilities and emoji map#10
Refactor mrkdwn converters: extract shared utilities and emoji map#10
Conversation
…ted modules - Add utils.ts with escapeHtml and escapeMrkdwn shared helpers - Add emoji.ts with EMOJI_MAP constant and renderEmoji helper
…odules - Remove EMOJI_MAP and escapeHtml from mrkdwn-to-html.ts, import from shared modules - Remove escapeMrkdwn from markdown-to-mrkdwn.ts, import from utils.ts - Use renderEmoji helper for emoji replacement in formatInline - Re-export EMOJI_MAP from index.ts for external extensibility
📝 WalkthroughWalkthroughAdds emoji resolution and HTML rendering plus HTML/mrkdwn escaping utilities, refactors renderers to use these modules, updates package exports and a UI component, and adds a gemoji dependency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/mrkdwn/src/emoji.tspackages/mrkdwn/src/index.tspackages/mrkdwn/src/markdown-to-mrkdwn.tspackages/mrkdwn/src/mrkdwn-to-html.tspackages/mrkdwn/src/utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript code
No semicolons in code, enforced by Prettier
Use single quotes instead of double quotes
Use trailing commas in es5 format
Prefix unused variables with underscore (_)
Files:
packages/mrkdwn/src/emoji.tspackages/mrkdwn/src/index.tspackages/mrkdwn/src/markdown-to-mrkdwn.tspackages/mrkdwn/src/mrkdwn-to-html.tspackages/mrkdwn/src/utils.ts
packages/mrkdwn/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
Files:
packages/mrkdwn/src/emoji.tspackages/mrkdwn/src/index.tspackages/mrkdwn/src/markdown-to-mrkdwn.tspackages/mrkdwn/src/mrkdwn-to-html.tspackages/mrkdwn/src/utils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to packages/mrkdwn/src/**/*.ts : Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
📚 Learning: 2026-02-20T00:31:35.696Z
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to packages/mrkdwn/src/**/*.ts : Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
Applied to files:
packages/mrkdwn/src/emoji.tspackages/mrkdwn/src/index.tspackages/mrkdwn/src/markdown-to-mrkdwn.tspackages/mrkdwn/src/mrkdwn-to-html.tspackages/mrkdwn/src/utils.ts
📚 Learning: 2026-02-20T00:31:35.696Z
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to **/*.svelte : Use `{html}` in Svelte for mrkdwn rendering (ESLint rule disabled for this specific use case)
Applied to files:
packages/mrkdwn/src/index.tspackages/mrkdwn/src/markdown-to-mrkdwn.tspackages/mrkdwn/src/mrkdwn-to-html.tspackages/mrkdwn/src/utils.ts
🧬 Code graph analysis (1)
packages/mrkdwn/src/mrkdwn-to-html.ts (1)
packages/mrkdwn/src/emoji.ts (1)
renderEmoji(54-58)
🔇 Additional comments (5)
packages/mrkdwn/src/index.ts (1)
3-3: Re-export looks goodNo issues with this barrel export addition.
packages/mrkdwn/src/utils.ts (1)
1-12: Centralised escaping helpers look solidThe helpers are clear and keep escaping consistent across the converters.
packages/mrkdwn/src/markdown-to-mrkdwn.ts (1)
10-10: Import refactor is finePulling
escapeMrkdwnfrom the shared utils keeps behaviour unchanged.packages/mrkdwn/src/mrkdwn-to-html.ts (2)
7-12: Updated dependency notes and imports look fineClearer separation of emoji and escaping concerns.
73-75: Emoji rendering fallback is sensibleUnknown shortcodes remain visible as
:name:while known ones render cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/mrkdwn/src/emoji.ts`:
- Around line 1-48: The exported constant EMOJI_MAP violates camelCase naming;
rename the internal binding to a camelCase identifier (e.g., emojiMap:
Record<string,string>) containing the same mapping, update any internal
references to use emojiMap, and re-export it under the original public name to
preserve compatibility (export { emojiMap as EMOJI_MAP }); ensure the type
annotation and all emoji entries remain unchanged.
- Around line 54-57: renderEmoji currently interpolates user-provided
keys/values from EMOJI_MAP directly into HTML, allowing injection; fix it by
HTML-escaping both the emoji name and the emoji value before embedding them. Add
or reuse a small helper like escapeHtml(str) that replaces &, <, >, " and ' with
entities, then use escapeHtml(name) and escapeHtml(emoji) in the returned string
(including the s-emoji-code and s-emoji-big parts). Ensure renderEmoji still
returns null when EMOJI_MAP[name] is falsy and that the escaped strings are used
everywhere the raw name/value were previously interpolated.
Use gemoji (1,913 shortcodes) instead of our 47-entry static map. Add 5 Slack-specific aliases for shortcode mismatches.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mrkdwn/src/emoji.ts (1)
4-10: Align constant with camelCase naming convention.
SLACK_ALIASESuses SCREAMING_SNAKE_CASE, which conflicts with the project's camelCase naming guideline. Consider renaming toslackAliases.💡 Suggested adjustment
-const SLACK_ALIASES: Record<string, string> = { +const slackAliases: Record<string, string> = { thinking_face: 'thinking', party_popper: 'tada', person_with_pouting_face: 'pouting_face', person_frowning: 'frowning_person', person_with_blond_hair: 'blond_haired_person', }Then update the reference in
resolveEmoji:- return nameToEmoji[name] ?? nameToEmoji[SLACK_ALIASES[name] ?? ''] + return nameToEmoji[name] ?? nameToEmoji[slackAliases[name] ?? '']As per coding guidelines, "Use camelCase for variable names in TypeScript/JavaScript code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mrkdwn/src/emoji.ts` around lines 4 - 10, Rename the constant SLACK_ALIASES to camelCase slackAliases and update all usages accordingly (notably in the resolveEmoji function) so the identifier follows the project's camelCase convention; ensure the export (if any) and any imports or references elsewhere are updated to the new name to avoid breaking references.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/ui/src/components/Message.sveltepackages/mrkdwn/package.jsonpackages/mrkdwn/src/emoji.tspackages/mrkdwn/src/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.svelte
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.svelte: Use{@html}in Svelte for mrkdwn rendering (ESLint rule disabled for this specific use case)
Use Svelte 5 runes ($state,$derived,$effect) for reactivity instead of legacy stores
Files:
apps/ui/src/components/Message.svelte
apps/ui/src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
$libpath alias when importing fromsrc/lib/in Svelte applications
Files:
apps/ui/src/components/Message.svelte
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript code
No semicolons in code, enforced by Prettier
Use single quotes instead of double quotes
Use trailing commas in es5 format
Prefix unused variables with underscore (_)
Files:
packages/mrkdwn/src/index.tspackages/mrkdwn/src/emoji.ts
packages/mrkdwn/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
Files:
packages/mrkdwn/src/index.tspackages/mrkdwn/src/emoji.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to packages/mrkdwn/src/**/*.ts : Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
📚 Learning: 2026-02-20T00:31:35.696Z
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to **/*.svelte : Use `{html}` in Svelte for mrkdwn rendering (ESLint rule disabled for this specific use case)
Applied to files:
apps/ui/src/components/Message.sveltepackages/mrkdwn/src/emoji.ts
📚 Learning: 2026-01-14T21:36:07.933Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:07.933Z
Learning: In the client-only Botarium UI (Vite + Svelte), do not add or rely on SSR-specific guards (e.g., checks for sessionStorage) in components. Since the app runs only in Electron or the browser, code that accesses browser APIs will execute in a non-SSR environment, so SSR-unsafe checks are unnecessary and can be removed for maintainability.
Applied to files:
apps/ui/src/components/Message.svelte
📚 Learning: 2026-02-20T00:31:35.696Z
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to packages/mrkdwn/src/**/*.ts : Slack mrkdwn converter must provide bidirectional conversion (mrkdwn to HTML and Markdown to mrkdwn)
Applied to files:
packages/mrkdwn/src/index.tspackages/mrkdwn/src/emoji.ts
📚 Learning: 2026-02-20T00:31:35.696Z
Learnt from: CR
Repo: tyom/botarium PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T00:31:35.696Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use camelCase for variable names in TypeScript/JavaScript code
Applied to files:
packages/mrkdwn/src/emoji.ts
🧬 Code graph analysis (1)
packages/mrkdwn/src/emoji.ts (1)
packages/mrkdwn/src/index.ts (2)
resolveEmoji(3-3)nameToEmoji(4-4)
🔇 Additional comments (5)
packages/mrkdwn/src/emoji.ts (1)
17-20: LGTM!The resolution logic is clean: direct lookup with fallback through the alias mapping. Returning
undefinedfor unrecognised shortcodes allows callers to implement their own fallback behaviour.packages/mrkdwn/src/index.ts (1)
3-4: LGTM!The new exports extend the public API cleanly:
resolveEmojienables consumers to resolve shortcodes without renderingnameToEmojire-export provides external extensibility for custom emoji handlingThis maintains the bidirectional conversion capability whilst adding emoji utilities.
apps/ui/src/components/Message.svelte (2)
30-30: LGTM!Clean import of
resolveEmojifrom the@botarium/mrkdwnpackage, replacing the previous hard-coded emoji map with the centralised resolution API.
339-341: LGTM!The
getEmojifunction now uses the sharedresolveEmojiutility with a sensible fallback to the:name:text format when an emoji cannot be resolved. This handles both recognised shortcodes and graceful degradation for unrecognised ones.packages/mrkdwn/package.json (1)
20-20: gemoji 8.1.0 is a valid and secure dependency.Version 8.1.0 exists on npm, is the latest release, and has no known security vulnerabilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/mrkdwn/src/emoji.ts`:
- Around line 26-30: The renderEmoji function interpolates the raw name into
HTML unescaped; update renderEmoji to call escapeHtml on the name before using
it in the returned string (keep using resolveEmoji for the emoji lookup and the
emoji variable as-is), replacing :{name}: with :${escapedName}: (or similar) so
user-controlled input is escaped; import or reference the existing escapeHtml
utility and use it where renderEmoji currently injects name into the markup.
---
Nitpick comments:
In `@packages/mrkdwn/src/emoji.ts`:
- Around line 4-10: Rename the constant SLACK_ALIASES to camelCase slackAliases
and update all usages accordingly (notably in the resolveEmoji function) so the
identifier follows the project's camelCase convention; ensure the export (if
any) and any imports or references elsewhere are updated to the new name to
avoid breaking references.
Summary by CodeRabbit
New Features
Refactor