Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
29 changes: 29 additions & 0 deletions .changeset/witty-ears-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"react-router": minor
---

Stabilize the `dataStrategy` `match.shouldRevalidateArgs`/`match.shouldCallHandler()` APIs.

- The `match.shouldLoad` API is now marked deprecated in favor of these more powerful alternatives
- If you're using this API in a custom `dataStrategy` today, you can swap to the new API at your convenience:

```tsx
// Before
const matchesToLoad = matches.filter((m) => m.shouldLoad);

// After
const matchesToLoad = matches.filter((m) => m.shouldCallHandler());
```

- `match.shouldRevalidateArgs` is the argument that will be passed to the route `shouldRevaliate` function
- Combined with the parameter accepted by `match.shouldCallHandler`, you can define a custom revalidation behavior for your `dataStrategy`:

```tsx
const matchesToLoad = matches.filter((m) => {
const defaultShouldRevalidate = customRevalidationBehavior(
match.shouldRevalidateArgs,
);
return m.shouldCallHandler(defaultShouldRevalidate);
// The argument here will override the internal `defaultShouldRevalidate` value
});
```
Original file line number Diff line number Diff line change
Expand Up @@ -1200,11 +1200,11 @@ describe("shouldRevalidate", () => {
async dataStrategy({ request, matches }) {
let keyedResults = {};
let matchesToLoad = matches.filter((match) =>
match.unstable_shouldCallHandler(
match.shouldCallHandler(
request.method === "POST"
? undefined
: !match.unstable_shouldRevalidateArgs?.actionStatus ||
match.unstable_shouldRevalidateArgs.actionStatus < 400,
: !match.shouldRevalidateArgs?.actionStatus ||
match.shouldRevalidateArgs.actionStatus < 400,
),
);
await Promise.all(
Expand Down
8 changes: 0 additions & 8 deletions packages/react-router/lib/dom/ssr/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -604,14 +604,6 @@ function getShouldRevalidateFunction(
}
}

// Single fetch revalidates by default, so override the RR default value which
// matches the multi-fetch behavior with `true`
if (ssr && route.shouldRevalidate) {
let fn = route.shouldRevalidate;
return (opts: ShouldRevalidateFunctionArgs) =>
fn({ ...opts, defaultShouldRevalidate: true });
}
Comment on lines -607 to -613
Copy link
Contributor Author

@brophdawg11 brophdawg11 Nov 24, 2025

Choose a reason for hiding this comment

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

This is not needed anymore since this was the old hacky solution when we only had shouldLoad. This behavior should be handled by this code now:

let defaultShouldRevalidate =
!m.unstable_shouldRevalidateArgs ||
m.unstable_shouldRevalidateArgs.actionStatus == null ||
m.unstable_shouldRevalidateArgs.actionStatus < 400;
let shouldCall = m.unstable_shouldCallHandler(defaultShouldRevalidate);

This is actually causing a not-yet-reported bug where if you have a shouldRevalidate in single fetch, the value of defaultShouldRevalidate is incorrect on acton 4xx/5xx responses. Added a new E2E test to this PR to confirm that fix.


return route.shouldRevalidate;
}

Expand Down
20 changes: 9 additions & 11 deletions packages/react-router/lib/dom/ssr/single-fetch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export function getSingleFetchDataStrategyImpl(

let foundRevalidatingServerLoader = matches.some((m) => {
let { hasLoader, hasClientLoader } = getRouteInfo(m);
return m.unstable_shouldCallHandler() && hasLoader && !hasClientLoader;
return m.shouldCallHandler() && hasLoader && !hasClientLoader;
});
if (!ssr && !foundRevalidatingServerLoader) {
// If this is SPA mode, there won't be any loaders below root and we'll
Expand Down Expand Up @@ -282,7 +282,7 @@ async function singleFetchActionStrategy(
fetchAndDecode: FetchAndDecodeFunction,
basename: string | undefined,
) {
let actionMatch = args.matches.find((m) => m.unstable_shouldCallHandler());
let actionMatch = args.matches.find((m) => m.shouldCallHandler());
invariant(actionMatch, "No action match found");
let actionStatus: number | undefined = undefined;
let result = await actionMatch.resolve(async (handler) => {
Expand Down Expand Up @@ -321,9 +321,7 @@ async function nonSsrStrategy(
fetchAndDecode: FetchAndDecodeFunction,
basename: string | undefined,
) {
let matchesToLoad = args.matches.filter((m) =>
m.unstable_shouldCallHandler(),
);
let matchesToLoad = args.matches.filter((m) => m.shouldCallHandler());
let results: Record<string, DataStrategyResult> = {};
await Promise.all(
matchesToLoad.map((m) =>
Expand Down Expand Up @@ -385,15 +383,15 @@ async function singleFetchLoaderNavigationStrategy(
getRouteInfo(m);

let defaultShouldRevalidate =
!m.unstable_shouldRevalidateArgs ||
m.unstable_shouldRevalidateArgs.actionStatus == null ||
m.unstable_shouldRevalidateArgs.actionStatus < 400;
let shouldCall = m.unstable_shouldCallHandler(defaultShouldRevalidate);
!m.shouldRevalidateArgs ||
m.shouldRevalidateArgs.actionStatus == null ||
m.shouldRevalidateArgs.actionStatus < 400;
let shouldCall = m.shouldCallHandler(defaultShouldRevalidate);

if (!shouldCall) {
// If this route opted out, don't include in the .data request
foundOptOutRoute ||=
m.unstable_shouldRevalidateArgs != null && // This is a revalidation,
m.shouldRevalidateArgs != null && // This is a revalidation,
hasLoader && // for a route with a server loader,
hasShouldRevalidate === true; // and a shouldRevalidate function
return;
Expand Down Expand Up @@ -538,7 +536,7 @@ async function singleFetchLoaderFetcherStrategy(
fetchAndDecode: FetchAndDecodeFunction,
basename: string | undefined,
) {
let fetcherMatch = args.matches.find((m) => m.unstable_shouldCallHandler());
let fetcherMatch = args.matches.find((m) => m.shouldCallHandler());
invariant(fetcherMatch, "No fetcher match found");
let routeId = fetcherMatch.route.id;
let result = await fetcherMatch.resolve(async (handler) =>
Expand Down
22 changes: 11 additions & 11 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5695,7 +5695,7 @@ function runClientMiddlewarePipeline(
),
// or the shallowest route that needs to load data
Math.max(
matches.findIndex((m) => m.unstable_shouldCallHandler()),
matches.findIndex((m) => m.shouldCallHandler()),
0,
),
);
Expand Down Expand Up @@ -5874,10 +5874,10 @@ function getDataStrategyMatch(
lazyRoutePropertiesToSkip: string[],
scopedContext: unknown,
shouldLoad: boolean,
unstable_shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null,
shouldRevalidateArgs: DataStrategyMatch["shouldRevalidateArgs"] = null,
): DataStrategyMatch {
// The hope here is to avoid a breaking change to the resolve behavior.
// Opt-ing into the `unstable_shouldCallHandler` API changes some nuanced behavior
// Opt-ing into the `shouldCallHandler` API changes some nuanced behavior
// around when resolve calls through to the handler
let isUsingNewApi = false;

Expand All @@ -5893,20 +5893,20 @@ function getDataStrategyMatch(
...match,
_lazyPromises,
shouldLoad,
unstable_shouldRevalidateArgs,
unstable_shouldCallHandler(defaultShouldRevalidate) {
shouldRevalidateArgs,
shouldCallHandler(defaultShouldRevalidate) {
isUsingNewApi = true;
if (!unstable_shouldRevalidateArgs) {
if (!shouldRevalidateArgs) {
return shouldLoad;
}

if (typeof defaultShouldRevalidate === "boolean") {
return shouldRevalidateLoader(match, {
...unstable_shouldRevalidateArgs,
...shouldRevalidateArgs,
defaultShouldRevalidate,
});
}
return shouldRevalidateLoader(match, unstable_shouldRevalidateArgs);
return shouldRevalidateLoader(match, shouldRevalidateArgs);
},
resolve(handlerOverride) {
let { lazy, loader, middleware } = match.route;
Expand Down Expand Up @@ -5951,7 +5951,7 @@ function getTargetedDataStrategyMatches(
targetMatch: AgnosticDataRouteMatch,
lazyRoutePropertiesToSkip: string[],
scopedContext: unknown,
shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null,
shouldRevalidateArgs: DataStrategyMatch["shouldRevalidateArgs"] = null,
): DataStrategyMatch[] {
return matches.map((match) => {
if (match.route.id !== targetMatch.route.id) {
Expand All @@ -5960,8 +5960,8 @@ function getTargetedDataStrategyMatches(
return {
...match,
shouldLoad: false,
unstable_shouldRevalidateArgs: shouldRevalidateArgs,
unstable_shouldCallHandler: () => false,
shouldRevalidateArgs: shouldRevalidateArgs,
shouldCallHandler: () => false,
_lazyPromises: getDataStrategyMatchLazyPromises(
mapRouteProperties,
manifest,
Expand Down
22 changes: 16 additions & 6 deletions packages/react-router/lib/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ export interface DataStrategyMatch
route: Promise<void> | undefined;
};
/**
* @deprecated Deprecated in favor of `shouldCallHandler`
*
* A boolean value indicating whether this route handler should be called in
* this pass.
*
Expand All @@ -459,12 +461,20 @@ export interface DataStrategyMatch
* custom `shouldRevalidate` implementations)
*/
shouldLoad: boolean;
// This can be null for actions calls and for initial hydration calls
unstable_shouldRevalidateArgs: ShouldRevalidateFunctionArgs | null;
// This function will use a scoped version of `shouldRevalidateArgs` because
// they are read-only but let the user provide an optional override value for
// `defaultShouldRevalidate` if they choose
unstable_shouldCallHandler(defaultShouldRevalidate?: boolean): boolean;
/**
* Arguments passed to the `shouldRevalidate` function for this `loader` execution.
* Will be `null` if this is not a revalidating loader {@link DataStrategyMatch}.
*/
shouldRevalidateArgs: ShouldRevalidateFunctionArgs | null;
/**
* Determine if this route's handler should be called during this `dataStrategy`
* execution. Calling it with no arguments will leverage the default revalidation
* behavior. You can pass your own `defaultShouldRevalidate` value if you wish
* to change the default revalidation behavior with your `dataStrategy`.
*
* @param defaultShouldRevalidate `defaultShouldRevalidate` override value (optional)
*/
shouldCallHandler(defaultShouldRevalidate?: boolean): boolean;
/**
* An async function that will resolve any `route.lazy` implementations and
* execute the route's handler (if necessary), returning a {@link DataStrategyResult}
Expand Down
Loading