Skip to content

Make route-loader consumption self-correcting - #666

Merged
kody-bot merged 2 commits into
mainfrom
cursor/self-correcting-route-loader-consumption-db97
Jul 8, 2026
Merged

kody-bot merged 2 commits into
mainfrom
cursor/self-correcting-route-loader-consumption-db97

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jul 8, 2026 •

Copy link
Copy Markdown
Member

Problem

tryConsumeRouteLoaderData runs mid-render and routes apply the consumed payload by mutating closure state. Any value a route derived before the consume call in the same render pass renders stale — and because the preload was successfully applied, no fallback refetch is queued, so the stale UI persists until the next unrelated update or a full reload. This class caused the empty secrets list (#664) and, still live on main, a stale /admin/users detail panel: after SPA navigation the sidebar auto-highlights the first user but the panel shows "Choose an account from the list..." (and pagination can be hidden).

Fix

Eliminate the class at the consumption layer instead of relying on per-route statement ordering:

  • tryConsumeRouteLoaderData now schedules one corrective re-render of the consuming component whenever it returns data (handle.queueTask(() => handle.update())). The remix/ui scheduler flushes queued tasks and the resulting update inside the same microtask-driven flush loop as the current render — before the browser paints — so even a mis-ordered route can no longer show stale derivations. Consume-once semantics guarantee the follow-up pass consumes nothing, so it cannot loop (and the scheduler's cascading-update guard backstops that). On the server queueTask is a no-op, so SSR is unaffected.
  • Reordered /admin/users to consume before deriving totalPages / selectedUser (the last mis-ordered route), keeping "consume first" the primary contract with the framework behavior as safety net.
  • Strengthened the admin RBAC e2e journey: after the SPA navigation to /admin/users it now asserts the auto-selected detail panel actually rendered ("Account metadata only"), which fails without this fix.
  • Added a unit test pinning the contract: consuming schedules exactly one corrective render; re-consuming schedules nothing.
  • Documented the safety net in docs/contributing/architecture/request-lifecycle.md.

Evidence

The safety net was verified in isolation: with only the loader-data-context.tsx change applied (admin-users still mis-ordered), the /admin/users detail panel rendered correctly after SPA navigation, with no visible flash.

Before (current main): stale placeholder panel after clicking Admin in the top nav; correct after F5:

Before: stale admin detail panel after SPA navigation

After (framework fix only, route untouched):

After: populated admin detail panel after SPA navigation

Final demo — SPA navigation to admin users (twice) and secrets, all populated immediately:

spa_navigation_admin_and_secrets_after_router_fix.mp4

Testing

  • ✅ npm run validate (format, lint, typecheck, 631 unit tests, 10 Playwright e2e incl. the strengthened admin assertion, MCP e2e)
  • Manual: reproduced the stale /admin/users panel on main, verified the framework fix corrects it with the route still mis-ordered, then applied the route reorder and re-verified both admin and secrets pages across repeated client-side navigations.
System recap — extends a primitive (medium risk)

Mode: recap · Base: main @ 27b28a4c · Head: c9e0bff6

Classification: extends — changes the client route-loader consumption contract in app-ui (successful consumes now schedule a corrective re-render). No primitives added.

Primitives touched

Primitive Group Impact
app-ui surfaces extends — tryConsumeRouteLoaderData self-corrects; /admin/users render-order cleanup

System map

flowchart LR
	appUi["app-ui"]:::extended
	appSessions["app-sessions"]:::untouched
	rbac["rbac"]:::untouched
	appUi --> appSessions
	appUi --> rbac
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff
Loading

Change flow

sequenceDiagram
	participant Router as client-router
	participant Route as route component
	participant Sched as remix/ui scheduler
	Router->>Route: commit navigation (preload stored)
	Route->>Route: render pass consumes preload (closure mutated)
	Route->>Sched: queueTask(update) on successful consume
	Sched->>Route: corrective render (same flush, before paint)
	Note over Route: derivations re-run from applied data — stale values cannot persist
Loading
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Bug Fixes

    • Improved admin user navigation so the detail view shows the correct selected user and metadata after route changes.
    • Fixed stale render state when consuming preloaded route data, reducing cases where the UI could briefly show outdated values.
  • Documentation

    • Clarified routing guidance for data consumption and render ordering in the contribution docs.

tryConsumeRouteLoaderData mutates route closure state mid-render, so any
values a route derived earlier in the same render pass were stale with no
follow-up render queued -- the bug class behind the empty secrets list
(#664) and the stale /admin/users detail panel. A successful consume now
schedules one corrective re-render of the consuming component; the
scheduler flushes it in the same microtask (before paint) and
consume-once semantics guarantee it cannot loop.

Also reorder /admin/users derivations after the apply call (last
mis-ordered route), assert the auto-selected detail panel after SPA
navigation in the admin RBAC e2e journey, and document the safety net in
the request lifecycle notes.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aaf387d7-4ae3-44dc-b6c0-0b4d4bf176e5

📥 Commits

Reviewing files that changed from the base of the PR and between 27b28a4 and c9e0bff.

📒 Files selected for processing (5)
  • docs/contributing/architecture/request-lifecycle.md
  • e2e/admin-rbac.spec.ts
  • packages/worker/client/loader-data-context.node.test.ts
  • packages/worker/client/loader-data-context.tsx
  • packages/worker/client/routes/admin-users.tsx

📝 Walkthrough

Walkthrough

The PR adds a corrective re-render mechanism to tryConsumeRouteLoaderData, which now schedules a follow-up render via handle.queueTask after successfully consuming loader data. AdminUsersRoute is reordered to consume route data before deriving render state. Tests and documentation are updated accordingly.

Changes

Corrective render for route-loader consumption

Layer / File(s) Summary
scheduleCorrectiveRender and consumption flow
packages/worker/client/loader-data-context.tsx
Adds scheduleCorrectiveRender(handle) helper and updates tryConsumeRouteLoaderData to queue a follow-up update() via handle.queueTask when consumed data is non-undefined.
AdminUsersRoute consumption ordering
packages/worker/client/routes/admin-users.tsx
Moves applyRouteLoaderData(currentHref) before computation of totalPages, selectedUser, and isMutating, with clarifying inline comments.
Tests and docs for corrective render
packages/worker/client/loader-data-context.node.test.ts, e2e/admin-rbac.spec.ts, docs/contributing/architecture/request-lifecycle.md
Adds a stub Handle and test verifying single queued corrective render on consume, an e2e assertion for admin user detail metadata, and documentation of the microtask-scoped follow-up render guarantee.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • kentcdodds/kody#601: Builds directly on the same tryConsumeRouteLoaderData/context logic in loader-data-context.tsx.
  • kentcdodds/kody#608: Extends the same route-loader/preloaded-navigation consume flow with the new corrective render queuing.
  • kentcdodds/kody#642: Touches the same AdminUsersRoute render-data flow around selectedUser derivation.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: route-loader consumption now triggers a corrective re-render to avoid stale derived state.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/self-correcting-route-loader-consumption-db97

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kentcdodds
kentcdodds marked this pull request as ready for review July 8, 2026 05:48
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-666.kody-a99.workers.dev

Worker: kody-pr-666
D1: kody-pr-666-db
KV: kody-pr-666-oauth-kv

Mocks:

@kody-bot
kody-bot merged commit c281ac4 into main Jul 8, 2026
8 checks passed
@kody-bot
kody-bot deleted the cursor/self-correcting-route-loader-consumption-db97 branch July 8, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants