refactor: split out blog route from shared MdxRoute file#1213
refactor: split out blog route from shared MdxRoute file#1213jderochervlk wants to merge 17 commits intomasterfrom
Conversation
…nto vlk/split-out-blog-route
- Use String.startsWith instead of String.includes for blog route filtering to avoid accidentally excluding non-blog routes that contain 'blog' as a substring - Replace JsExn.throw with JsError.throwWithMessage in BlogArticleRoute for consistency with BlogApi.res and to get proper Error objects with stack traces - Normalize path separators in MdxFile.scanDir to fix Windows compatibility where Node.Path.join2 produces backslashes
- Use String.startsWith instead of String.includes for blog route filtering to avoid accidentally excluding non-blog routes that contain 'blog' as a substring - Replace JsExn.throw with JsError.throwWithMessage in BlogArticleRoute for consistency with BlogApi.res and to get proper Error objects with stack traces - Normalize path separators in MdxFile.scanDir to fix Windows compatibility where Node.Path.join2 produces backslashes
There was a problem hiding this comment.
Pull request overview
Refactors routing so blog article pages are handled by a dedicated route instead of being a special case inside the shared MdxRoute.
Changes:
- Added
BlogArticleRouteand updated route registration to generate per-post blog article routes from the filesystem. - Introduced
MdxFileutilities and updatedMarkdownParserto strip YAML frontmatter from rendered content. - Added
rehype-rawdependency/binding and updated Vitest/browser config plus refreshed screenshot artifacts.
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Locks new transitive deps for rehype-raw and related hast/parse5 utilities. |
| vitest.config.mjs | Minor config formatting/line adjustment around Playwright provider. |
| src/MdxFile.resi | Public interface for resolving/loading/scanning .mdx files. |
| src/MdxFile.res | Implements disk path resolution, file loading, and recursive .mdx scanning for route generation. |
| src/common/MarkdownParser.res | Adds a unified plugin to strip YAML frontmatter nodes from the output content. |
| src/bindings/Rehype.res | Adds ReScript binding for rehype-raw. |
| package.json | Adds rehype-raw dependency. |
| app/routes/MdxRoute.resi | Removes blogPost from MdxRoute loader data type. |
| app/routes/MdxRoute.res | Removes blog-specific loader/rendering branches (blog handled elsewhere now). |
| app/routes/BlogArticleRoute.resi | New route interface for blog articles. |
| app/routes/BlogArticleRoute.res | New loader/view for individual blog posts using filesystem reads + ReactMarkdown. |
| app/routes.res | Registers generated blog article routes and filters blog paths out of generic MDX routes. |
| AGENTS.md | Adds additional ReScript guidance for contributors. |
| tests/screenshots/Button_.test.jsx/button-secondary-red-chromium-linux.png | Updated screenshot artifact. |
| tests/screenshots/Button_.test.jsx/button-primary-blue-chromium-linux.png | Updated screenshot artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/routes/BlogArticleRoute.res
Outdated
| let default = () => { | ||
| let {content, blogPost: {frontmatter, archived, path}} = ReactRouter.useLoaderData() | ||
|
|
||
| <BlogArticle frontmatter isArchived=archived path> | ||
| <ReactMarkdown components=MarkdownComponents.default rehypePlugins=[Rehype.Plugin(Rehype.raw)]> |
There was a problem hiding this comment.
BlogArticleRoute renders blog posts via ReactMarkdown, but the blog content uses MDX-style components like <CodeTab labels={[...]} />. react-markdown treats HTML/JSX attributes as strings (it won’t evaluate the {[...]} expression), so labels will arrive as a string instead of an array<string>, which breaks Markdown.CodeTab.make (it indexes labels[i]). Consider switching blog articles back to the same MDX pipeline used elsewhere (e.g. loadMdx + useMdxComponent), or introduce a dedicated parsing layer that converts these props into the expected types before rendering.
| let default = () => { | |
| let {content, blogPost: {frontmatter, archived, path}} = ReactRouter.useLoaderData() | |
| <BlogArticle frontmatter isArchived=archived path> | |
| <ReactMarkdown components=MarkdownComponents.default rehypePlugins=[Rehype.Plugin(Rehype.raw)]> | |
| /* Rehype transformer to normalize <CodeTab labels={["A", "B"]}> props. | |
| `react-markdown` + `rehype-raw` will otherwise pass `labels` as a string, | |
| but `Markdown.CodeTab.make` expects `array<string>`. */ | |
| let codeTabLabelsPlugin = | |
| [%raw | |
| {| | |
| function (tree) { | |
| function normalizeLabels(value) { | |
| if (typeof value !== "string") return value; | |
| var raw = value.trim(); | |
| // Strip surrounding `{` ... `}` from MDX-style attributes: labels={["A","B"]} | |
| if (raw[0] === "{" && raw[raw.length - 1] === "}") { | |
| raw = raw.slice(1, -1).trim(); | |
| } | |
| // If it already looks like JSON (starts with [ and ends with ]), try JSON.parse directly. | |
| if (raw[0] === "[" && raw[raw.length - 1] === "]") { | |
| try { | |
| var parsed = JSON.parse(raw); | |
| if (Array.isArray(parsed)) { | |
| // Ensure all entries are strings | |
| return parsed.map(function (x) { return String(x); }); | |
| } | |
| } catch (_) { | |
| return value; | |
| } | |
| } | |
| // Fallback: try to turn something like ["A","B"] without quotes normalization into JSON. | |
| try { | |
| var normalized = raw.replace(/'/g, '"'); | |
| var parsed2 = JSON.parse(normalized); | |
| if (Array.isArray(parsed2)) { | |
| return parsed2.map(function (x) { return String(x); }); | |
| } | |
| } catch (_) { | |
| // Ignore and keep original value | |
| } | |
| return value; | |
| } | |
| function visit(node) { | |
| if (!node || typeof node !== "object") return; | |
| if (Array.isArray(node.children)) { | |
| for (var i = 0; i < node.children.length; i++) { | |
| visit(node.children[i]); | |
| } | |
| } | |
| if (node.type === "element" && node.tagName === "CodeTab" && node.properties) { | |
| if (Object.prototype.hasOwnProperty.call(node.properties, "labels")) { | |
| node.properties.labels = normalizeLabels(node.properties.labels); | |
| } | |
| } | |
| } | |
| visit(tree); | |
| } | |
| |} | |
| ] | |
| let default = () => { | |
| let {content, blogPost: {frontmatter, archived, path}} = ReactRouter.useLoaderData() | |
| <BlogArticle frontmatter isArchived=archived path> | |
| <ReactMarkdown | |
| components=MarkdownComponents.default | |
| rehypePlugins=[Rehype.Plugin(Rehype.raw), Rehype.Plugin(codeTabLabelsPlugin)] | |
| > |
app/routes/BlogArticleRoute.res
Outdated
| let {content, blogPost: {frontmatter, archived, path}} = ReactRouter.useLoaderData() | ||
|
|
||
| <BlogArticle frontmatter isArchived=archived path> | ||
| <ReactMarkdown components=MarkdownComponents.default rehypePlugins=[Rehype.Plugin(Rehype.raw)]> |
There was a problem hiding this comment.
Enabling rehype-raw allows raw HTML in markdown to be parsed into the output tree. If blog content can ever come from an untrusted source (or if contributors can submit markdown), this can introduce XSS vectors (e.g. javascript: links, unexpected attributes). If the content is strictly trusted, it’s fine—otherwise consider adding rehype-sanitize (with an allowlist for your custom tags) or removing rehype-raw.
| <ReactMarkdown components=MarkdownComponents.default rehypePlugins=[Rehype.Plugin(Rehype.raw)]> | |
| <ReactMarkdown components=MarkdownComponents.default> |
| } else { | ||
| pathname | ||
| } | ||
| let relativePath = path->String.replace(alias, dir) |
There was a problem hiding this comment.
resolveFilePath uses String.replace(alias, dir), which will replace the first occurrence of alias anywhere in the path, not necessarily as a path prefix. This can produce incorrect file paths if the alias appears later in the URL segment. Safer approach: explicitly strip a leading /${alias}/ (or ${alias}/) prefix and then Node.Path.join2(dir, rest) (or use a prefix-based replacement via regexp anchored at the start).
| let relativePath = path->String.replace(alias, dir) | |
| let relativePath = | |
| if path->String.startsWith(alias ++ "/") { | |
| let rest = path->String.slice(~start=String.length(alias) + 1, ~end=String.length(path)) | |
| Node.Path.join2(dir, rest) | |
| } else if path->String.startsWith(alias) { | |
| let rest = path->String.slice(~start=String.length(alias), ~end=String.length(path)) | |
| Node.Path.join2(dir, rest) | |
| } else { | |
| path | |
| } |
|
|
||
| let mdxRoutes = | ||
| mdxRoutes("./routes/MdxRoute.jsx")->Array.filter(r => | ||
| !(r.path->Option.map(String.startsWith(_, "blog"))->Option.getOr(false)) |
There was a problem hiding this comment.
The filter String.startsWith(_, "blog") will exclude any generated MDX route whose path happens to start with blog (e.g. blogger/...), not just blog/ and blog. To avoid surprising exclusions, consider matching "blog" exactly or the "blog/" prefix instead.
| !(r.path->Option.map(String.startsWith(_, "blog"))->Option.getOr(false)) | |
| !(r.path | |
| ->Option.map(path => path === "blog" || String.startsWith(path, "blog/")) | |
| ->Option.getOr(false) | |
| ) |
Cloudflare deploymentDeployement ID: 84ecc8ec-06aa-4628-bba6-bd41a0f05995 ⛅️ wrangler 4.63.0 (update available 4.80.0) ✨ Uploading _redirects |
Summary
Splits blog article rendering out of the monolithic
MdxRouteinto a dedicatedBlogArticleRoute, and introduces reusable infrastructure (MdxFile,MdxContent,CompiledMdx) that paves the way for splitting out the remaining routes (docs, community, syntax-lookup) and eventually removingreact-router-mdx.Motivation
MdxRoute.reshandles every MDX-backed page on the site — blog posts, docs, community pages, and syntax lookup — in a single loader and a long chain ofif/elsebranches. This makes it hard to reason about, test, and modify individual sections without risk of breaking others. Blog is the simplest case and a good first candidate to extract.What changed
New files
src/common/CompiledMdx.res/.resisrc/MdxFile.res/.resi.mdxfiles, and compile MDX via@mdx-js/mdx.src/components/MdxContent.res/.resi<MdxContent compiledMdx />React component. Evaluates compiled MDX usingrunSyncfrom@mdx-js/mdxwith proper ReScript bindings toreact/jsx-runtime(no%raw). Contains the shared component map used across all MDX pages.app/routes/BlogArticleRoute.res/.resisrc/common/MarkdownParser.resstripFrontmatterPluginto remove YAML nodes from the remark tree so frontmatter doesn't appear in rendered output.Modified files
app/routes.resBlogArticleRoute.jsx;mdxRoutesfilters outblog/*paths to avoid duplicates.app/routes/MdxRoute.res/.resiblogPost?field fromloaderDataand the blog-specific branch in both the loader and renderer.src/bindings/Rehype.resrehype-rawbinding.package.json/yarn.lockrehype-raw(used by other parts of the site, not by this route).How MDX rendering works
The blog route uses the same MDX pipeline as
react-router-mdxbut with our own code, which will make it possible to remove that dependency later:Server (loader):
Client (component):
This means custom MDX components like
<CodeTab labels={["RE", "JS"]}>work correctly — unlike the earlier approach of passing raw markdown toreact-markdown, which can't handle JSX expressions in attributes.Key design decisions
CompiledMdx.tis opaque — the.resiexposes onlytype tandfromCompileResult. You can't accidentally pass a random string where compiled MDX is expected.MdxContentowns the component map — since the same components are used across all MDX pages on the site, they live in one place rather than being duplicated in each route.%raw—MdxContentuses proper@modulebindings toreact/jsx-runtimeand@mdx-js/mdxinstead of raw JS. The jsx-runtime exports are imported as opaquejsxRuntimeValuetypes to sidestep ReScript's monomorphisation of the polymorphicReact.jsx/React.jsxssignatures.MdxFile.compileMdxcalls@mdx-js/mdxcompiledirectly, makingreact-router-mdxno longer needed for blog routes. The remaining routes still use it for now.