Skip to content

Commit 1ae7265

Browse files
jderochervlkjderochervlk
committed
empty commit to test verification
1 parent 6c9d307 commit 1ae7265

File tree

1 file changed

+313
-0
lines changed

1 file changed

+313
-0
lines changed

‎PLAN-simplify-mdx-further.md‎

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
# Plan: Further Simplify MDX & Markdown Code
2+
3+
## Current State (post Phase 1-4 cleanup)
4+
5+
The MDX/markdown infrastructure is functional but has accumulated structural debt:
6+
7+
- **`Mdx.res`** is a grab-bag: domain types, sidebar utilities, remark plugins, and AST helpers all in one file
8+
- **`Mdx.attributes`** is a super-union of every frontmatter field across blog, docs, community, and syntax-lookup
9+
- **Dead types** remain in `Mdx.res` (`t`, `tree`, `badge`)
10+
- **Duplicate types** exist between `Mdx.res` and `BlogFrontmatter.res` (`social`, `author`)
11+
- **`MdxFile.loadFile`** is dead code
12+
- **`MarkdownParser.res`** exposes all internals (no `.resi`)
13+
- **Double frontmatter parsing**: every route runs both `MarkdownParser.parseSync` and `MdxFile.compileMdx` on the same raw content; the compile step internally re-parses frontmatter via `remark-frontmatter`
14+
- **Identical boilerplate** across 7 route loaders: `resolveFilePath -> readFile -> parseSync -> compileMdx`
15+
- **Identical ToC extraction** copy-pasted across 4 routes
16+
- **Identical frontmatter field extraction** (`switch frontmatter | Object(dict) => switch dict->Dict.get("field") ...`) repeated 10+ times
17+
18+
### File inventory
19+
20+
| File | Role | External API surface |
21+
| -------------------------------- | --------------------------------------- | ------------------------------------------------------------------------------------------ |
22+
| `src/Mdx.res` | Types + sidebar utils + remark plugins | `attributes`, `remarkPlugin`, `plugins`, `filterMdxPages`, `groupBySection`, `sortSection` |
23+
| `src/MdxFile.res` | File I/O + MDX compilation | `compileMdx`, `compileMarkdown`, `resolveFilePath`, `scanPaths`, `loadAllAttributes` |
24+
| `src/common/MarkdownParser.res` | Unified/remark pipeline for frontmatter | `parseSync`, `result` |
25+
| `src/common/CompiledMdx.res` | Opaque compiled MDX string | `t`, `compileResult`, `fromCompileResult` |
26+
| `src/components/MdxContent.res` | Runtime MDX evaluation + rendering | `render`, `make`, `components` |
27+
| `src/components/Markdown.res` | 22+ styled React components for MDX | All sub-modules (P, H1, H2, Code, A, Li, etc.) |
28+
| `src/bindings/Mdast.res` | mdast-util bindings for ToC extraction | `fromMarkdown`, `toc`, `reduceHeaders` |
29+
| `src/common/BlogFrontmatter.res` | Blog frontmatter decoder | `decode`, `t`, `author`, `social`, `Badge.t` |
30+
| `src/common/BlogApi.res` | Blog post listing from filesystem | `getAllPosts`, `getLivePosts`, `getArchivedPosts`, `RssFeed` |
31+
32+
---
33+
34+
## Goals
35+
36+
1. Remove dead code
37+
2. Eliminate duplicate types
38+
3. Reduce route loader boilerplate
39+
4. Encapsulate internal modules behind `.resi` files
40+
5. Give `Mdx.res` a clearer single responsibility
41+
42+
---
43+
44+
## Step-by-step Plan
45+
46+
### Phase 1: Delete dead code
47+
48+
**Low risk, no behavior change.**
49+
50+
1. Delete `Mdx.badge` variant type (dead -- `attributes.badge` is `Nullable.t<string>`, not `badge`)
51+
2. Delete `Mdx.t` type (dead -- never referenced; all compiled MDX uses `CompiledMdx.t`)
52+
3. Delete `Mdx.tree` type (dead -- never referenced externally; the `visit` binding uses `{..}` internally)
53+
4. Delete `MdxFile.loadFile` function (dead -- never called)
54+
55+
#### Files to modify
56+
57+
| File | Change |
58+
| ----------------- | ------------------------------------------ |
59+
| `src/Mdx.res` | Remove `type badge`, `type t`, `type tree` |
60+
| `src/MdxFile.res` | Remove `loadFile` function |
61+
62+
---
63+
64+
### Phase 2: Deduplicate `social` and `author` types
65+
66+
**Low risk. `BlogFrontmatter.res` is the canonical owner of these types since it defines the author registry and decoder.**
67+
68+
`Mdx.res` and `BlogFrontmatter.res` both define identical `social` and `author` types. The only reason they exist in `Mdx.res` is that `Mdx.attributes.co_authors` is typed as `Nullable.t<array<author>>`. But `Mdx.attributes` is only ever populated via an unsafe `%identity` cast in `MdxFile.loadAllAttributes`, so the structural shape is what matters, not the type provenance.
69+
70+
1. Remove `type social` and `type author` from `Mdx.res`
71+
2. Change `Mdx.attributes.co_authors` to reference `BlogFrontmatter.author`:
72+
```
73+
co_authors: Nullable.t<array<BlogFrontmatter.author>>,
74+
```
75+
76+
#### Files to modify
77+
78+
| File | Change |
79+
| ------------- | -------------------------------------------------------------------------------------------------- |
80+
| `src/Mdx.res` | Remove `social` and `author` types; update `attributes.co_authors` to use `BlogFrontmatter.author` |
81+
82+
---
83+
84+
### Phase 3: Add frontmatter helper and reduce route boilerplate
85+
86+
**Medium risk. Touches every route loader, but each change is mechanical.**
87+
88+
#### 3a: Create `Frontmatter.res` helper module
89+
90+
The pattern below appears 10+ times across route loaders:
91+
92+
```rescript
93+
let title = switch frontmatter {
94+
| Object(dict) =>
95+
switch dict->Dict.get("title") {
96+
| Some(String(s)) => s
97+
| _ => ""
98+
}
99+
| _ => ""
100+
}
101+
```
102+
103+
Create a small helper:
104+
105+
```rescript
106+
// src/common/Frontmatter.res
107+
let getString = (json: JSON.t, key: string): string =>
108+
switch json {
109+
| Object(dict) =>
110+
switch dict->Dict.get(key) {
111+
| Some(String(s)) => s
112+
| _ => ""
113+
}
114+
| _ => ""
115+
}
116+
117+
let getOptionalString = (json: JSON.t, key: string): option<string> =>
118+
switch json {
119+
| Object(dict) =>
120+
switch dict->Dict.get(key) {
121+
| Some(String(s)) => Some(s)
122+
| _ => None
123+
}
124+
| _ => None
125+
}
126+
```
127+
128+
Then every route replaces 8-line blocks with one-liners:
129+
130+
```rescript
131+
let title = Frontmatter.getString(frontmatter, "title")
132+
let description = Frontmatter.getString(frontmatter, "description")
133+
```
134+
135+
#### 3b: Create `MdxLoader.res` shared loader pipeline
136+
137+
Every route loader repeats the same 5-step core. Extract it:
138+
139+
```rescript
140+
// src/common/MdxLoader.res
141+
142+
type pageData = {
143+
compiledMdx: CompiledMdx.t,
144+
frontmatter: JSON.t,
145+
filePath: string,
146+
raw: string,
147+
}
148+
149+
let loadPage = async (~dir: string, ~alias: string, ~request: WebAPI.Request.t) => {
150+
let {pathname} = WebAPI.URL.make(~url=request.url)
151+
let filePath = MdxFile.resolveFilePath((pathname :> string), ~dir, ~alias)
152+
let raw = await Node.Fs.readFile(filePath, "utf-8")
153+
let {frontmatter}: MarkdownParser.result = MarkdownParser.parseSync(raw)
154+
let compiledMdx = await MdxFile.compileMdx(raw, ~filePath, ~remarkPlugins=Mdx.plugins)
155+
{compiledMdx, frontmatter, filePath, raw}
156+
}
157+
```
158+
159+
#### 3c: Extract shared ToC builder
160+
161+
The identical Mdast ToC block in 4 routes becomes:
162+
163+
```rescript
164+
// src/common/MdxLoader.res (continued)
165+
166+
let buildToc = (raw: string): array<TableOfContents.entry> => {
167+
let markdownTree = Mdast.fromMarkdown(raw)
168+
let tocResult = Mdast.toc(markdownTree, {maxDepth: 2})
169+
let headers = Dict.make()
170+
Mdast.reduceHeaders(tocResult.map, headers)
171+
headers
172+
->Dict.toArray
173+
->Array.map(((header, url)): TableOfContents.entry => {
174+
header,
175+
href: (url :> string),
176+
})
177+
->Array.slice(~start=2)
178+
}
179+
```
180+
181+
#### Route loaders after refactoring (example: DocsManualRoute)
182+
183+
Before (~40 lines of loader boilerplate):
184+
185+
```rescript
186+
let loader: ReactRouter.Loader.t<loaderData> = async ({request}) => {
187+
let {pathname} = WebAPI.URL.make(~url=request.url)
188+
let filePath = MdxFile.resolveFilePath(...)
189+
let raw = await Node.Fs.readFile(filePath, "utf-8")
190+
let {frontmatter}: MarkdownParser.result = MarkdownParser.parseSync(raw)
191+
let title = switch frontmatter { | Object(dict) => ... }
192+
let description = switch frontmatter { | Object(dict) => ... }
193+
let compiledMdx = await MdxFile.compileMdx(raw, ~filePath, ~remarkPlugins=Mdx.plugins)
194+
let markdownTree = Mdast.fromMarkdown(raw)
195+
let tocResult = Mdast.toc(markdownTree, {maxDepth: 2})
196+
let headers = Dict.make()
197+
Mdast.reduceHeaders(tocResult.map, headers)
198+
let entries = headers->Dict.toArray->Array.map(...)
199+
let categories = await manualTableOfContents()
200+
{ compiledMdx, categories, entries, title, description, filePath }
201+
}
202+
```
203+
204+
After (~15 lines):
205+
206+
```rescript
207+
let loader: ReactRouter.Loader.t<loaderData> = async ({request}) => {
208+
let {compiledMdx, frontmatter, filePath, raw} =
209+
await MdxLoader.loadPage(~dir="markdown-pages/docs/manual", ~alias="docs/manual", ~request)
210+
let title = Frontmatter.getString(frontmatter, "title")
211+
let description = Frontmatter.getString(frontmatter, "description")
212+
let entries = MdxLoader.buildToc(raw)
213+
let categories = await manualTableOfContents()
214+
{ compiledMdx, categories, entries, title, description, filePath }
215+
}
216+
```
217+
218+
#### Files to create
219+
220+
| File | Purpose |
221+
| ---------------------------- | ----------------------------------------------- |
222+
| `src/common/Frontmatter.res` | JSON frontmatter field extraction helpers |
223+
| `src/common/MdxLoader.res` | Shared loader pipeline (`loadPage`, `buildToc`) |
224+
225+
#### Files to modify
226+
227+
| File | Change |
228+
| ---------------------------------------- | ------------------------------------------------------------------------- |
229+
| `app/routes/ApiOverviewRoute.res` | Use `MdxLoader.loadPage` + `Frontmatter.getString` |
230+
| `app/routes/BlogArticleRoute.res` | Use `MdxLoader.loadPage` + keep blog-specific slug logic |
231+
| `app/routes/CommunityRoute.res` | Use `MdxLoader.loadPage` + `MdxLoader.buildToc` + `Frontmatter.getString` |
232+
| `app/routes/DocsGuidelinesRoute.res` | Use `MdxLoader.loadPage` + `MdxLoader.buildToc` + `Frontmatter.getString` |
233+
| `app/routes/DocsManualRoute.res` | Use `MdxLoader.loadPage` + `MdxLoader.buildToc` + `Frontmatter.getString` |
234+
| `app/routes/DocsReactRoute.res` | Use `MdxLoader.loadPage` + `MdxLoader.buildToc` + `Frontmatter.getString` |
235+
| `app/routes/SyntaxLookupDetailRoute.res` | Use `MdxLoader.loadPage` + `Frontmatter.getString` |
236+
237+
---
238+
239+
### Phase 4: Encapsulate `MarkdownParser` internals
240+
241+
**No risk. Only adds a `.resi` file to hide internal plumbing.**
242+
243+
Only `parseSync` and `result` are used outside `MarkdownParser.res`. All other symbols (the unified processor, individual plugins, custom plugin functions, internal types) are implementation details.
244+
245+
#### Files to create
246+
247+
| File | Content |
248+
| -------------------------------- | ---------------------------------------------------------------------------------------------------------- |
249+
| `src/common/MarkdownParser.resi` | Expose only `type result = { frontmatter: JSON.t, content: string }` and `let parseSync: string => result` |
250+
251+
---
252+
253+
### Phase 5: Clean up `Mdx.res` responsibilities
254+
255+
**Low risk. Rename/reorganize for clarity.**
256+
257+
After Phases 1-4, `Mdx.res` contains three distinct concerns:
258+
259+
1. **`attributes` type** -- the frontmatter schema for MDX pages
260+
2. **Sidebar utilities** -- `filterMdxPages`, `groupBySection`, `sortSection`
261+
3. **Remark plugins** -- `remarkLinkPlugin`, `remarkReScriptPreludePlugin`, `anchorLinkPlugin`, and the bundled `plugins` array
262+
263+
These are all tightly coupled (the sidebar utilities operate on `attributes`, the plugins are always passed alongside `attributes`-producing loaders), so splitting into separate files would create more indirection than clarity. Instead, add a `.resi` to hide internals and document the three groups:
264+
265+
#### Files to create
266+
267+
| File | Content |
268+
| -------------- | --------------------------------------------------------------------------------------------------------------------------------- |
269+
| `src/Mdx.resi` | Expose only: `type attributes`, `type remarkPlugin`, `let plugins`, `let filterMdxPages`, `let groupBySection`, `let sortSection` |
270+
271+
This hides: `gfm`, `childrenToString`, `visit`, `makePlugin`, `remarkReScriptPrelude`, `remarkLinkPlugin` (function), `remarkReScriptPreludePlugin`, `anchorLinkPlugin`, and the re-exported `remarkLinkPlugin` (plugin).
272+
273+
---
274+
275+
## Execution Order
276+
277+
```
278+
Phase 1 (dead code) -- standalone, no dependencies
279+
Phase 2 (dedupe types) -- standalone, no dependencies
280+
Phase 3a (Frontmatter helper) -- standalone, new file
281+
Phase 3b (MdxLoader helper) -- depends on 3a
282+
Phase 3c (route refactors) -- depends on 3a + 3b
283+
Phase 4 (MarkdownParser resi) -- standalone
284+
Phase 5 (Mdx resi) -- depends on Phase 1 + 2
285+
```
286+
287+
Phases 1, 2, 3a, and 4 can be done in parallel. Phase 3b depends on 3a. Phase 3c depends on 3b. Phase 5 depends on 1 and 2.
288+
289+
---
290+
291+
## Impact Summary
292+
293+
| Metric | Before | After |
294+
| ------------------------------------- | ------------------------ | ------------------------------------------ |
295+
| Dead types in `Mdx.res` | 3 (`t`, `tree`, `badge`) | 0 |
296+
| Dead functions in `MdxFile.res` | 1 (`loadFile`) | 0 |
297+
| Duplicate type definitions | 2 (`social`, `author`) | 0 |
298+
| Lines of loader boilerplate per route | ~35-45 | ~10-15 |
299+
| Repeated frontmatter switch blocks | 10+ | 0 |
300+
| Repeated ToC extraction blocks | 4 | 0 |
301+
| Modules with `.resi` encapsulation | 1 (`CompiledMdx`) | 3 (`CompiledMdx`, `MarkdownParser`, `Mdx`) |
302+
303+
---
304+
305+
## Risks & Considerations
306+
307+
1. **`Mdx.attributes` is populated via `%identity` cast.** Changing the type shape (Phase 2) doesn't affect runtime behavior since the cast ignores types. But if `BlogFrontmatter.author` ever diverges structurally from the old `Mdx.author`, the cast would silently produce bad data. This is a pre-existing risk, not a new one.
308+
309+
2. **`MdxLoader.loadPage` returns `raw` for ToC building.** This means the raw content travels through the loader return. Since `raw` is only used server-side (in the loader itself), it should NOT be included in `loaderData` sent to the client. The `raw` field is consumed in the loader and discarded before returning `loaderData`.
310+
311+
3. **Blog and API routes are special cases.** `BlogArticleRoute` uses `BlogFrontmatter.decode` instead of `Frontmatter.getString`. `ApiRoute` doesn't load MDX files at all (it reads JSON and compiles individual docstrings). Neither should be forced into the `MdxLoader.loadPage` pattern if it doesn't fit naturally. The blog route CAN use `loadPage` and then layer its own logic on top. The API route should be left alone.
312+
313+
4. **Adding `.resi` files is a one-way door.** Once added, removing them requires auditing all call sites. But since these modules already have a clear internal/external split, this is low risk.

0 commit comments

Comments
 (0)