Fix corrupt markdown code block crash#1603
Conversation
|
| Filename | Overview |
|---|---|
| packages/app/src/components/highlighted-code-block.tsx | Refactors both highlighted and plain code paths to render one View+MarkdownTextSpan per line instead of a single large Text node, adds web-only whiteSpace/overflowX styles; approach is correct but keyPlainCodeLines embeds full line content in React keys. |
| packages/app/src/components/highlighted-code-block.test.tsx | New regression test for issue-1589; uses vi.mock, vi.hoisted, vi.stubGlobal, JSDOM, and component mounting — all banned by project test-discipline rules — and asserts CSS style attributes rather than behavioral output. |
| packages/app/src/components/fixtures/issue-1589-homelab-microvms-proposal.broken.txt | Test fixture containing the corrupted markdown from the linked issue; used verbatim to seed the regression test — no code changes, just a data file addition. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[HighlightedCodeBlock\nreceives code + language] --> B{highlightToKeyedLines\nreturns non-null?}
B -- Yes\nsyntax-highlighted --> C[CodeLines\nper-line View + MarkdownTextSpan]
B -- No\nplain text --> D[keyPlainCodeLines\ncontent-keyed array]
D --> E[PlainCodeLines\nper-line View + MarkdownTextSpan]
C --> F[DOM: one span per line\nwhite-space: pre]
E --> F
F --> G[Container View\noverflowX: auto]
G --> H[CopyButton absolute overlay]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[HighlightedCodeBlock\nreceives code + language] --> B{highlightToKeyedLines\nreturns non-null?}
B -- Yes\nsyntax-highlighted --> C[CodeLines\nper-line View + MarkdownTextSpan]
B -- No\nplain text --> D[keyPlainCodeLines\ncontent-keyed array]
D --> E[PlainCodeLines\nper-line View + MarkdownTextSpan]
C --> F[DOM: one span per line\nwhite-space: pre]
E --> F
F --> G[Container View\noverflowX: auto]
G --> H[CopyButton absolute overlay]
Reviews (1): Last reviewed commit: "Fix markdown code block rendering crash" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /** | ||
| * @vitest-environment jsdom | ||
| */ | ||
| import { readFileSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import React, { act } from "react"; | ||
| import { createRoot, type Root } from "react-dom/client"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const { theme } = vi.hoisted(() => ({ | ||
| theme: { | ||
| spacing: { 1: 4, 2: 8, 3: 12 }, | ||
| colors: { | ||
| foreground: "#fff", |
There was a problem hiding this comment.
This test file uses vi.hoisted, vi.mock (×6), and vi.stubGlobal — all explicitly flagged by the project's testing rules — plus a @vitest-environment jsdom component-mounting test where the rules call for extracting logic from the component and testing it as a pure function. The pure functions here — keyPlainCodeLines and extractFirstFence — could be tested without any mocking or JSDOM. The crash-safety behavioral claim (one Text node → per-line Views) is better validated by E2E or by testing keyPlainCodeLines directly and asserting on its output shape, rather than mounting the full component in JSDOM and inspecting inline styles.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| textStyle={CODE_TEXT_STYLE} | ||
| />, | ||
| ); | ||
|
|
||
| const lineElements = Array.from(container?.querySelectorAll("div, span") ?? []).filter( | ||
| (element) => | ||
| (element.textContent?.includes("│") || element.textContent?.includes("┌")) && | ||
| !element.textContent.includes("Host"), | ||
| ); | ||
|
|
||
| expect(container?.textContent).toContain("bridge: fcbr0"); | ||
| expect(lineElements.length).toBeGreaterThanOrEqual(3); | ||
| expect( | ||
| lineElements.some((element) => | ||
| (element.getAttribute("style") ?? "").includes("white-space: pre"), | ||
| ), |
There was a problem hiding this comment.
element.getAttribute("style")?.includes("white-space: pre") asserts a specific inline-style attribute. If whiteSpace: "pre" is ever moved to a StyleSheet class or a CSS rule, this assertion silently breaks while the actual behavior stays correct. The stronger behavioral check is asserting each diagram line appears as its own text node (one <span> per line with no embedded newlines), which would prove the structural fix more directly.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| function keyPlainCodeLines(code: string): { key: string; text: string }[] { | ||
| const seen = new Map<string, number>(); | ||
| return code.split("\n").map((text) => { | ||
| const seenCount = seen.get(text) ?? 0; | ||
| seen.set(text, seenCount + 1); | ||
| return { key: `${text}:${seenCount}`, text }; | ||
| }); |
There was a problem hiding this comment.
key: \${text}:${seenCount}`` embeds the complete line content into the React key. For the issue-1589 fixture, diagram lines become keys 60+ characters long. A content-hash or sequential counter keyed only on position would achieve the same stable-reconciliation goal with predictable key lengths.
1 participant
Linked issue
Closes #1589
Type of change
What does this PR do
Fixes a desktop/web markdown preview crash path where a corrupted markdown file can feed a diagram-heavy fenced code block into one large React Native Web text node.
Markdown code blocks now render as separate preformatted line rows for both highlighted and plain-code paths. The web style also preserves whitespace and disables forced wrapping inside the code block, with horizontal overflow on the code container. This keeps box-drawing diagrams from going through one complex text layout node while preserving the existing copy button, monospace styling, and syntax-token colors.
A regression test uses the exact attachment from #1589 as a fixture and renders its first diagram fence through
HighlightedCodeBlock.How did you verify it
markdown-itparsing of the attachment completes quickly on current main; the risky path is code-block rendering, not markdown parsing.packages/app/src/components/highlighted-code-block.test.tsxcovering the bug: corrupt markdown crashes the client #1589 attachment diagram fence.npm run build:serveronce to refresh generated workspace declarations in the fresh worktree before typecheck.npm run test --workspace=@getpaseo/app -- src/components/highlighted-code-block.test.tsx src/components/markdown/html-ish.test.ts --bail=1passed: 2 files, 28 tests.npm run lintpassed.npm run typecheckpassed.npm run formatran.git diff --checkpassed.I did not capture desktop screenshots/video; this is a crash-prevention rendering-path change covered by a component regression test, not a visual redesign.
Checklist
npm run typecheckpassesnpm run lintpassesnpm run formatran (Biome)