docs: add remote markdown and mermaid rendering#145
Open
Kzoeps wants to merge 1 commit into
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds remote markdown loading at browser runtime, Mermaid diagram rendering from code fences, and build-time integration to fetch and cache remote content. Pages can now embed canonical documentation from GitHub via the ChangesRemote Markdown & Mermaid Rendering System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 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 (2)
components/TableOfContents.js (1)
20-49: ⚖️ Poor tradeoffHeading id generation from text may create duplicates.
Lines 24-27 generate a heading
idby slugifyingtextContentwhen the element lacks an existingid. If two headings share identical text (e.g., "Overview" appears twice), both will receive the sameid, which violates HTML uniqueness and breaks TOC link targeting.For typical documentation with distinct heading text this is unlikely, but consider appending a numeric suffix if a collision is detected.
♻️ Optional: De-duplicate heading IDs
const collectHeadings = () => { const elements = article.querySelectorAll("h2, h3, h4"); + const usedIds = new Set(); const items = Array.from(elements).map((el) => { if (!el.id) { - el.id = el.textContent + let baseId = el.textContent .toLowerCase() .replace(/[^a-z0-9]+/g, "-") .replace(/(^-|-$)/g, ""); + let id = baseId; + let counter = 1; + while (usedIds.has(id)) { + id = `${baseId}-${counter}`; + counter++; + } + el.id = id; } + usedIds.add(el.id); return { id: el.id, text: el.textContent, level: Number(el.tagName.slice(1)), }; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/TableOfContents.js` around lines 20 - 49, collectHeadings currently slugifies el.textContent and assigns el.id directly which can produce duplicate ids; change the id generation in collectHeadings to ensure uniqueness by checking existing ids (e.g., document.getElementById or a Set of collected ids) and appending a numeric suffix like "-1", "-2", etc., until an unused id is found before assigning el.id, then continue returning the item for setHeadings and keeping setActiveId("") behavior unchanged.components/RemoteMarkdown.js (1)
147-150: ⚡ Quick winConsider caching the transformed AST to avoid redundant parsing.
transformMarkdownis called three times per successful fetch: once at Line 188 (GitHub validation), once at Line 198 (cache validation), and once at Line 219 (final render memoization). The first two calls discard the result and only validate that parsing succeeds, but Markdoc parsing and transformation are not free.♻️ Refactor to parse once
Store the transformed content from the fetch effect and skip re-transformation in the render memo:
export function RemoteMarkdown({ source, children }) { ... - const [markdown, setMarkdown] = useState(null); + const [transformedContent, setTransformedContent] = useState(null); ... useEffect(() => { ... async function loadRemoteMarkdown() { ... try { const nextMarkdown = await fetchMarkdown(sourceInfo.rawUrl, controller.signal, 'GitHub'); - transformMarkdown(nextMarkdown); - setMarkdown(nextMarkdown); + const content = transformMarkdown(nextMarkdown); + setTransformedContent(content); return; } catch (err) { ... } try { const cachedMarkdown = await fetchMarkdown(rawCacheUrl, controller.signal, 'Build-time raw cache'); - transformMarkdown(cachedMarkdown); - setMarkdown(cachedMarkdown); + const content = transformMarkdown(cachedMarkdown); + setTransformedContent(content); } catch (err) { ... } } ... }, [rawCacheUrl, sourceInfo]); const renderedState = useMemo(() => { - if (!markdown || !sourceInfo) return { renderedContent: null, renderError: null }; + if (!transformedContent || !sourceInfo) return { renderedContent: null, renderError: null }; try { - const content = transformMarkdown(markdown); return { - renderedContent: Markdoc.renderers.react(content, React, { + renderedContent: Markdoc.renderers.react(transformedContent, React, { components: remoteMarkdocComponents, }), renderError: null, }; } catch (err) { return { renderedContent: null, renderError: err }; } - }, [markdown, sourceInfo]); + }, [transformedContent, sourceInfo]);Also applies to: 183-206, 215-229
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/RemoteMarkdown.js` around lines 147 - 150, transformMarkdown is being invoked multiple times per fetch; compute and store the transformed result once in the fetch effect and reuse it instead of re-parsing. In RemoteMarkdown's fetch handler, call Markdoc.parse/Markdoc.transform (via transformMarkdown) once, save the transformed AST/output into a state or ref (e.g., parsedDoc or transformedMarkdown), use that stored value for the GitHub validation and cache validation checks, and pass it into the memoized render instead of calling transformMarkdown again; keep stripFrontmatter/markdocConfig usage the same and wrap the single transform in try/catch to preserve error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/generate-raw-pages.js`:
- Around line 50-55: Add a timeout to the build-time fetch by passing an
AbortSignal to the fetch call (e.g., signal: AbortSignal.timeout(10000)) so
slow/unresponsive remote servers cancel after ~10s; update the fetch in
generate-raw-pages.js where rawUrl is fetched (affecting the
response/response.ok logic and returned response.text()) to include this signal
and handle the abort error as needed, and make the identical change in
generate-search-index.js for its fetch calls to keep behavior consistent across
both scripts.
- Around line 36-42: getFrontmatterRawUrl currently uses a frontmatter regex
that requires a trailing newline after the closing '---' and extracts rawUrl
with an inline quoted-value regex, causing inconsistency with
generate-search-index.js; update getFrontmatterRawUrl to use the same
frontmatter pattern as generate-search-index.js (do not require a trailing
newline after the closing '---') and replace the inline rawUrl extraction with
the shared stripYamlValue helper (import or extract stripYamlValue into a common
module) so quote normalization and parsing behavior are identical across both
scripts.
---
Nitpick comments:
In `@components/RemoteMarkdown.js`:
- Around line 147-150: transformMarkdown is being invoked multiple times per
fetch; compute and store the transformed result once in the fetch effect and
reuse it instead of re-parsing. In RemoteMarkdown's fetch handler, call
Markdoc.parse/Markdoc.transform (via transformMarkdown) once, save the
transformed AST/output into a state or ref (e.g., parsedDoc or
transformedMarkdown), use that stored value for the GitHub validation and cache
validation checks, and pass it into the memoized render instead of calling
transformMarkdown again; keep stripFrontmatter/markdocConfig usage the same and
wrap the single transform in try/catch to preserve error handling.
In `@components/TableOfContents.js`:
- Around line 20-49: collectHeadings currently slugifies el.textContent and
assigns el.id directly which can produce duplicate ids; change the id generation
in collectHeadings to ensure uniqueness by checking existing ids (e.g.,
document.getElementById or a Set of collected ids) and appending a numeric
suffix like "-1", "-2", etc., until an unused id is found before assigning
el.id, then continue returning the item for setHeadings and keeping
setActiveId("") behavior unchanged.
🪄 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: f54b1a5b-feff-4292-bd43-e77377b1bd54
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
components/CopyRawButton.jscomponents/Layout.jscomponents/MermaidDiagram.jscomponents/RemoteMarkdown.jscomponents/TableOfContents.jsdocs/remote-markdown.mdlib/generate-raw-pages.jslib/generate-search-index.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/tags/index.jsmarkdoc/tags/remote-doc.markdoc.jspackage.jsonpages/_app.jspages/architecture/epds.mdstyles/globals.css
Comment on lines
+36
to
+42
| function getFrontmatterRawUrl(markdown) { | ||
| const match = markdown.match(/^---\n([\s\S]*?)\n---\n/); | ||
| if (!match) return null; | ||
|
|
||
| const files = walkDir(PAGES_DIR); | ||
| const rawUrlMatch = match[1].match(/^rawUrl:\s*["']?([^"'\n]+)["']?\s*$/m); | ||
| return rawUrlMatch?.[1] || null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Inconsistent frontmatter and rawUrl extraction compared to generate-search-index.js.
The frontmatter regex on line 37 requires a trailing newline after the closing ---, but generate-search-index.js line 26 does not. Additionally, this file extracts rawUrl with an inline regex that handles quotes, while generate-search-index.js uses a cleaner stripYamlValue(...) utility for quote normalization.
Consider extracting a shared stripYamlValue helper (or importing from a common module) and using consistent frontmatter regex patterns across both scripts to reduce maintenance burden and edge-case divergence.
♻️ Proposed refactor for consistency
+function stripYamlValue(value) {
+ return value.trim().replace(/^['"]|['"]$/g, '');
+}
+
function getFrontmatterRawUrl(markdown) {
- const match = markdown.match(/^---\n([\s\S]*?)\n---\n/);
+ const match = markdown.match(/^---\n([\s\S]*?)\n---/);
if (!match) return null;
- const rawUrlMatch = match[1].match(/^rawUrl:\s*["']?([^"'\n]+)["']?\s*$/m);
- return rawUrlMatch?.[1] || null;
+ const rawUrlMatch = match[1].match(/^rawUrl:\s*(.+)$/m);
+ return rawUrlMatch ? stripYamlValue(rawUrlMatch[1]) : null;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/generate-raw-pages.js` around lines 36 - 42, getFrontmatterRawUrl
currently uses a frontmatter regex that requires a trailing newline after the
closing '---' and extracts rawUrl with an inline quoted-value regex, causing
inconsistency with generate-search-index.js; update getFrontmatterRawUrl to use
the same frontmatter pattern as generate-search-index.js (do not require a
trailing newline after the closing '---') and replace the inline rawUrl
extraction with the shared stripYamlValue helper (import or extract
stripYamlValue into a common module) so quote normalization and parsing behavior
are identical across both scripts.
Comment on lines
+50
to
+55
| const response = await fetch(rawUrl, { cache: 'no-store' }); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch rawUrl for ${relative(PAGES_DIR, file)}: ${rawUrl} returned ${response.status} ${response.statusText || ''}`.trim()); | ||
| } | ||
|
|
||
| return response.text(); |
There was a problem hiding this comment.
Add timeout protection to all build-time fetch operations.
Both generate-raw-pages.js and generate-search-index.js perform fetch calls without timeout. If any remote server is slow or unresponsive, the build will hang indefinitely. Node.js fetch supports signal: AbortSignal.timeout(ms) to automatically abort after a deadline. Recommend adding a consistent timeout (e.g., 10 seconds) to both scripts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/generate-raw-pages.js` around lines 50 - 55, Add a timeout to the
build-time fetch by passing an AbortSignal to the fetch call (e.g., signal:
AbortSignal.timeout(10000)) so slow/unresponsive remote servers cancel after
~10s; update the fetch in generate-raw-pages.js where rawUrl is fetched
(affecting the response/response.ok logic and returned response.text()) to
include this signal and handle the abort error as needed, and make the identical
change in generate-search-index.js for its fetch calls to keep behavior
consistent across both scripts.
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.
What changed
mermaiddiagrams.rawUrlcanonical sources.Validation
Summary by CodeRabbit
New Features
Style
Documentation
Chores