Skip to content

MarkdownPreview における HTML の二重パースを解消#216

Open
kitsne241 wants to merge 2 commits intomainfrom
refactor/extract-headings-in-dompurify-hook
Open

MarkdownPreview における HTML の二重パースを解消#216
kitsne241 wants to merge 2 commits intomainfrom
refactor/extract-headings-in-dompurify-hook

Conversation

@kitsne241
Copy link
Copy Markdown
Collaborator

@kitsne241 kitsne241 commented Feb 15, 2026

緊急ではありません あとブランチ名はうそです

MarkdownPreview.vue において DOMPurify.sanitize の結果(HTML テキスト)を再度パースして見出しを見つける無駄な処理が走っていたので、DOMPurify から DOM を返させて見出し探しに利用することでパースを 1 回にします

また、大した効果はないと思うけれど MarkdownIt インスタンスを utils/markdown.ts に引っ越してコンポーネント毎に作成されないようにすることでメモリ効率を少しだけ上げた(かもしれない)

Summary by CodeRabbit

リリースノート

  • Refactor
    • マークダウンレンダリング処理を統一ユーティリティに移行し、内部実装を効率化しました。
    • DOM処理の最適化により、マークダウン表示のパフォーマンスと安定性が向上しています。

Copilot AI review requested due to automatic review settings February 15, 2026 02:45
@github-actions
Copy link
Copy Markdown

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

MarkdownItのインスタンス生成をコンポーネントから専用ユーティリティに抽出し、DOMPurifyのサニテーション処理フローをHTML文字列ベースからDOM直接返却に変更。

Changes

Cohort / File(s) Summary
MarkdownItユーティリティの新規作成
src/utils/markdown.ts
breaks: true、linkify: trueオプション付きの新規MarkdownItインスタンスを作成・エクスポート。
MarkdownPreviewコンポーネントのリファクタリング
src/components/markdown/MarkdownPreview.vue
MarkdownIt処理を外部ユーティリティに移行。DOMPurifyをDOM直接返却(RETURN_DOM: true)に変更、HTMLパース処理を削除。サニテーション済みDOMから直接見出しメタデータを抽出、最終HTML出力をcleanDom.innerHTMLから取得。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed プルリクエストのタイトル「MarkdownPreview における HTML の二重パースを解消」は、変更内容の主要な目的(HTML の二重パースの削減)を正確に反映しており、開発者の視点から最も重要な改善点を簡潔に表現しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/extract-headings-in-dompurify-hook

No actionable comments were generated in the recent review. 🎉


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

MarkdownPreview.vue で、Markdown → HTML 生成後に DOMPurify のサニタイズ結果(HTML文字列)を再度 DOMParser でパースしていた処理を、DOMPurify から DOM を直接受け取る形に変えて二重パースを解消する PR です。あわせて MarkdownIt インスタンスを utils に切り出し、コンポーネント毎の生成を避けることで再利用可能にしています。

Changes:

  • DOMPurify のサニタイズ結果を文字列ではなく DOM として扱い、見出し抽出時の再パースを削除
  • MarkdownIt インスタンスを src/utils/markdown.ts に移動して共有化

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/utils/markdown.ts 再利用用の MarkdownIt インスタンスを定義・export
src/components/markdown/MarkdownPreview.vue DOMPurify の RETURN_DOM を使って DOM から直接見出し抽出し、HTML 出力もその DOM 由来に変更

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const level = parseInt(heading.tagName.charAt(1))
const id = `heading-${idCounter++}`
const text = heading.textContent ?? ''
const text = heading.textContent
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HeadingInfo.text is typed as string, but heading.textContent is string | null under strict: true. This will either fail type-checking or force downstream handling of null. Consider using a nullish fallback (e.g., heading.textContent ?? '') or updating HeadingInfo.text to allow null if that's intended.

Suggested change
const text = heading.textContent
const text = heading.textContent ?? ''

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strict: true の場合にも heading.textContent は string と型推論され、null にはなりえません。heading 自体が Element として型推論されているからです

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants