Repository navigation
Document the five-file add-a-page checklist - #878
Conversation
Update Remix and request-lifecycle contributing docs for routePattern registries, page-auth helpers, and loader-data type imports so adding a typical authenticated page is a five-file checklist. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughContributing documentation now explains the canonical process for adding Remix pages, links to that checklist, and details SPA route-loader registration and preload-then-commit navigation behavior. ChangesContributing documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 deployed: https://kody-pr-878.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/contributing/architecture/request-lifecycle.md`:
- Around line 128-131: Revise the request-lifecycle documentation around
RouteLoaderRedirect and routeLoaderRedirect to state explicitly that loaders
must remain side-effect-free because they may execute speculatively during
intent prefetch before navigation commits. Keep the router’s ownership of
redirect execution clear without implying that this mechanism makes loaders
side-effect-free.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3603fd6a-af04-4391-9ae4-dc23dadf4233
📒 Files selected for processing (3)
docs/contributing/architecture/request-lifecycle.mddocs/contributing/index.mddocs/contributing/remix.md
| `RouteLoaderRedirect` (via `routeLoaderRedirect`) to abort the SPA navigation | ||
| with a full-document redirect (for example, `401` → login). The router performs | ||
| the redirect, never the loader itself, so speculative loader runs stay side- | ||
| effect free. Loader errors still commit the navigation so the destination route |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not imply that the router makes loaders side-effect-free.
The router owns redirect execution, but loaders can still run speculatively during intent prefetch. State the requirement explicitly: loaders must remain side-effect-free because they may execute before navigation commits.
Suggested wording
- The router performs the redirect, never the loader itself, so speculative loader runs stay side-effect free.
+ The router performs the redirect, never the loader itself. Loaders must remain side-effect free because they may run speculatively.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `RouteLoaderRedirect` (via `routeLoaderRedirect`) to abort the SPA navigation | |
| with a full-document redirect (for example, `401` → login). The router performs | |
| the redirect, never the loader itself, so speculative loader runs stay side- | |
| effect free. Loader errors still commit the navigation so the destination route | |
| `RouteLoaderRedirect` (via `routeLoaderRedirect`) to abort the SPA navigation | |
| with a full-document redirect (for example, `401` → login). The router performs | |
| the redirect, never the loader itself. Loaders must remain side-effect free | |
| because they may run speculatively. Loader errors still commit the navigation so | |
| the destination route |
🤖 Prompt for 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.
In `@docs/contributing/architecture/request-lifecycle.md` around lines 128 - 131,
Revise the request-lifecycle documentation around RouteLoaderRedirect and
routeLoaderRedirect to state explicitly that loaders must remain
side-effect-free because they may execute speculatively during intent prefetch
before navigation commits. Keep the router’s ownership of redirect execution
clear without implying that this mechanism makes loaders side-effect-free.
Summary
Track A (web routing consolidation), PR 4 of 4: contributing docs for the reduced add-a-page surface.
docs/contributing/remix.md(5 core files; document-head as the optional 6th).request-lifecycle.mdso client registries/document-headare described as keyed byroutePattern(routes.*)(withoauthPathsfor authorize/callback).Risk: LOW — docs only.
Test plan
npm run validateSystem recap — composes existing primitives (low risk)
Mode: recap · Base:
main@f2f137e7· Head:24fc5eb3Classification: composes — documentation only; no runtime code changes.
Primitives touched
None matched by code roots (docs-only).
System map
Contributing docs now describe the post–Track A page-wiring path:
routes.ts→ handler with page-auth → client route +routePatternregistries.Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Summary by CodeRabbit