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
6 changes: 6 additions & 0 deletions docs/contributing/architecture/request-lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ loader finishes (or is skipped when no loader matches) does the router commit
the URL change and notify subscribers. Route components consume that payload
synchronously on first render via `tryConsumeRouteLoaderData`, so the UI updates
once with data already present instead of swapping routes into a loading state.
Because consumption mutates route closure state mid-render, a successful consume
also schedules one follow-up render of the consuming component (flushed in the
same microtask, before paint); values a route derived before the consume call
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
Expand Down
4 changes: 4 additions & 0 deletions e2e/admin-rbac.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ test('admin RBAC controls access, role assignment, and privacy boundaries', asyn
await expect(page).toHaveURL(/\/admin\/users\/?$/)
await expect(page.getByRole('heading', { name: 'Admin users' })).toBeVisible()
await expect(page.getByText('Unable to load admin users.')).toHaveCount(0)
// The detail panel must render the auto-selected user after SPA
// navigation (regression: preloaded loader data consumed mid-render left
// derivations from pre-navigation state on screen).
await expect(page.getByText('Account metadata only')).toBeVisible()
expect(
await page.evaluate(
() => (window as Window & { __e2eMarker?: boolean }).__e2eMarker,
Expand Down
76 changes: 76 additions & 0 deletions packages/worker/client/loader-data-context.node.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { expect, test } from 'vitest'
import { type Handle } from 'remix/ui'
import {
AppLoaderDataProvider,
hrefMatchesSsrUrl,
normalizeRouterHref,
tryConsumeRouteLoaderData,
} from './loader-data-context.tsx'
import {
clearPreloadedNavigationData,
setPreloadedNavigationData,
} from './navigation-data.ts'

test('loader href helpers normalize URLs and compare pathname and search', () => {
expect(normalizeRouterHref('/community?q=beta')).toBe('/community?q=beta')
Expand Down Expand Up @@ -32,3 +39,72 @@ test('loader href helpers normalize URLs and compare pathname and search', () =>
),
).toBe(false)
})

function createStubHandle() {
const queuedTasks: Array<(signal?: AbortSignal) => unknown> = []
let updateCount = 0
const handle = {
context: {
get(provider: unknown) {
if (provider === AppLoaderDataProvider) {
return { loaderData: undefined, consumedKeys: new Set() }
}
// No RouterLocationProvider in the stub tree; consumption
// falls through to the preloaded navigation slot.
throw new Error('context not available')
},
},
queueTask(task: (signal?: AbortSignal) => unknown) {
queuedTasks.push(task)
},
update() {
updateCount++
return Promise.resolve(new AbortController().signal)
},
} as unknown as Handle
return {
handle,
queuedTasks,
getUpdateCount: () => updateCount,
}
}

test('consuming route loader data schedules a corrective re-render exactly once', () => {
clearPreloadedNavigationData()
setPreloadedNavigationData('/account', {
accountProfile: {
ok: true,
email: 'kody@example.com',
emailVerified: true,
username: 'kody',
displayName: 'Kody',
},
})
const { handle, queuedTasks, getUpdateCount } = createStubHandle()

// Routes apply consumed payloads by mutating closure state mid-render, so
// a follow-up render must be scheduled or derivations made before the
// consume call would persist stale.
const consumed = tryConsumeRouteLoaderData(
handle,
'accountProfile',
'/account',
)
expect(consumed?.email).toBe('kody@example.com')
expect(queuedTasks).toHaveLength(1)
for (const task of queuedTasks.splice(0)) task()
expect(getUpdateCount()).toBe(1)

// Consume-once: the follow-up render finds nothing and schedules nothing,
// so the corrective render cannot loop.
const reconsumed = tryConsumeRouteLoaderData(
handle,
'accountProfile',
'/account',
)
expect(reconsumed).toBeUndefined()
expect(queuedTasks).toHaveLength(0)
expect(getUpdateCount()).toBe(1)

clearPreloadedNavigationData()
})
31 changes: 28 additions & 3 deletions packages/worker/client/loader-data-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,41 @@ export function tryConsumeEmbeddedLoaderData<K extends keyof AppLoaderData>(
return embedded
}

/**
* Consumption happens mid-render and the caller applies the payload by
* mutating route closure state, so anything the route derived earlier in the
* same render pass is stale. Schedule one follow-up render so the route
* re-derives everything from the applied data. The scheduler flushes queued
* tasks and the resulting update in the same microtask as the current render
* (before the browser paints), and consume-once semantics guarantee the
* follow-up pass consumes nothing, so this cannot loop. `queueTask` (not a
* direct `update()`) because a component's update scheduling is not wired
* until its first render returns; on the server `queueTask` is a no-op.
*/
function scheduleCorrectiveRender(handle: Handle) {
handle.queueTask(() => {
void handle.update()
})
}

/**
* Returns loader data for `key` from SSR-embedded props or a preloaded SPA
* navigation slot. Each source is consume-once per key.
* navigation slot. Each source is consume-once per key. Successful reads
* schedule a follow-up render of the consuming component so values derived
* before consumption in the same render pass cannot persist stale.
*/
export function tryConsumeRouteLoaderData<K extends keyof AppLoaderData>(
handle: Handle,
key: K,
currentHref: string,
): AppLoaderData[K] | undefined {
const embedded = tryConsumeEmbeddedLoaderData(handle, key, currentHref)
if (embedded !== undefined) return embedded
return tryConsumePreloadedLoaderData(key, currentHref)
const data =
embedded !== undefined
? embedded
: tryConsumePreloadedLoaderData(key, currentHref)
if (data !== undefined) {
scheduleCorrectiveRender(handle)
}
return data
}
11 changes: 7 additions & 4 deletions packages/worker/client/routes/admin-users.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,9 @@ export function AdminUsersRoute(handle: Handle) {
lastSeenHref = currentHref
lastFailedHref = null
}
const totalPages = Math.max(1, Math.ceil(total / pageSize))
const selectedUser = getSelectedUser()
const isMutating = actionState !== 'idle'

// Consume route-loader data before deriving `totalPages` and
// `selectedUser`; deriving first would render this pass from the
// stale pre-navigation closure state.
const appliedRouteData = applyRouteLoaderData(currentHref)
// A same-path refresh whose loader failed leaves no preload and no
// href change; the stale marker forces the fallback refetch.
Expand All @@ -268,6 +267,10 @@ export function AdminUsersRoute(handle: Handle) {
handle.queueTask(loadAdminUsers)
}

const totalPages = Math.max(1, Math.ceil(total / pageSize))
const selectedUser = getSelectedUser()
const isMutating = actionState !== 'idle'

return (
<AccountManagementShell>
<AdminPageHeader
Expand Down
Loading