Additional parameters for shouldRevalidate to refetch stale data

[ronparkdev]

hi. I'm running a service that needs to cover a large amount of traffic, and I'm going through a technical review to apply react-router (I want to switch from universal-router to react-router).

I'm looking for ways to reduce page load time, and I'm proposing an additional feature. Before I say something, I'm not sure if this is in line with the direction I want to go, so I'll start with some context.

The idea is that when a page is navigated to a CSR, the goal is to reuse data already received from the loader if it's not out of date.

To refetch a loader holding stale data,
I expected shouldRevalidate, which is provided by the loader, to provide this functionality, but it doesn't.

So I came up with a few ideas for what data we could provide to shouldRevalidate.

• The ability to read the last result of the loader from shouldRevalidate.
  • If we could provide the time in loader or get it from server data, shouldRevalidate could determine if we should call loader for this route again.

      <Route
      path="meals-plans"
      element={<MealPlans />}
      loader={async () => {
        const result = axios.get('/meals-plans')
        return {
          result,
          expireTime: new Date().getTime() + 10 * 60 * 1000, // 10m cache
        }
      }}
      shouldRevalidate={({ prevLoaderResult }) => {
        return prevLoaderResult.expireTime > new Date().getTime()
      }}
    >

• Alternatively, we could just pass the time of the last loader call to shouldRevalidate.
  • If I were to modify this library, this would be simpler.

      <Route
      path="meals-plans"
      element={<MealPlans />}
      loader={loadMealPlans}
      shouldRevalidate={({ lastLoaderResponsedTime }) => {
        return lastLoaderResponsedTime > new Date().getTime() + 10 * 60 * 1000, // 10m cache
      }}
    >

Providing additional data to shouldRevalidate would be a major change in the current flow of library development, but I think it's a necessary feature for production use in high traffic environments.

Not only for the backend API traffic that comes from moving every page to the CSR, but also to improve usability for site visitors by reusing slightly heavier APIs that are already called.

If you have any suggestions, even if they're small, I'd love to hear them.
Thank you :)

[brophdawg11]

Instead of adding "cache-like" ability to shouldRevalidate, why not just implement a cache in your loader if it's expensive? If you think about this as if the requests were HTTP requests (as they are in Remix), the idiomatic way would be to add Cache-Control headers to your loader HTTP responses.

async function loader(request) {
  if (cache.has(request.url)) {
    return cache.get(request.url);
  }

  let data = await getExpensiveData();
  return data;
}

[ronparkdev]

I hadn't thought of that. As you say, it can be solved by using a cache in the loader.

This would require an additional setting like shouldRevalidate: () => true to make the loader run every time.

I'll try to implement it this way, thanks for the suggestion!

[AndrewWebDev]

Hi, I also have a case where this possibility would be very suitable

Always has pros and cons but what I see is that function shouldRevalidate allows you to opt-out of revalidation for a route's loader as an optimization and the loader will not be called and the existing data for that loader will persist on the page.

For me, this is the cache already build-in because the function controls the caller of the loader and returns a persistent state.

I suppose the idea was about a single source of truth for controlling the call of the loader.

I believe your suggestion is violating this behavior because we have the shouldRevalidate function and plus extra an additional cache inside the loader.

Would be very handy if we can grab requestTime from

{ currentParams: AgnosticDataRouteMatch["params"]; nextParams: AgnosticDataRouteMatch["params"]; }

and compare times if needed: (nextParams.requestTime - currentParams.requestTime) > 10 * 60 * 1000.

In this way, we can realize Incremental Client Regeneration for any our API calls and control from one source of truth and it increases usage.

Of course, we can:

• add Cache-Control for our API calls
  (should write on function for setting cache depends on params on Back-End),
• set a service worker for caching fetch requests
  (add extra proxy control between network and browser for caching depends on params for particular route,
  also should split our cache logic again on Front-End)
  ,
• create custom searchParams.get('requestTime')
  (users can see it and change it),

We should do quite enough extra work despite the fact that we already have a single source function for revalidating.

I suppose the core idea was to make one manifest for controlling each route.

What I am curious about is that in the current case, we have already a built-in function for controlling the loader caller where we can revalidate our data by request params, by any params how we want, but not request time.

My opinion is just we should think about it,
as controlling revalidation over time gives us more control and possibilities for optimization

[damaafer]

Hi, I also think that loaderData should be added to shouldRevalidate args.

Imagine a loader returning data that can be either static or dynamic depending on params for example

import { ShouldRevalidateFunctionArgs } from 'react-router';
import { Route } from './+types/conditionallyRevalidatedRoute';

export async function loader({ params: { id } }: Route.LoaderArgs) {
  const n = Number(id);
  return n % 2 === 0
    ? {
        shouldRevalidate: true,
        value: Math.floor(Math.random() * 100) + n
      }
    : { shouldRevalidate: false, value: 42 + n };
}

export default function ConditionallyRevalidatedRoute({
  loaderData
}: Route.ComponentProps) {
  return <div>{JSON.stringify(loaderData)}</div>;
}

It would be very suitable to define the shouldRevalidate function as

export function shouldRevalidate({
  currentParams,
  nextParams,
  loaderData,
  defaultShouldRevalidate
}: Route.ShouldRevalidateArgs) {
  if (currentParams.id !== nextParams.id) {
    return true;
  }

  return loaderData.shouldRevalidate && defaultShouldRevalidate;
}

The proposed workaround (using a clientLoader and a cache) is not really suitable since RR7 in framework mode and singleFetched server loader functions.

[AndrewWebDev]

+1

For me could be very useful revalidating cache by some condition, for example by last requested time

At the current moment we have loader which in v7 returning raw objects.

So, we have two options for returning values (value and promise):

1. Return value for useLoaderData() - for example some Object
2. Return promise and use useAsyncValue() hook in <YourComponent /> inside <Await resolve={useLoaderData()}><YourComponent /></Await>

Let's imagine we returns promise from loader and in shouldRevalidate -> loaderData we would have a promise,
luckly the same promise have the same reference, so we can resolve it anytime and get the same values, but after resolving we get the same values only from this promise (for example fetch.json())

What if we want also check in shouldRevalidate some data which already available in loader?
How can we pass them on?

We can populate loaderData with our custom data and then use all of it for revalidating cache,
but it depends how - create custom value with params for shouldRevalidate and fetchPromise or create promise which returns our custom value (downside is that we should wait resolving inner promise, so get delay processing)

As a result we create our own response from loader and process it accordingly in Component level (useAsyncValue) and differently in shouldRevalidate

I believe we should think about return structure for loader for eliminating mixing values, for example we could return:

value or promise or {
 shouldRevalidateParams: ..., // params for shouldRevalidate
 value: ... // could be value or promise
}

[damaafer]

Maybe shouldRevalidate should be an async function ?
I don't know the implication of such a change for RR internals.