-
Notifications
You must be signed in to change notification settings - Fork 14
🤖 perf: remove legacy syntax highlighting in favor of Shiki #350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
**Shiki theme:** - Changed from 'dark-plus' to 'min-dark' throughout - Centralized theme constant in shikiHighlighter.ts **Search highlighting with decorations:** - Replaced DOM-based HTML post-processing with Shiki decorations - Decorations compute character positions at highlight time - Removed DOMParser, CRC32 caching for HTML manipulation - Added highlightSearchInText() for non-code text (file paths) - Removed highlightSearchMatches() complexity **Markdown integration:** - Integrated @shikijs/rehype plugin for code block highlighting - Removed react-syntax-highlighter dependency - Removed syntaxHighlighting.ts style definitions - Simplified MarkdownComponents (Shiki handles syntax now) - Enabled lazy language loading for better performance **Performance safeguards:** - Added MAX_DIFF_SIZE_BYTES (4kb) limit for diff highlighting - Large diffs fall back to plain text automatically - Enforced in one place (highlightDiffChunk) _Generated with `cmux`_
**Problem:** - @shikijs/rehype requires async processing - react-markdown uses runSync() (synchronous only) - Caused "runSync finished async" error **Solution:** - Removed @shikijs/rehype from rehype plugin pipeline - Created custom CodeBlock component with async Shiki highlighting - Reuses shared getShikiHighlighter() instance - Progressive enhancement: plain code → highlighted **Implementation:** - CodeBlock uses useState/useEffect pattern (matches DiffRenderer) - On-demand language loading with graceful fallback - Shows plain code during loading or on error - Renders highlighted HTML once ready **Benefits:** - Works with react-markdown's sync pipeline - Consistent with existing diff highlighting approach - Reuses highlighter instance (no duplication) - Better error handling and fallback behavior _Generated with `cmux`_
- Use reduce on line lengths instead of TextEncoder for better performance - Add span.search-highlight CSS selector (Shiki uses span, not mark) - Decorations now properly styled with yellow highlight
- Changed 'span' selector to 'span:not(.search-highlight)' - Prevents transparent !important from overriding search highlight background - Search terms now properly highlighted in yellow in diff code
- Replace hardcoded rgba(0, 0, 0, 0.3) with var(--color-code-bg) - Consistent styling with rest of application
- Shiki's min-dark theme uses #1f1f1f inline - Override with var(--color-code-bg) for consistency
- Removed manual language loading check - codeToHtml already handles lazy-loading internally - Cleaner code with same functionality
- Added global pre and pre > code styling in App.tsx - Removed duplicate inline styles from MarkdownComponents - Single source of truth for code block appearance - Net -20 LoC
- Changed from 'pre' to 'pre code' to avoid styling non-code pre elements - Consolidated into single rule with display: block
Reverts to the performant DOM-based approach that was in main before this branch. The Shiki decoration-based approach was slower due to: - Computing decorations synchronously blocks initial render - Decorations applied during tokenization couples search with highlighting Restored approach (Option A from plan): - Highlight code WITHOUT search decorations (fast, immediate render) - Post-process HTML to wrap matches using DOM manipulation - Uses LRU caches: parsed DOMs (CRC32-keyed) + regex patterns Architecture changes: - highlightSearchTerms.ts: Restored highlightSearchMatches() and highlightSearchInText() - highlightDiffChunk.ts: Removed searchConfig parameter and decoration logic - DiffRenderer.tsx: Added highlightedLineData useMemo that applies search post-process Performance benefits: - Initial syntax highlighting renders immediately - Search highlighting non-blocking (computed in useMemo) - DOM cache reused across different searches (same HTML, different terms) All 708 tests passing, typecheck clean.
Now that we've fully migrated to Shiki and removed react-syntax-highlighter, these files are no longer needed: - scripts/generate_prism_css.ts - Generated CSS for react-syntax-highlighter - src/styles/prism-syntax.css - Output of the generator Both were used for the old Prism-based syntax highlighting approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The highlightSearchInText() function was building HTML strings without escaping text content, allowing potential XSS via malicious file paths or search terms containing HTML/script tags. Fix: Added escapeHtml() helper and escape all text content before concatenating into HTML string: - Text before matches - The matched substring - Text after matches This prevents script injection while maintaining search highlighting functionality.
Replaced custom HTML escape implementation with the well-tested escape-html package from the Express.js team. This provides better security confidence through: - Industry-standard implementation - Extensive testing and auditing - Active maintenance - Small bundle size (~2KB) Added dependencies: - escape-html - @types/escape-html (dev)
Summary
Migrates from
react-syntax-highlighterto Shiki for improved performance and bundle size. Uses DOM-based post-processing for search highlighting to maintain fast, non-blocking rendering.Changes
Syntax Highlighting:
react-syntax-highlighterwith Shikimin-dark(centralized inSHIKI_THEMEconstant)Search Highlighting:
highlightSearchMatches()inuseMemofor performanceMarkdown Code Blocks:
CodeBlockcomponentcodeToHtml()CSS:
App.tsxvar(--color-code-bg)mark.search-highlightandspan.search-highlightPerformance Benefits
Test Coverage
✅ All 708 tests passing
✅ Real Shiki integration tests (no mocks)
✅ Typecheck clean (main + renderer)
Breaking Changes
None - maintains full backwards compatibility with existing search functionality.
Dependencies
react-syntax-highlighter,@types/react-syntax-highlightershiki(already present),crc-32(already present)Generated with
cmux