Repository navigation
docs: make the API examples compile, render and read as written - #965
Conversation
Pasted into a fresh Vite react-ts app, a run of API page examples failed its type-check. State started as useState(null) or an untyped [] and was then handed to a typed callback, and a few literal arrays were mapped into union-typed props. Each one now types its state or parameters with an annotation, uses `as const` on a literal array, or imports the library type it needs with `import type`, which the live preview drops like any other import line. Autocomplete's string examples narrow the selected item, the object example finds the pick in its own data, and the examples whose selection was never shown lose the unused state. Its item template tag uses isLight instead of the unknown light prop. Field's static Input gains readOnly, as the Input page already has. Collapse's triggers drop the white-ter background that left their text unreadable in dark mode. Rate's wrapper example drops the Control icon that sat on its first star. The Tags dismissible tag now starts visible. The Variations page drops its unused React imports and shows the markup the prefixed example really renders. The MCP index is regenerated from the updated pages.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (60)
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 |
Preview DeploymentPreview URL: https://4d1f1d0f.bestax.pages.dev |
|
deep-review: fresh The previous run was cancelled at the runner limit before it posted anything, so this asks for the same full review again. The change is the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 3 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | API | BulmaClassesProps annotation makes rest statically {}, so the example can no longer show the forwarding the prose above it describes |
docs/docs/api/helpers/usebulmaclasses.md:201 |
| 2 | 🔵 Advisory | Robustness | Both object-data examples recover the typed item by reference equality, which onSelect's contract does not promise |
docs/docs/api/form/autocomplete.md:164 |
| 3 | 🔵 Advisory | Correctness | Dark-mode contrast is fixed by deleting the trigger's background outright, leaving no header affordance in either scheme | docs/docs/api/components/collapse.md:44 |
Overall: Sound, and the evidence holds up end to end — every claim in #943 maps to a real defect and to a fix I could verify against the source. The six interesting ones: Tag's prop really is isLight (elements/Tag.tsx:44); Input isStatic with a value and no handler really does warn, and readOnly is what the Input page already uses; Control iconLeftName really did overlay Rate's first star; Title size={2} really renders <h2 class="title is-2"> (elements/Title.tsx:89-99) and helper classes really are prefixed (useSpacingClasses.tsx:95-155), so the rewritten Variations snippet bestax-title bestax-is-2 / bestax-button bestax-is-primary bestax-mt-3 matches the real output in both name and order; useState() with no argument really did leave show undefined, so the dismissible tag was invisible. Every new annotation checks out against the declared types — the four import type names are all genuinely exported (ToastPosition, TaginputTag, BulmaClassesProps, and AutocompleteItem by name in the prose), the six forwarded-ref element types each match what the component casts to (PolymorphicComponent<…, 'button' | 'a' | 'figure'>, forwardRef<HTMLDivElement | HTMLElement, …>), and the four as const unions are each exactly the prop's declared union. The import type lines are safe in the live previews: transformCode drops any line starting with import (docs/src/theme/CodeBlock/index.js:31-43), and the fence gate mirrors that predicate exactly in asRendered, so the site and the test agree. The riskiest part is advisory #2 — reference-equality lookup in two examples, including the itemTemplate one where a miss blanks every dropdown row. Look at that first, then advisory #1, which is the one a reader actually trips over while adapting the snippet.
Residual risk:
- A fence that parses but no longer type-checks. Refuted as covered:
scripts/eslint-plugin-docs.test.mjsparses everyjsx/tsxfence underdocs/docsandskillswith@typescript-eslint/parser(jsx + latest), so the ~25 addeduseState<…>generics and ~12as constwrappers are held to being parseable TSX, and a fence that parses no way at all fails the suite. What it does not do is resolve types, so the exact class #943 reported can still land; that gap, and the per-fence compile that would close it, is #961. - A live preview that compiles but throws in the browser. Open, and structural: previews are
BrowserOnly+IntersectionObserver-deferred, so the Docusaurus build never evaluates one.react-livewraps the whole fence asreturn (<code>)(generateElement,react-live@5.0.0), which holds only because each changed fence is a single statement after the import strip — all of them are, and the gate above would catch a fence that stopped being one, but a render-time error stays invisible to CI either way. - The same defects surviving elsewhere. Refuted by sweep: no
useState(null)/useState()/useRef(null)/useState([])remains anywhere underdocs/docs/api, noisStaticwith avalueand noreadOnlyremains in the repo, nocolor="info" lightand nowhite-terbackground remain indocs,skillsorbulma-ui/src. The deferred guide and skill pages are enumerated in #961 with the wrong prop names named individually, so the PR body's "left out" paragraph is tracked rather than dropped. - A stale generated index. Refuted by inspection:
bestax-mcp/data/components/*.jsonmoves in lockstep with the pages it is derived from (spot-checked Collapse and Toast, including the embeddedimport typeline), andgen:mcp:checkholds it in CI. - A missing
skills/update. Refuted: none of the four corrected patterns appears underskills/, so there is nothing in this PR for the same-PR skills rule to carry.
🏄 Someone actually paddled out and surfed every example on these pages instead of reading them from the beach, and it shows — wrong prop, phantom warning, a star with an icon parked on it, all cleaned up, and the Variations markup finally matches the wave it claims to be. Three mellow notes to chew on, nothing holding it back. Good to go.
Main's MCP catalog still names 5.26.6 after the 5.27.0 release, so pnpm gen restamps it.
DateRangeInput's page arrived with the merge from main and starts its Controlled example from useState([null, null]), which the template's type-check reads as null[] and refuses to hand to onChange. It now types the state as DateRangeValue, imported with import type, and the weekend predicate types its date, the pattern the other picker pages already follow.
…ue, border Collapse MyColumns typed its props as the helper props alone, so rest had nothing in it and a reader passing an id got a type error on the page that teaches rest. It now takes the div's own props too. The Autocomplete object examples found the picked item by reference, which only holds while the component hands back the array's own objects, so they look it up by value instead. The Collapse examples lost their light header plate for dark mode and with it any sign the header is clickable, so they use the component's bordered prop, which reads in both schemes.
|
deep-review: verify Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on cdbaab8. |
Preview DeploymentPreview URL: https://646dddd2.bestax.pages.dev |
There was a problem hiding this comment.
Deep review (verify) — 3 resolved · 0 open
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | API | Annotation rested away on the page that teaches rest — verified fixed |
docs/docs/api/helpers/usebulmaclasses.md:201 |
| 2 | 🔵 Advisory | Robustness | Object examples rested on an undocumented identity guarantee — verified fixed | docs/docs/api/form/autocomplete.md:164 |
| 3 | 🔵 Advisory | Correctness | Collapse triggers lost every affordance along with the hardcoded plate — verified fixed | docs/docs/api/components/collapse.md:45 |
Overall: This pass settled the threads the earlier review left open and reviewed no commits. All three were 🔵 Advisory and all three are genuinely addressed by cdbaab8e: MyColumns now takes BulmaClassesProps & React.ComponentPropsWithoutRef<'div'>, so rest carries the div attributes the prose promises; both Autocomplete object examples look the picked item up by value instead of by reference, within the declared AutocompleteItem | string | null signature; and every Collapse example uses the component's own bordered prop, whose border resolves through the scheme-derived --bulma-border, so the header reads in light and dark with no hardcoded white-ter. Nothing is left for a human to arbitrate.
Residual risk:
- Docs fences are still unexecuted and untyped — open, and unchanged by this PR.
docs/has notsconfig.jsonand no jest suite, so the three corrected snippets are verified by reading the library's declarations (useBulmaClasses.tsx:77-83,Autocomplete.tsx:106,112), not by a compiler. A later signature change can re-break any of them silently. - Helper-prop / DOM-attribute collisions in the new
MyColumnsannotation — refuted. I enumerated the keys ofBulmaColorProps,BulmaSpacingProps,BulmaTypographyProps,BulmaVisibilityProps,BulmaFlexboxPropsandBulmaOtherPropsagainstReact.HTMLAttributes; none collide (colorlives onAllHTMLAttributes, notHTMLAttributes), so the intersection yields noneverproperty and the example's owntextAlign/textTransformstill compile. - Generated MCP index drifting from the edited pages — refuted.
cdbaab8eregeneratedbestax-mcp/data/components/{Autocomplete,Collapse,useBulmaClasses}.jsonin the same commit, and the recordedcodestrings match the new fences, sogen:mcp:checkhas nothing stale to catch.
🏄 Three mellow advisories rolled in, three paddled back out — the docs now actually do what their own prose says they do. Clean set, no wipeouts, good to go.
github-actions
Bot
added
the
review-converged
allxsmith
added
the
needs-human-review
📸 Story screenshots at handoff —
|
|
🎉 This PR is included in version 5.27.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.2.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.25.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.14.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
A smoke test of the API pages in a fresh Vite react-ts app found examples that fail that template's type-check when pasted, a wrong prop, a React warning, two rendering problems, and a stale snippet on the Variations page.
Examples that started state as
useState(null), an untyped empty array or an untyped date, and then handed the setter to a typed callback, now type that state. The main ones are Autocomplete, the date and time pickers, Taginput, Toast and Dialog, and the ref examples type their refs too. Literal arrays mapped into union-typed props useas const, and untyped helper parameters gain annotations. Where an example needs a library type, the fence carries animport typeline, which the live preview drops like its other import lines.Autocomplete's string examples narrow the selected item, its object example finds the pick in its own data so the fields stay typed, and the examples that never showed a selection lose the unused state. The item template uses
isLight. Field's static Input gainsreadOnly, as the Input page already has. Collapse's triggers drop thewhite-terbackground that left their text unreadable in dark mode. Rate's Field and Control example drops the Control icon that sat on its first star. The Tags example's dismissible tag now starts visible. The Variations page drops its unused React imports and shows the markup the prefixed example really renders. The MCP index is regenerated from the updated pages.Every live fence under the docs was type-checked as its own module under the react-ts template's settings with TypeScript 6, and the API pages now pass, unused-locals checks included. The changed fences were also run through the site's live pipeline and rendered, and the affected pages were checked in the browser for the warnings and the dark-mode contrast.
Left out: some guide and skill pages fail the same check, a few of them with wrong prop names. They are outside this issue and are better handled in one of their own.
pnpm allpasses locally.Fixes #943