Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions docs/contributing/architecture/request-lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,24 @@ therefore cannot persist stale. Routes should still consume before deriving
list/detail state — the follow-up render is a safety net, not the primary
ordering contract.

Route loaders are registered in `clientRouteLoaders` (`routes/index.tsx`) and
matched with the same Remix route-pattern specificity as `clientRoutes`. Loaders
return a `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 can fall back to its own fetch; the router marks the
destination stale (`markNavigationDataStale`) so same-path refreshes — where no
href change would otherwise trigger a refetch — still reload. Hash-only changes
commit immediately without a loader. Back/forward (`popstate`) and same-path
refreshes after form POST also run loaders before notifying, keeping the
previous UI visible until data is ready.
Route loaders are registered in `packages/worker/client/routes/index.tsx` under
`clientRouteLoaders`, keyed by `routePattern(routes.<name>)`. The same keying
scheme is used by `clientRoutes` and `document-head.ts`, so pathname renames
flow from `packages/worker/src/app/routes.ts` instead of duplicated literal
strings. OAuth authorize/callback are the exception: those shells use
`oauthPaths.authorize` and `oauthPaths.callback` because the Cloudflare OAuth
provider wrapper, not `routes.ts`, owns those pathnames. Loaders still match
with the same Remix route-pattern specificity as `clientRoutes`. They return a
`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
Comment on lines +128 to +131

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
`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.

can fall back to its own fetch; the router marks the destination stale
(`markNavigationDataStale`) so same-path refreshes — where no href change would
otherwise trigger a refetch — still reload. Hash-only changes commit immediately
without a loader. Back/forward (`popstate`) and same-path refreshes after form
POST also run loaders before notifying, keeping the previous UI visible until
data is ready.

### Intent prefetch

Expand Down
2 changes: 1 addition & 1 deletion docs/contributing/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ style, tests, MCP kody, and runtime architecture.
[testing](./testing-principles.md)
- [Package discovery routing evaluation](./package-discovery-evaluation.md)
- [Cursor Cloud Agent notes](./cloud-agents.md)
- [Remix skills](./remix.md), [frames](./frames.md)
- [Remix skills and page checklist](./remix.md), [frames](./frames.md)
- [Packages and manifests](./packages-and-manifests.md)
- [Community packages](./community-packages.md)
- [External package invocation API](./package-invocation-api.md)
Expand Down
35 changes: 35 additions & 0 deletions docs/contributing/remix.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,38 @@ template:

If a Node server entry point is added later, evaluate `trustProxy` only at that
trusted reverse-proxy boundary. Do not copy it into Worker request handling.

## Adding a page

For a typical authenticated HTML page, start with the route contract and wire
the narrowest owners. The core route-wiring path is five files:

1. `packages/worker/src/app/routes.ts` — add the route identity. Treat
`routes.<name>` as the canonical source of the pathname.
2. `packages/worker/src/app/router.ts` — map the route to its handler in
`router.map(...)`.
3. `packages/worker/src/app/handlers/<page>.ts` — implement the page handler.
Use the helpers in `#app/page-auth.ts` (`requireAuthenticatedPageUser`,
`requirePageSession`, `requirePageUserWithRole`) for page auth, and use
`#app/request-body.ts` helpers when the page reads structured request bodies.
4. `packages/worker/client/routes/<page>.tsx` — add the route component and,
when the page participates in SPA preload navigation, its route loader.
Import shared loader payload types from `#app/loader-data.ts`
(`kody-custom/prefer-loader-data-types` enforces this). Keep UI-only state
types local.
5. `packages/worker/client/routes/index.tsx` — register the component in
`clientRoutes` and any loader in `clientRouteLoaders`, keyed by
`routePattern(routes.<name>)` instead of a duplicated literal pathname.

That covers the server contract, page handler, client route, and client
registries in five edits. Treat `packages/worker/src/app/document-head.ts` as
the 5-vs-6 check: the count stays at five when the page can reuse existing head
behavior, and it becomes six when the new pathname needs its own title,
canonical URL, or Open Graph metadata. A brand-new standalone route commonly
needs that sixth edit because unmatched pathnames fall back to the `Not found`
title.

OAuth authorize/callback shells are the exception to the `routes.ts` rule. Those
pathnames are owned by the Cloudflare OAuth provider wrapper, so client
registries and document head use `#app/oauth-paths.ts` (`oauthPaths.authorize`
and `oauthPaths.callback`) instead of `routes.ts`.
Loading