feat: Render JSX components in headings (ToC + nav sidebar)#1929
feat: Render JSX components in headings (ToC + nav sidebar)#1929
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Preview build of published Zudoku package for commit 317ee27. See the deployment at: https://1df8d923.cosmocargo-public-package.pages.dev Note This is a preview of the Cosmo Cargo example using the Zudoku package published to a local registry to ensure it'll be working when published to the public NPM registry. Last updated: 2026-02-18T13:12:52.550Z |
dan-lee
force-pushed
the
feat/rich-jsx-headings
branch
from
be798a7 to
e23cc24
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dan-lee
force-pushed
the
feat/rich-jsx-headings
branch
from
0141d18 to
44952b0
Compare
dan-lee
force-pushed
the
feat/rich-jsx-headings
branch
from
891f069 to
84b1383
Compare
dan-lee
force-pushed
the
feat/rich-jsx-headings
branch
from
84b1383 to
ea0c89b
Compare
📝 WalkthroughWalkthroughAdds support for preserving and rendering JSX content inside headings by extracting rich H1/TOC AST nodes, exposing them through navigation metadata, updating TOC/navigation components to render rich content via a new RichText renderer, and replacing the frontmatter plugin with a doc-metadata plugin that tracks document metadata and H1 changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Vite as Vite/MDX
participant Rehype as Rehype TOC Extractor
participant Plugin as Doc-Metadata Plugin
participant Nav as NavigationResolver
participant UI as Navigation/TOC Components
participant Rich as RichText Renderer
Dev->>Vite: Load MDX file with JSX in H1
Vite->>Rehype: Parse AST, extract headings (mark rich nodes)
Rehype-->>Vite: Attach Toc (with rich AST) to vfile.data
Vite->>Plugin: Collect frontmatter + top-level H1 (rich)
Plugin-->>Nav: Provide metadata + rich H1
Nav->>UI: Emit NavigationDoc with optional `rich` content
UI->>Rich: Render `rich` AST nodes via RichText
Rich->>UI: Return rendered JSX components
UI-->>Dev: Display JSX inside headings/TOC/sidenav
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
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)
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
🤖 Fix all issues with AI agents
In `@packages/zudoku/src/config/validators/NavigationSchema.ts`:
- Around line 134-139: The code in NavigationSchema.ts unsafely casts mdast
children (variable children of type MdxPhrasingContent[]) to hast RootContent[]
when returning rich for hasJsx, which masks AST differences; either explicitly
convert mdast->hast using an available utility or, if safe in practice, add a
clear explanatory comment next to the cast (referencing hasJsx, children, and
RichText) documenting why the cast is acceptable; also replace the silent catch
with error handling that logs or rethrows the caught error so MDX parsing issues
aren't hidden (e.g., surface the error via processLogger or throw a wrapped
error).
🧹 Nitpick comments (7)
packages/zudoku/src/vite/plugin-doc-metadata.ts (3)
8-12: Consider adding error handling for file read failures.If
readFrontmatterthrows (e.g., file deleted between glob scan and read, or permission issues), the error will propagate unhandled. This could crash the initial plugin setup or leave the watcher callback in a failed state.🛡️ Proposed fix with error handling
const serializeMetadata = async (filePath: string): Promise<string> => { - const fm = await readFrontmatter(filePath); - const h1 = fm.content.match(/^#\s+(.*)$/m)?.[1]; - return JSON.stringify({ frontmatter: fm.data, h1 }); + try { + const fm = await readFrontmatter(filePath); + const h1 = fm.content.match(/^#\s+(.*)$/m)?.[1]; + return JSON.stringify({ frontmatter: fm.data, h1 }); + } catch { + return JSON.stringify({ frontmatter: {}, h1: undefined }); + } };
19-22: Glob ignore patterns may not exclude nested directories.Simple strings like
"node_modules"only match that exact directory at the cwd level. Files in nestednode_modulesdirectories (e.g.,packages/foo/node_modules/) may still be included.♻️ Use glob patterns for nested exclusion
const files = await glob("**/*.{md,mdx}", { cwd: config.__meta.rootDir, - ignore: ["node_modules", "dist"], + ignore: ["**/node_modules/**", "**/dist/**"], absolute: true, });
33-43: File deletions are not handled.When a file is deleted (
unlinkevent), it remains inmetadataMap. While this is a minor memory leak during dev sessions, stale entries could cause unexpected behavior if the navigation relies on this map's contents.♻️ Handle file deletion
server.watcher.on("all", async (event, filePath) => { - if (event !== "change" && event !== "add") return; if (!/\.mdx?$/.test(filePath)) return; + + if (event === "unlink") { + if (metadataMap.delete(filePath)) { + invalidateNavigation(server); + reload(server); + } + return; + } + + if (event !== "change" && event !== "add") return; const metadata = await serializeMetadata(filePath);packages/zudoku/src/lib/components/navigation/NavigationItem.tsx (1)
109-115: Consider truncation behavior for rich text items.When
item.richis present, theRichTextcomponent renders without truncation or tooltip support. For plain labels,TruncatedLabelprovides a tooltip when text overflows. Long JSX-rendered content could overflow without user feedback.This may be intentional if rich content is expected to be short (badges, small components), but consider whether truncation/tooltip behavior should be consistent.
packages/zudoku/src/vite/mdx/rehype-extract-toc-with-jsx.ts (1)
32-33: Type assertion is safe, but clarify intent of hasRichContent check with comments.The cast
node.children as RootContent[]is semantically correct:ElementContent(Comment | Element | Text) is fully compatible withRootContent(Comment | Doctype | Element | Text). TheDoctypenode type inRootContentwill never appear in element children, so the cast carries no safety risk.However, the
hasRichContentcheck on line 33 usingchild.type !== "text"will match bothelementandcommentnodes. Consider whether HTML comments in headings should be treated as rich content, or whether comments should be explicitly excluded.packages/zudoku/src/lib/util/MdxComponents.tsx (1)
3-3: Inconsistent import paths for UI components.
Alertis imported via package alias (zudoku/ui/Alert.js) whileBadgeuses a relative path (../ui/Badge.js). Consider using consistent import style for maintainability.♻️ Suggested fix
-import { Alert } from "zudoku/ui/Alert.js"; +import { Alert } from "../ui/Alert.js";Or alternatively, update Badge to use the package alias if that's the preferred convention.
Also applies to: 10-10
packages/zudoku/src/lib/util/hastToJsx.tsx (1)
27-40: Complex JSX attribute expressions are silently ignored.When an MDX JSX element has attribute expressions (e.g.,
<Badge variant={someVariable}>), these are silently dropped since only primitive values and null are handled. This is likely intentional for the ToC/nav use case, but could lead to unexpected behavior if users expect dynamic expressions to work.Consider adding a comment explaining this limitation, or logging a warning in development mode when expression attributes are encountered.
dan-lee
force-pushed
the
feat/rich-jsx-headings
branch
from
ea0c89b to
317ee27
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zudoku/src/config/validators/NavigationSchema.ts`:
- Around line 113-136: extractRichH1 only checks direct H1 children for JSX so
JSX wrapped inside emphasis/strong/link nodes is missed; update extractRichH1 to
perform a recursive/descendant search (e.g., add a helper like containsJsx(node)
that returns true if node isMdxJsxElement or any of its children contain JSX)
and replace the hasJsx = children.some(isMdxJsxElement) line with hasJsx =
children.some(child => containsJsx(child)); keep returning { label, rich:
children as RootContent[] } when hasJsx is true.
In `@packages/zudoku/src/lib/util/MdxComponents.tsx`:
- Line 10: The Badge component is imported via a relative path in
MdxComponents.tsx; replace the relative import of Badge (currently from
"../ui/Badge.js") with the shared module import from "zudoku/ui" so the file
imports Badge from the central UI package (keep the named import Badge and leave
usages unchanged). Ensure the import statement uses the exact module specifier
"zudoku/ui" and remove the old relative import.
In `@packages/zudoku/src/vite/mdx/rehype-extract-toc-with-jsx-export.ts`:
- Around line 9-11: Replace the plain Error thrown when an invalid export name
is detected with the project's custom ZudokuError: in the branch that checks
isIdentifierName(name) and currently does throw new Error(...), import or
reference ZudokuError and throw new ZudokuError(`Invalid identifier name:
${JSON.stringify(name)}`) (keep the original message content), so the check in
isIdentifierName(name) uses the project-specific error class; ensure the
ZudokuError import is added if not already present.
| // Extract rich H1 heading content from MDX. Returns AST nodes only when H1 contains JSX elements. | ||
| const extractRichH1 = (content: string) => { | ||
| try { | ||
| const mdast = fromMarkdown(content, { | ||
| extensions: [mdxjs()], | ||
| mdastExtensions: [mdxFromMarkdown()], | ||
| // biome-ignore lint/suspicious/noExplicitAny: mdast-util-from-markdown has type incompatibilities between versions | ||
| } as any); | ||
|
|
||
| const h1 = mdast.children.find( | ||
| (node): node is Heading => node.type === "heading" && node.depth === 1, | ||
| ); | ||
|
|
||
| if (!h1) return undefined; | ||
|
|
||
| const children = h1.children as MdxPhrasingContent[]; | ||
| const hasJsx = children.some(isMdxJsxElement); | ||
|
|
||
| // Extract all text content including from emphasis/strong/links | ||
| const label = mdastToString(h1).trim(); | ||
|
|
||
| // Note: rich only contains MDAST nodes. RichText handles text and mdxJsxTextElement, | ||
| // but markdown formatting (strong/emphasis/link) in H1 won't render styled. | ||
| return hasJsx ? { label, rich: children as RootContent[] } : { label }; |
There was a problem hiding this comment.
Detect JSX nested within formatted heading nodes.
hasJsx only checks direct H1 children. If JSX is wrapped by emphasis/strong/link nodes, rich won’t be set and the nav/ToC will still lose JSX. A recursive check avoids that gap.
Proposed fix
const isMdxJsxElement = (node: MdxPhrasingContent): node is MdxJsxTextElement =>
node.type === "mdxJsxTextElement";
+const containsJsx = (node: MdxPhrasingContent): boolean => {
+ if (isMdxJsxElement(node)) return true;
+ if ("children" in node && Array.isArray(node.children)) {
+ return node.children.some((child) =>
+ containsJsx(child as MdxPhrasingContent),
+ );
+ }
+ return false;
+};
+
const mdastToString = (node: MdxPhrasingContent | Heading): string => {
if ("value" in node && typeof node.value === "string") return node.value;
if ("children" in node && Array.isArray(node.children)) {
return node.children
.map((c) => mdastToString(c as MdxPhrasingContent))
.join("");
}
return "";
};
@@
if (!h1) return undefined;
const children = h1.children as MdxPhrasingContent[];
- const hasJsx = children.some(isMdxJsxElement);
+ const hasJsx = children.some(containsJsx);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/zudoku/src/config/validators/NavigationSchema.ts` around lines 113 -
136, extractRichH1 only checks direct H1 children for JSX so JSX wrapped inside
emphasis/strong/link nodes is missed; update extractRichH1 to perform a
recursive/descendant search (e.g., add a helper like containsJsx(node) that
returns true if node isMdxJsxElement or any of its children contain JSX) and
replace the hasJsx = children.some(isMdxJsxElement) line with hasJsx =
children.some(child => containsJsx(child)); keep returning { label, rich:
children as RootContent[] } when hasJsx is true.
| import { InlineCode } from "../components/InlineCode.js"; | ||
| import { Mermaid } from "../components/Mermaid.js"; | ||
| import { HIGHLIGHT_CODE_BLOCK_CLASS } from "../shiki.js"; | ||
| import { Badge } from "../ui/Badge.js"; |
There was a problem hiding this comment.
Align Badge import with zudoku/ui.
Badge is pulled via a relative path; use the zudoku/ui module import for consistency with the UI import guideline.
Suggested fix
-import { Badge } from "../ui/Badge.js";
+import { Badge } from "zudoku/ui/Badge.js";As per coding guidelines: Use UI components from the zudoku/ui module based on shadcn/ui.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Badge } from "../ui/Badge.js"; | |
| import { Badge } from "zudoku/ui/Badge.js"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/zudoku/src/lib/util/MdxComponents.tsx` at line 10, The Badge
component is imported via a relative path in MdxComponents.tsx; replace the
relative import of Badge (currently from "../ui/Badge.js") with the shared
module import from "zudoku/ui" so the file imports Badge from the central UI
package (keep the named import Badge and leave usages unchanged). Ensure the
import statement uses the exact module specifier "zudoku/ui" and remove the old
relative import.
| if (!isIdentifierName(name)) { | ||
| throw new Error(`Invalid identifier name: ${JSON.stringify(name)}`); | ||
| } |
There was a problem hiding this comment.
Use ZudokuError for invalid export names.
Guidelines require custom errors to throw/extend ZudokuError; please swap the thrown error accordingly.
As per coding guidelines: Throw and/or extend ZudokuError for custom errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/zudoku/src/vite/mdx/rehype-extract-toc-with-jsx-export.ts` around
lines 9 - 11, Replace the plain Error thrown when an invalid export name is
detected with the project's custom ZudokuError: in the branch that checks
isIdentifierName(name) and currently does throw new Error(...), import or
reference ZudokuError and throw new ZudokuError(`Invalid identifier name:
${JSON.stringify(name)}`) (keep the original message content), so the check in
isIdentifierName(name) uses the project-specific error class; ensure the
ZudokuError import is added if not already present.
2 participants
RichTextcomponent converts HAST/MDX AST nodes to Reacth1s at build time via mdast/micromarkExample from Cosmo Cargo:
Fix #1440
Summary by CodeRabbit
New Features
Documentation