Repository navigation
docs: routing integration guide; skill routing note (#190 docs half) - #308
Conversation
New Routing Integration feature guide covering the three first-hour questions from #190, source-verified against current typings: - Menu.Item as={Link} to=... is fully typed today (index signature + explicit to forwarding). - Navbar.Item as={Link} forwards to at runtime but to isn't in its prop type — the guide documents the one-line typed alias; first-class typing is now tracked in #306. - Button's as is polymorphic since #238, so ButtonLink (typed alias) or useNavigate are the two navigation patterns; active-route styling via useLocation + the active prop; Next.js needs no alias (href is typed). The layout skill's Navbar section gains a routing note (stop copying href="#" in routed apps) pointing at the guide. Closes #190 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdded routing documentation for Bestax components covering React Router and Next.js links, button navigation, active-route styling, TypeScript aliases, and related API references. Updated the Navbar skill reference with concise routing guidance. ChangesRouting documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Preview DeploymentPreview URL: https://d1153e2d.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docs/guides/features/routing.md`:
- Around line 19-20: The routing documentation overstates Menu.Item’s static
typing guarantees. In docs/docs/guides/features/routing.md lines 19-20 and
skills/bestax-layout-scaffold/references/layout-components.md lines 227-230,
replace “fully type-checked” with wording that accurately describes router-prop
forwarding and the available typing without claiming router-specific validation.
- Around line 118-122: Update the Menu.Item active condition in the routing
example to match the /customers route only when the pathname is exactly
/customers or begins with /customers/. Preserve active highlighting for customer
subroutes while excluding similarly prefixed routes such as /customerships.
- Around line 137-149: Correct the Next.js version guidance in the routing
documentation: state that legacyBehavior/passHref apply to Next.js 13–15, that
Next.js 16 removes them, and that pre-13 requires the anchor-child pattern by
default. Replace the current compatibility example with an explicit Link wrapper
using Navbar.Item as="a" for older versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2abf4560-9b53-4a9b-8337-635f0694025e
📒 Files selected for processing (2)
docs/docs/guides/features/routing.mdskills/bestax-layout-scaffold/references/layout-components.md
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| `Menu.Item` supports routers out of the box — pass the router's link component via `as`, and | ||
| `to` (or any other router prop) is forwarded to it, fully type-checked: |
There was a problem hiding this comment.
"fully type-checked" overstates Menu.Item's typing — 🟡 Minor · API
What: Menu.Item accepts to only because MenuItemProps has an index signature ([key: string]: unknown, Menu.tsx:153). That makes to (and any other extra prop) type unknown — accepted, but not validated. <Menu.Item as={Link} to={42}> or a typo'd too="/x" both compile with no error.
Why it matters: The page's whole thesis is TypeScript typing precision (it carefully explains why Navbar.Item needs a cast). Calling Menu's forwarding "fully type-checked" is the opposite of what the index signature does — a reader trusting the compiler here gets no safety net on router props.
Fix: soften the claim to what's actually true (it compiles / is forwarded), without promising type-checking:
| `Menu.Item` supports routers out of the box — pass the router's link component via `as`, and | |
| `to` (or any other router prop) is forwarded to it, fully type-checked: | |
| `Menu.Item` supports routers out of the box — pass the router's link component via `as`, and | |
| `to` (or any other router prop) is forwarded to it, no cast required: |
Source
Menu.tsx:
export interface MenuItemProps ... {
as?: React.ElementType;
[key: string]: unknown; // <- accepts `to`, but as `unknown`
}By contrast NavbarItemProps extends only AnchorHTMLAttributes with no index signature, which is exactly why to errors there — the section below documents that correctly.
- Menu.Item wording no longer claims full type-checking: the index signature accepts extra keys as unknown — compiles without casts, but no router-prop validation (both docs and skill note reworded). - Active-route example uses a boundary match (exact or /customers/ prefix) so /customerships can't ring the bell. - Next.js version ranges corrected: 13-15 also accepted the deprecated legacyBehavior/passHref, 16 removes them, and pre-13 needed the anchor child — now shown with the explicit wrapper pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up
|
All four review findings (CodeRabbit ×3 + the deep review's, which duplicates CodeRabbit's first) verified and fixed in 9fc7eae:
Generated by Claude Code |
There was a problem hiding this comment.
Deep review — 1 finding
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | API | "fully type-checked" overstates Menu.Item typing — the index signature accepts to as unknown, so router props aren't actually validated |
docs/docs/guides/features/routing.md:19-20 |
Overall: This is a docs-only PR and the technical content is sound — I source-verified every claim against the current typings. Menu.Item's index signature + explicit to/href forwarding makes as={Link} to=… compile and work; Navbar.Item genuinely lacks the extra prop in its type (no index signature over AnchorHTMLAttributes), so the documented cast alias is the correct workaround; Button's as is polymorphic and forwards to via anchorRest when rendered as a non-button. All three cross-links resolve, the skill URL is right, and sidebar_position: 6 doesn't collide. The one thing to fix is a wording overstatement: Menu's forwarding isn't "type-checked" — the index signature makes those props unknown, so there's no compiler safety on to. Human should focus on that single phrasing tweak; everything else is good to merge.
🏄 Clean little docs set, dude — the routing waves are mapped out right and every link lands. Just one gnarly word, "type-checked," that's telling surfers the water's safer than it really is. Patch that phrase and this one's cruising straight to shore.
Preview DeploymentPreview URL: https://d5491dbb.bestax.pages.dev |
|
🎉 This PR is included in version 5.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.3.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Pull Request
Description
The docs half of #190: a new Routing Integration feature guide (
docs/guides/features/routing) answering the three first-hour questions, plus a routing note in the layout skill so agents stop copyinghref="#"into routed apps.@allxsmith/bestax-bulma)create-bestax) — ships the updated layout skill@allxsmith/bestax-docs) — new feature guideskills/bestax-layout-scaffoldrouting noteEverything is source-verified against current typings — and the issue's landscape has shifted since it was filed:
Menu.Item as={Link} to="…"is fully typed today (MenuItemPropshas an index signature and explicitly forwardsto) — the guide leads with it as the zero-friction case.Button as={Link}type-checks now — the issue'sTS2322was fixed by feat(bulma-ui): make Button and Link as prop polymorphic (React.ElementType) #238 (polymorphicas). The guide documents theButtonLinktyped alias forto, theuseNavigatealternative, and when to prefer each (real anchors vs. imperative side-effects).Navbar.Item's gap is real and remains:toisn't in its prop type (runtime forwards it fine). The guide documents the recommended one-line typed alias; the API fix is now spec'd in [Feature] Navbar.Item: first-class router-link typing (accepttolike Menu.Item) #306 (filed with this PR — index-signature parity withMenu.Itemvs. proper generic polymorphic typing).useLocation().pathnamedriving theactiveprop (with a note on whyNavLink's function-style props don't mix with the components' class handling).<Link>useshref, which is already typed —as={NextLink} href="…"needs no alias; pre-13legacyBehaviornoted.Skill:
layout-components.md's Navbar section gains a compact routing block (Menu.Item typed pattern, Navbar.Item alias,activefromuseLocation, link to the guide).Related Issue(s)
Closes #190
Refs #238 (Button polymorphism, already shipped), #306 (new — Navbar.Item first-class
totyping)Type of Change
Checklist
CLAUDE.mdfiles are updated (none affected)Additional Context
Code samples in the guide are plain
tsxblocks (nottsx live) since they importreact-router-dom, which isn't in the docs live scope.🤖 Generated with Claude Code
https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up
Generated by Claude Code
Summary by CodeRabbit
Linkpatterns (includingasusage for menu items and buttons).Navbar.Item.Linkbehavior comparison.Navbarreference with a routing note and links to the full guide.