fix: code blocks invisible in dark mode#763
fix: code blocks invisible in dark mode#763WolfieLeader wants to merge 4 commits intoTanStack:mainfrom
Conversation
The initial render used className "shiki github-light dark:aurora-x".
In dark mode, `html.dark .shiki.github-light { display: none }` hid
the element. Since dark:aurora-x is not a valid Tailwind utility, the
aurora-x class was never applied — leaving code blocks permanently
hidden when Shiki's async highlighting failed to replace them.
- Use Tailwind dark: variants for proper light/dark initial render
- Add try/catch to Shiki effect to prevent silent failures
- Add cleanup flag to prevent state updates after unmount
Fixes TanStack#761, fixes TanStack#760, fixes TanStack#759
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
📝 WalkthroughWalkthroughRefactors code block rendering: adds async cancellation and error handling, restructures syntax-highlighting and mermaid rendering, and removes an unused import in CopyPageDropdown. Rendering control flow and DOM updates are guarded against component unmounts. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/React
participant CodeBlock as CodeBlock component
participant Highlighter as SyntaxHighlighter
participant Mermaid as MermaidRenderer
participant DOM as DOM / state
Browser->>CodeBlock: mount with code + lang + theme
CodeBlock->>Highlighter: request highlight (async)
alt lang == mermaid
CodeBlock->>Mermaid: render mermaid to svg (async)
Mermaid-->>CodeBlock: svg/html
else
Highlighter-->>CodeBlock: highlighted HTML per theme
end
CodeBlock->>CodeBlock: check cancelled flag
alt not cancelled
CodeBlock->>DOM: setState/update innerHTML
else
CodeBlock--xDOM: abort update
end
Browser->>CodeBlock: unmount -> set cancelled flag
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 1
🧹 Nitpick comments (1)
src/components/markdown/CodeBlock.tsx (1)
169-221: Good async cancellation and error handling pattern.The cancellation flag correctly prevents state updates after unmount. The try/catch ensures the component gracefully degrades to the initial placeholder if highlighting fails.
One minor note:
isEmbeddedis used in the effect (line 207) but not included in the dependency array. IfisEmbeddedcould change during the component's lifecycle, the styling would become stale. In practice this prop is likely stable, but consider adding it for correctness:- }, [code, lang]) + }, [code, lang, isEmbedded])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/markdown/CodeBlock.tsx` around lines 169 - 221, The effect uses the isEmbedded prop inside its async work (affecting className logic) but the dependency array only lists [code, lang], so add isEmbedded to the dependency array of the useEffect that contains the async IIFE (the block that sets cancelled, calls getHighlighter/getMermaid and calls setCodeElement) to ensure the effect re-runs when isEmbedded changes; update the dependency list from [code, lang] to [code, lang, isEmbedded] (or include isEmbedded alongside existing deps) so styling remains correct and React rules of hooks are satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/markdown/CodeBlock.tsx`:
- Around line 187-196: The code builds a div using preAttributes.class from
extractPreAttributes(output) which can be null, causing class='null ...'; update
the mermaid handling in CodeBlock.tsx to null-safe the class value returned by
extractPreAttributes: read preAttributes via extractPreAttributes(output),
coalesce preAttributes.class to an empty string (or only include the class
attribute when non-empty) before interpolating into the returned HTML string
(references: extractPreAttributes, preAttributes, genSvgMap, getMermaid,
trimmedCode).
---
Nitpick comments:
In `@src/components/markdown/CodeBlock.tsx`:
- Around line 169-221: The effect uses the isEmbedded prop inside its async work
(affecting className logic) but the dependency array only lists [code, lang], so
add isEmbedded to the dependency array of the useEffect that contains the async
IIFE (the block that sets cancelled, calls getHighlighter/getMermaid and calls
setCodeElement) to ensure the effect re-runs when isEmbedded changes; update the
dependency list from [code, lang] to [code, lang, isEmbedded] (or include
isEmbedded alongside existing deps) so styling remains correct and React rules
of hooks are satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 389c9f92-3560-462a-8ea4-199f99c2fe3a
📒 Files selected for processing (2)
src/components/CopyPageDropdown.tsxsrc/components/markdown/CodeBlock.tsx
- Add null safety for preAttributes.class in mermaid rendering - Add isEmbedded to effect dependency array
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/markdown/CodeBlock.tsx`:
- Around line 215-217: The catch block in CodeBlock.tsx only logs errors ("Shiki
highlighting failed") and leaves the component blank; update the error path to
set a real fallback UI so users see a sensible placeholder: in the catch of the
highlighting routine (the function that performs Shiki highlighting, e.g.,
highlightCode or the async block around shiki), set the component state used to
render output (e.g., setHighlightedHtml / setRenderState) to a fallback markup
for normal code blocks (plain escaped code wrapped in a
<pre><code>...)</code></pre> and for mermaid blocks call the existing mermaid
render path fallback (renderMermaid) or set the mermaid state to an empty <svg
/> placeholder; ensure the fallback path is used by the render branch that reads
the highlighted state so users see the placeholder instead of nothing.
- Around line 191-193: The current mermaid.render call uses a constant id 'foo',
which causes SVG id collisions; update the CodeBlock.tsx rendering logic to
generate a unique id per render (for example using a short UUID, a timestamp +
counter, or the trimmedCode hash) and pass that id into mermaid.render instead
of 'foo', ensure the same generated id is used if caching via genSvgMap for
trimmedCode so lookups remain consistent, and obtain the mermaid instance via
getMermaid() as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ece5f9c-9186-4471-83b9-ed4a626d6cea
📒 Files selected for processing (1)
src/components/markdown/CodeBlock.tsx
| const mermaid = await getMermaid() | ||
| const { svg } = await mermaid.render('foo', trimmedCode) | ||
| genSvgMap.set(trimmedCode, svg) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Mermaid v11.11.0, what does the first idargument ofmermaid.render(id, text) control, and is it required/recommended to be unique per rendered diagram instance?
💡 Result:
In Mermaid v11.11.0, the first argument id in await mermaid.render(id, text) is used as the DOM id of the generated root <svg> element. Mermaid’s internal renderer literally does .append('svg').attr('id', id). [1]
It’s effectively required (the parameter is not optional) and it’s strongly recommended to be unique per rendered diagram instance in the same document, because Mermaid also derives other temporary element IDs/selectors from it (e.g. 'd' + id, 'i' + id) and will remove any existing elements with the same id before rendering—so reusing an id can overwrite/remove a previously-rendered diagram or otherwise cause collisions. [2]
Sources
[1] https://raw.githubusercontent.com/mermaid-js/mermaid/9c85521/packages/mermaid/src/mermaidAPI.ts
[2] https://raw.githubusercontent.com/mermaid-js/mermaid/9c85521/packages/mermaid/src/mermaidAPI.ts
🏁 Script executed:
cat -n src/components/markdown/CodeBlock.tsx | sed -n '185,200p'Repository: TanStack/tanstack.com
Length of output: 776
🏁 Script executed:
# Check broader context - how many mermaid blocks might be rendered on a single page
# and if this function is called multiple times
grep -n "lang === 'mermaid'" src/components/markdown/CodeBlock.tsxRepository: TanStack/tanstack.com
Length of output: 167
Use a unique Mermaid render ID instead of a constant.
The hardcoded 'foo' ID in mermaid.render('foo', trimmedCode) at line 192 will cause duplicate SVG IDs and broken marker/style references when multiple Mermaid blocks are rendered on the same page. According to Mermaid's API, the id parameter becomes the DOM id of the root <svg> element, and reusing the same ID can cause collisions, overwrites, and derived ID conflicts within Mermaid's internal selectors.
Generate a unique ID for each render call:
Suggested fix
let mermaidInstance: Mermaid | null = null
+let mermaidRenderCounter = 0
const genSvgMap = new Map<string, string>()
...
- const { svg } = await mermaid.render('foo', trimmedCode)
+ const renderId = `mermaid-${++mermaidRenderCounter}`
+ const { svg } = await mermaid.render(renderId, trimmedCode)
genSvgMap.set(trimmedCode, svg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/markdown/CodeBlock.tsx` around lines 191 - 193, The current
mermaid.render call uses a constant id 'foo', which causes SVG id collisions;
update the CodeBlock.tsx rendering logic to generate a unique id per render (for
example using a short UUID, a timestamp + counter, or the trimmedCode hash) and
pass that id into mermaid.render instead of 'foo', ensure the same generated id
is used if caching via genSvgMap for trimmedCode so lookups remain consistent,
and obtain the mermaid instance via getMermaid() as before.
| } catch (err) { | ||
| console.warn('Shiki highlighting failed:', err) | ||
| } |
There was a problem hiding this comment.
Add a real fallback UI in the error path.
When highlighting/rendering fails, the catch path only logs. For Mermaid blocks, the initial state can stay visually empty (placeholder <svg />), so users still get a blank block on failure.
Suggested fix
} catch (err) {
console.warn('Shiki highlighting failed:', err)
+ if (!cancelled) {
+ setCodeElement(
+ <pre
+ ref={ref}
+ className="h-full p-4 bg-white dark:bg-gray-950 text-gray-800 dark:text-gray-200"
+ >
+ <code>{code || ''}</code>
+ </pre>,
+ )
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/markdown/CodeBlock.tsx` around lines 215 - 217, The catch
block in CodeBlock.tsx only logs errors ("Shiki highlighting failed") and leaves
the component blank; update the error path to set a real fallback UI so users
see a sensible placeholder: in the catch of the highlighting routine (the
function that performs Shiki highlighting, e.g., highlightCode or the async
block around shiki), set the component state used to render output (e.g.,
setHighlightedHtml / setRenderState) to a fallback markup for normal code blocks
(plain escaped code wrapped in a <pre><code>...)</code></pre> and for mermaid
blocks call the existing mermaid render path fallback (renderMermaid) or set the
mermaid state to an empty <svg /> placeholder; ensure the fallback path is used
by the render branch that reads the highlighted state so users see the
placeholder instead of nothing.
|
I appreciate the effort you put into this, however, the issue was due to a prev commit. If we end up needing to revisit this implementation due to any errors, we can revisit opening it! |
2 participants
Summary
<pre>render usedclassName="shiki github-light dark:aurora-x"— the CSS rulehtml.dark .shiki.github-light { display: none }hid it, anddark:aurora-xis not a valid Tailwind utility so theaurora-xclass was never appliedChanges
CodeBlock.tsx: Replace theme-specific classes with Tailwinddark:variants so the initial render is visible in both modesCodeBlock.tsx: Add try/catch + cleanup flag to the Shiki highlighting effectCopyPageDropdown.tsx: Remove unusedPACKAGE_MANAGERSimport (lint cleanup)Test plan
Fixes #761, fixes #760, fixes #759
Summary by CodeRabbit
Improvements
Bug Fixes
Refactor