feat(desktop): 大規模ファイル向け CodeMirror diff ビューア#5
Merged
Conversation
The existing @pierre/diffs diff viewer renders all lines at once, causing severe lag on files with 2000+ lines (~180k+ DOM nodes). Add CodeMirrorDiffViewer using @codemirror/merge which provides virtualized rendering (only visible lines in DOM). Files with >2000 total lines automatically use this viewer instead. - Reuses existing CodeMirror theme, fonts, and syntax highlighting - Unchanged regions are collapsed (margin: 3, minSize: 4) - Read-only side-by-side view with line numbers - No changes to LightDiffViewer (small files use it as before)
📝 WalkthroughWalkthroughThe pull request introduces a new CodeMirror-based diff viewer component that uses Changes
Sequence DiagramsequenceDiagram
participant FC as FileViewerContent
participant CMDV as CodeMirrorDiffViewer
participant LS as Language Support
participant CM as CodeMirror/MergeView
participant Settings as Editor Settings
FC->>FC: Calculate totalLines from diff
alt totalLines > 2000
FC->>CMDV: Render with original/modified/language
activate CMDV
CMDV->>LS: loadLanguageSupport(language)
activate LS
LS-->>CMDV: Language loaded
deactivate LS
CMDV->>Settings: getFontSettings()
activate Settings
Settings-->>CMDV: Font settings
deactivate Settings
CMDV->>CMDV: Create compartments for language/theme
CMDV->>CM: Initialize MergeView with panes
activate CM
CM->>CM: Apply base extensions + language + theme
CM-->>CMDV: MergeView ready
deactivate CM
CMDV->>CMDV: Store MergeView ref
deactivate CMDV
else totalLines ≤ 2000
FC->>FC: Use LightDiffViewer instead
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsx (1)
27-32:viewModecurrently causes expensive no-op remounts.
viewModeis in the creation effect deps (Line 105) but not used to configureMergeView, so toggling mode rebuilds the whole viewer without changing behavior.♻️ Proposed refactor (remove no-op dependency until mode is implemented)
interface CodeMirrorDiffViewerProps { original: string; modified: string; language: string; - viewMode: DiffViewMode; } export function CodeMirrorDiffViewer({ original, modified, language, - viewMode, }: CodeMirrorDiffViewerProps) { @@ - }, [original, modified, language, viewMode]); + }, [original, modified, language]);Also applies to: 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsx` around lines 27 - 32, The effect that creates the CodeMirror MergeView is currently listing the viewMode prop in its dependency array but viewMode is not used to configure MergeView, causing unnecessary remounts when toggling it; update the effect in CodeMirrorDiffViewer so it no longer includes viewMode in the creation effect deps (or alternatively wire viewMode into the MergeView configuration if you intend it to change runtime behavior) — locate the useEffect that instantiates MergeView and remove viewMode from its dependencies to prevent no-op rebuilds until viewMode is actually implemented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsx`:
- Around line 94-99: Guard against stale async language loads by capturing the
current mergeViewRef and language (or a generation token) before calling
loadLanguageSupport(language), then when the promise resolves verify the
captured ref/token still matches the current mergeViewRef and language; if not,
bail without applying the extension. Also add rejection handling for
loadLanguageSupport(language) (try/catch or .catch) and log or ignore the error
instead of letting it throw. Apply this pattern around the loadLanguageSupport
call that updates mv.a/mv.b with langCompartmentA/B.reconfigure.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx`:
- Around line 323-335: The large-diff early return renders CodeMirrorDiffViewer
directly and omits the DiffViewerContextMenu wrapper used by the small-diff
path, which strips diff-pane actions; update the large-diff branch so that
instead of returning CodeMirrorDiffViewer alone you return it wrapped with the
same DiffViewerContextMenu component (preserving props like diffData.original,
diffData.modified, diffData.language and viewMode/diffViewMode) so the context
menu handlers (split/move/close/edit-at-location) remain available for large
diffs.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsx`:
- Around line 27-32: The effect that creates the CodeMirror MergeView is
currently listing the viewMode prop in its dependency array but viewMode is not
used to configure MergeView, causing unnecessary remounts when toggling it;
update the effect in CodeMirrorDiffViewer so it no longer includes viewMode in
the creation effect deps (or alternatively wire viewMode into the MergeView
configuration if you intend it to change runtime behavior) — locate the
useEffect that instantiates MergeView and remove viewMode from its dependencies
to prevent no-op rebuilds until viewMode is actually implemented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65d875a8-a526-42a6-a66b-a260fd09237f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/desktop/package.jsonapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/CodeMirrorDiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx
The default scanLimit (500) causes the diff algorithm to fall back to an imprecise mode for large files, marking everything as changed. Increase to 50000 with a 5s timeout.
Keep context menu actions (split, move, close, edit at location) available for large file diffs. Also conditionally skip MarkdownSearch and DiffScrollbarDecorations for CodeMirror path since they are @pierre/diffs specific.
This was referenced Mar 31, 2026
This was referenced Apr 12, 2026
This was referenced Apr 21, 2026
MocA-Love
added a commit
that referenced
this pull request
Apr 23, 2026
upstream 取り込み PR #5: superset-sh#3295 + 19 procedure architecture rework
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
既存の
@pierre/diffsdiff ビューアは全行を一括でDOMに出力するため、2000行以上のファイルでカクカクになる問題（~18万DOMノード）を解決。CodeMirror 6 の
@codemirror/merge拡張を使った仮想化レンダリング対応の diff ビューアを追加。ビューポートに表示されている行だけをDOMに描画するため、15000行のファイルでもスムーズに表示できる。仕組み
CodeMirrorDiffViewerに切り替えLightDiffViewer（@pierre/diffs）を使用LightDiffViewerは一切変更なしDiffViewerContextMenuでラップし、コンテキストメニュー機能（分割、移動、閉じる等）を維持変更ファイル
新規 (2)
CodeMirrorDiffViewer/CodeMirrorDiffViewer.tsx— CodeMirror merge 拡張によるdiffビューアCodeMirrorDiffViewer/index.ts— barrel export変更 (3)
FileViewerContent.tsx— 行数判定の分岐を追加。大規模ファイルでもDiffViewerContextMenuで包んでコンテキストメニューを維持package.json—@codemirror/merge依存追加bun.lock— lockfile更新特徴
scanLimit: 50000/timeout: 5000msで大規模ファイルの正確な差分検出DiffViewerContextMenuによるコンテキストメニュー維持（分割・移動・閉じる等）テスト