refactor(markdown-renderer): restructure renderer with recursive pipeline and caching#1811
refactor(markdown-renderer): restructure renderer with recursive pipeline and caching#1811Killusions wants to merge 2 commits intomainfrom
Conversation
Add max-inline-size constraint to message bubble and wrapper to prevent overflow. Fix attachment preview backdrop click behavior. Use index-based tracking for message list and add scroll-to-bottom on message send. Update example action labels for clarity.
…line and caching - rewrite the markdown renderer to use a recursive processing pipeline with placeholder-based architecture - add SSR support via optional doc/isBrowser parameters - introduce LRU caching for code blocks and tables - iprove table rendering with proper thead/th support - add code-wrapper structure with language labels - update SCSS for new structural layout including code-wrapper, table-wrapper, and table-scroll-container
Killusions
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the markdown renderer to support Server-Side Rendering (SSR) and introduces an LRU caching mechanism for code blocks and tables. The getMarkdownRenderer function now accepts a document reference and platform state to ensure compatibility across environments. The changes also include CSS updates for better code and table presentation, and label updates (e.g., "Copy" to "Export") to follow UX writing guidelines. Review feedback highlights the need to move placeholder replacement logic out of browser-only blocks for SSR support, suggests optimizing table row filtering to avoid DOM-dependent APIs, and recommends using an incrementing counter for placeholders instead of random strings.
| if (isInBrowser) { | ||
| const walker = docRef.createTreeWalker(div, NodeFilter.SHOW_COMMENT); | ||
|
|
||
| let currentNode = walker.nextNode(); | ||
| while (currentNode) { | ||
| const comment = currentNode as Comment; | ||
|
|
||
| // Handle code block placeholders | ||
| const codeMatch = comment.textContent?.match(/CODE-BLOCK-PLACEHOLDER-(.*)/); | ||
| if (codeMatch) { | ||
| const placeholderId = codeMatch[1]; | ||
| const cacheKey = codeBlockPlaceholderMap.get(placeholderId); | ||
| if (cacheKey) { | ||
| const keyParts = cacheKey.split('|||'); | ||
| const language = keyParts[0]; | ||
| const content = keyParts[1]; | ||
| const cachedElement = createCodeBlockElement(language, content); | ||
| if (cachedElement) { | ||
| commentsToReplace.push({ comment, element: cachedElement }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Handle table placeholders | ||
| const tableMatch = comment.textContent?.match(/TABLE-PLACEHOLDER-(.*)/); | ||
| if (tableMatch) { | ||
| const placeholderId = tableMatch[1]; | ||
| const tableDataJson = tablePlaceholderMap.get(placeholderId); | ||
| if (tableDataJson) { | ||
| try { | ||
| const { hasSeparator, tableLines } = JSON.parse(tableDataJson); | ||
| const cachedElement = createTableElement(tableLines, hasSeparator); | ||
| if (cachedElement) { | ||
| commentsToReplace.push({ comment, element: cachedElement }); | ||
| } | ||
| } catch (e) { | ||
| console.warn('Failed to parse table placeholder data', e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| currentNode = walker.nextNode(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of placeholder replacement is wrapped in an isInBrowser block. This means that in an SSR environment, the rendered HTML will contain comment placeholders instead of the actual code blocks or tables. To support SSR as intended, the replacement logic should be unified or performed on the DOM tree using a platform-agnostic reference (like the provided docRef) rather than being restricted to the browser environment.
References
- Ensure that UI features and logic are compatible with Server-Side Rendering (SSR) by avoiding unnecessary browser-only execution blocks when the logic can be performed via platform-agnostic APIs.
| const filteredRows = rows.filter(row => { | ||
| const tempTable = docRef.createElement('table'); | ||
| tempTable.innerHTML = `<tbody>${row}</tbody>`; | ||
| const tableCells = tempTable.querySelectorAll('td, th'); | ||
| return Array.from(tableCells).some(cell => { | ||
| const hasText = !!cell.textContent?.trim(); | ||
| const hasHtml = !!cell.innerHTML?.trim(); | ||
| return hasText || hasHtml; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Creating a temporary table element and parsing HTML for every single row to check if it's empty is a significant performance bottleneck. Furthermore, this approach relies on browser-specific DOM APIs which are not available during Server-Side Rendering (SSR). Since you already have the processedCells array of strings, you can check if any cell contains non-whitespace content directly using platform-agnostic logic.
const filteredRows = rows.filter((_, index) => {
const cellContents = allProcessedCells[index];
return cellContents.some(cell => cell.trim().length > 0);
});References
- Avoid using browser-specific DOM APIs in code that needs to support Server-Side Rendering (SSR), as they are not available on the server.
| result = result.replace( | ||
| /(^|\n)([\s]*)(````+)([^\n]*)\n?([\s\S]*?)\n?\s*\3/gm, | ||
| (match, prefix, indent, backticks, language, content) => { | ||
| const placeholder = `--CODE-BLOCK-${Math.random().toString(36).substring(2, 15)}--`; |
There was a problem hiding this comment.
Using Math.random() for generating placeholders can theoretically lead to collisions, although unlikely. A more robust and performant approach is to use a simple incrementing counter within the renderer instance.
| const placeholder = `--CODE-BLOCK-${Math.random().toString(36).substring(2, 15)}--`; | |
| const placeholder = "--CODE-BLOCK-" + placeholderCounter++ + "--"; |
1 participant
Merge after #1810
Preparation for #1812 #1813 #1814 #1815 #1816
with placeholder-based architecture
including code-wrapper, table-wrapper, and table-scroll-container
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: