fix(ui): marked v18 parseSync removal#141
Conversation
marked v13+ removed parseSync(). The parse() method is synchronous by default (unless async: true is passed). Fixes CI build failure.
|
Caution Review failedPull request was closed or merged during review ОбзорЗаменён синхронный вызов парсера Markdown: Изменения
Оценка сложности код-ревью🎯 2 (Simple) | ⏱️ ~10 минут Стихотворение
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Code Review
This pull request updates the markdown rendering logic in useMarkdown.ts by replacing marked.parseSync with marked.parse. A review comment points out that the current type assertion is unsafe because marked.parse can return a Promise if async extensions are used, and suggests using a type guard instead to ensure the output is a string.
| export function renderMarkdown(text: string): string { | ||
| if (!text) return '' | ||
| const html = marked.parseSync(text) | ||
| const html = marked.parse(text) as string |
There was a problem hiding this comment.
The type assertion as string is potentially unsafe. While marked.parse is synchronous by default, it returns a Promise if any async extensions or hooks are registered globally. Since marked is a global singleton, this could cause DOMPurify.sanitize to receive a Promise object (resulting in [object Promise] being rendered in the UI) if the configuration is modified elsewhere in the application. Using a type guard ensures the result is a string as expected by the rest of the function and the TypeScript compiler.
| const html = marked.parse(text) as string | |
| const html = marked.parse(text) | |
| if (typeof html !== 'string') return '' |
…arseSync - Dockerfile: update build path after cmd/mcp-stdio-proxy was renamed to cmd/engram in PR #140 - useMarkdown.ts: marked.parseSync → marked.parse (parseSync removed in marked v13+)
Summary
marked.parseSync(text)→marked.parse(text) as stringparseSync(sync is default since v13)Test plan
npm run buildpasses locallySummary by CodeRabbit