fix(docs): build the On-this-page rail from the compiled MDX toc - #9953
Merged
Conversation
Closes #9872. Follow-up to #9588, which fixed 26 of 27 react-hooks 7 sites. DocsToc scanned the rendered article for h2/h3, back-filled ids onto any heading missing one, and read the result into state from an effect. That was the app's last react-hooks/set-state-in-effect site -- the reason the rule sat at warn -- and it was fragile independently of lint: the rail was a function of the rendered markup, so a styling change to the article wrapper silently emptied it. It now renders the page's compiled toc. The rule is back at error with no suppressions. WHY THE TOC CANNOT GO THROUGH THE ROUTE LOADER, which is what the issue flagged as needing resolution first: a toc entry's title is a ReactNode, and 29 of this repo's 451 docs headings carry inline markup, so those entries are React elements and do not survive the server-function JSON boundary. It is read on the client instead, from the same docsClientLoader cache the page content already resolves through, so this adds no second fetch. useChildMatches supplies the compiled path, which only the MDX route's loader has. Rendering title as a node is also a small improvement: a heading like 'wantedPaths (string list)' keeps its inline code, where reading textContent off the DOM flattened it. The id back-fill is gone because fumadocs already emits ids that match the toc's own urls, and the scroll-margin the effect used to assign in JS is a CSS class on the prose wrapper now. Non-MDX docs routes render no rail. The index page's only headings are its card titles, which produced a list of link names rather than a table of contents, and the API-reference spike is a standalone widget. Deliberate, per the issue's 'or deliberately opt out'. VERIFIED IN A BROWSER for structure: 11 entries on a long page, every anchor target present, h2/h3 nesting correct, inline code rendered as a real element, scroll-margin 80px. Active-section highlighting could NOT be exercised there -- IntersectionObserver does not deliver callbacks in that pane at all, including a plain observer with no options -- so that path is covered by unit tests rather than by the browser check.
Contributor
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-30 20:13:33 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | c36594a | Commit Preview URL Branch Preview URL |
Jul 30 2026, 08:08 PM |
Bundle ReportChanges will increase total bundle size by 820 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
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.
Closes #9872 — the last of the 27 react-hooks 7 sites from #9588.
react-hooks/set-state-in-effectis back aterrorwith no suppressions.The blocker the issue named, resolved
There was a third, which decided the design: the toc cannot travel through the route loader. A toc entry's
titleis aReactNode, and 29 of this repo's 451 docs headings carry inline markup (### \wantedPaths` (string list, default: `[]`)`), so those entries are React elements and do not survive the server-function JSON boundary.So it is read on the client, from the same
docsClientLoadercache the page content already resolves through — no second fetch.useChildMatchessupplies the compiled path, which only the MDX route's loader has.What that removes
The DOM scrape, the id back-fill, and the
setStatein the effect body, together. What remains is an IntersectionObserver for active-section tracking — real external-system work, and it sets state only from the observer callback, never from the effect body.Two incidental wins: rendering
titleas a node means a heading keeps its inline code instead of being flattened bytextContent; and thescrollMarginTopthe effect used to assign in JS is now a CSS class.Non-MDX routes render no rail — deliberate, per the issue's "or deliberately opt out". The index page's only headings are its card titles, which produced a list of link names rather than a table of contents; the API-reference spike is a standalone widget.
Browser verification, and its limit
Structure verified live at 1400×900 against
/docs/fairness-methodologyand/docs/ams-fleet-manifest:<code>, not flattened textscroll-mt-20replaces the JS assignment80pxActive-section highlighting could not be exercised there. IntersectionObserver delivers no callbacks in that pane — I confirmed a plain observer with no options also fires nothing, so this is an environment limit, not a defect in the change. That path is covered by unit tests instead (aria-current from a remembered section, and the click path), and I am flagging it rather than implying the whole component was verified end-to-end.
Worth stating plainly: I initially read that silence as a real bug and wrote a code comment asserting it. That was wrong, and the comment now says what is actually known.
The retry loop
The headings are rendered by the page, in a different Suspense boundary from the rail. A single up-front
getElementByIdsweep would observe nothing permanently on any commit where the rail lands first, since the effect's deps do not change when content later appears. Both boundaries suspend on the same cached promise and usually commit together, so this is a guard against an ordering hazard, not a fix for an observed failure — the comment says so. It costs one pass and schedules no frame when the headings are already present, and is bounded so a toc entry whose heading never renders cannot leave a loop running.Validation
ui:test1,099 passing (7 new),ui:lintandui:typecheckclean with the rule aterror,ui:buildgreen.