fix: cache initial data

[aj-may]

It appears some time ago the initialData configuration was used as a fallback. When #211 was merged, this behavior changed to be used with SSR like in the next.js example in the README. Issue #230 explains this was the expectation. I'm using SSR, so im fine with the new behavior.

Since initialData is now not quickly revalidated, another issue (#308) has been raised. Since initialData is not cached, and the mutate w/ callback grabs the curerent data from the cache, when initialData is used, undefined is returned.

fixes #308

[aj-may]

@Timer @shuding @pacocoursey Anything else I need to do to get this reviewed? I wasn't able to find any contributing docs, so I'm not sure how your process works.

Let me know if there's anything I can do to move this along. Thanks!

[pacocoursey]

Thanks for this fix! Seems reasonable that initialData should populate the cache immediately.

[sergiodxa]

What happens when the hook is updated and you still have initialData in the config?

[sergiodxa]

Maybe this should do something like:

let initialData = cache.get(key) ?? config.initialData
if (!cache.has(key)) cache.set(key, config.initialData, false)

If the key is already in cache ready it, if not use the config.initialData and update the cache.

[aj-may]

Sorry for the delay, took me some time to figure out how to test this in my own project. Seems to be working fine as is without these proposed changes.

Can you clarify what you mean by "when the hook is updated". I don't fully understand the comment.

[aj-may]

@sergiodxa I think I understand your point now.

If I initialize the hook with key foo in one part of my application with initial data ['1', '2'], but in another part of the application init the hook, again with key foo, but with initial data ['3', '4'] what is the desired behavior?

@sergiodxa @pacocoursey should the first or last hook take precedence?

[sergiodxa]

Yeh if there is initialData defined you are first overwriting the cache and then reading from it. I think if already have some cached value the expected result is to get that value and then wait for a revalidation, so the second usage of the same key should receive the cached value even if that cached value is the initial value of another call of SWR.

[pacocoursey]

I agree, which means the first hook should take precedence.

Hook called with initial data: a → a cached
Hook called somewhere else with different initial data → return a

This also means that:

Hook called without initial data but returns data: b → b cached
Hook called somewhere else with initial data → return b (ignore initialdata)

[aj-may]

@sergiodxa @pacocoursey Updated based on the feedback. ☺️

[mrmntte]

When usSWR is used with SSR, a unique global instance is created to store the cache.
How could I prevent the initialData data from being stored in the server cache?

[sergiodxa]

Ohh right, this will start filling the cache server side too.

You can use cache.clear() after or before the render to clear the cache. Maybe this change should also check if it’s running server side and don’t save in cache the initial data.

[aj-may]

The current version of SWR will also fill the serve-side cache if not passed initialData.

What's the concern with using the cache server-side? memory consumption?

If we don’t want to use the cache server-side, then what would the desired behavior be for useSWR server-side?

I don't have a good reason why, but I feel like adding a typeof window === 'undefined' in the library is not a great option.

[sergiodxa]

I'm pretty sure right now SWR is never filling the cache server-side, because the fetch always happen client-side (effects only run client-side), even if you are using Suspense it will not because current (not experimental) version of React doesn't support Suspense for SSR so you need to avoid rendering the component doing the fetch at all.

The main concern with using the cache server-side is leaking data, imagine user A access /dashboard in Vercel, the data for that user will be stored in a cache, then user B access the same page, because the required data is already in cache we will SSR the dashboard using user A data instead of fetching user B data, then client-side we will revalidate the data and correctly fetch user B data updating the UI, while the happens the second user could access the first user dashboard and see its deployments, projects, etc.

This could be somehow avoided in a Serverless paradigm because each request should have its own Node.js instance with its own memory, nevertheless even in Serverless you usually want to keep a function warm if you are receiving multiple requests to avoid the cold start time, so you could still leak data between users.

[oran1248]

@aj-may So when the fix will be released ?

[aj-may]

@sergiodxa Ahh, your right! I just tested it again and master doesn't fill the cache server side. I push an update that will check the IS_SERVER value before caching.

@oran1248 I'm not a maintainer so I can't answer that question, but I just pushed an update to address comments.

[aj-may]

@Timer @shuding @pacocoursey can I get a re-review on this?

[sergiodxa]

This should fallback to config.initialData as before for the case where SWR is running server-side

[shuding]

TBH I don't think it's a good idea to directly write to cache, in the data read phrase. Let me try to explain my concerns.

Correctness

Usually cache read is safe because it doesn't change the data. However mixing cache read with write might change that behavior and cause unexpect issues, for example this causes a side effect:

useSWR('data', { initialData: 1 }) // → 1
useSWR('data')                     // → 1

// changing the order will cause a different result
useSWR('data')                     // → undefined
useSWR('data', { initialData: 1 }) // → 1

If you have a very complex component tree, with Suspense or Concurrent mode enabled in the future, it will be impossible to track these side effects.

In fact, the data flow of SWR is a loop, which makes things easier to maintain:

                 +--> read cache ----+
                 |                   |
  render         |                   v
+--------------->+                 fetch
  mutate         |                   |
                 |                   |
                 +--- write cache <--+

As you can see, cache read happens on render, which is the same as React itself: render is pure, and doesn't have any side effect (thus it's safe to render as many times as you want, in any order). Then fetch and cache write are wrapped with useEffect, which can cause side effects and can be scheduled.

Generally SWR obeys the rules of React, that ensures its correctness.

Performance

We plan to support different cache backend in the future. So it can be a Map (memory), localStorage (disk, sync), IndexedDB (disk, async) or even a remote storage (network). That means cache write can somehow be very expensive, comparing to read.

API Design

Regarding this issue (#230), this is one of my regrets about SWR's current API. initialData is per hook, and it's treated as a "fallback", rather than a "cache preset" (which is what this PR trying to do):

const { data } = useSWR('data', { initialData: 1 })

// the mental model is more like
const data = useSWR('data').data || 1

There're many issues with that API design. It's a wrong direction and we shouldn't have gone with it. Instead, the correct way of doing "presets" is context (SWRConfig). Imagine this API:

<SWRConfig value={{ initialData: { data: 1 } }}>
  <Component/>
</SWRConfig>

The context provides a default cache object, that all its children can read from or override to. The beauty is it doesn't add any extra logic to the SWR hook.

With the new cache interface (thanks @sergiodxa), I think we should even simplify it to:

<SWRConfig value={{ cache: initialCache }}>
  <Component/>
</SWRConfig>

So we can have namespaced cache and initial data at the same time, and it will be extremely friendly for SSR/SSG as well.

above^

[nandorojo]

@shuding That makes sense to me. However, I'm not sure how it will fit into Next's SSG patterns. Since static generation is per-page, but the SWRConfig goes in the root of the app, how would statically-generated data get passed to SWRConfig?

I want to share a single page's static data across my Next site, similar to Gatsby's useStaticQuery.

Should I call cache.set('page-data', staticData) on the page's mount in order to share the static data globally, and forget initialData altogether?

[Develliot]

I have the same issue,

I need still need cache when using initialData with SSR

I have fundraising page and which uses initialData and I have an other end point for adding donations. when I add a donation I want to update the running total on the fundraising page. I can't just revalidate the page because I have no guarantee that the page endpoint has updated in time I have to mutate it locally.

from the donation component I want to do something like this:

const pageCache = cache.get(pageUrl);
        if (pageCache) {
          const difference = amountState - (originalAmount || 0);
          const newPageData = {
            ...pageCache,
            donationsTotalAmount: pageCache.donationsTotalAmount + difference,
          };
          mutate(pageUrl, newPageData, false);
        }

I can't mutate the page by key when it doesn't exist in cache because I need to know the old page data before sending it modified page data. All the donation endpoint knows is the donation, it has no data about the page. I'm forced to do use useEffect like this on the fundraising page to make sure the cache is set when using initialData:

const FundraisingPage = ({ pageData }: Props) => {
  //  figure out unique page url for page cacheId
  const router = useRouter();
  const { slug } = router.query;
  const slugString = queryAsString(slug);
  const url = `${createFundraisingPageUrl({ slug: slugString })}`;

  // page cache
  const opts = { initialData: pageData };
  const { data, error, mutate } = useSwrGet(url?.length ? url : null, opts);
  // manually setting cache as this doesn't seem to happen with swr for ssr only on the browser
  useEffect(() => {
    if (data) {
      cache.set(url || "", data);
    }
  }, []); 

.....

[nandorojo]

I might be reading this incorrectly, but 2 thoughts:

1. If you're using query parameters in router.query, rather than dynamic routes, they won't exist on the server. This is why it's not working for SSR. You should be using get server side props from Next.js if you want the initial server load to always be up to date (I think, I've only used getStaticProps.) Also, use dynamic routes instead of query parameters.

2. Your server should return to you only after the money has posted to your database. I personally wouldn't optimistically render for something as sensitive as dollar amounts, but that's your call. In any case, revalidating should be safe by the time you've POSTed your data.

A pattern I like: POST data to the server, it returns the mutated data, and then inject that into SWR's cache.

[mthines]

@aj-may

I ended up checking if an identical URL and request body already has been sent, then settings revalidateOnMount to false.

Then it fetches only onces, and then only use the cache if it exists.

  const response = useSWR<Data, Error>(
    apiUrl && token ? [apiUrl, 'GET', reqData] : null,
    {
      // If we already have cached results, we don't want to call the endpoint again.
      // If we don't have cached results, then call the end point
      revalidateOnMount: !cache.keys().some((k) => apiUrl && k.includes(apiUrl) && reqData && k.includes(reqData)),
      revalidateOnFocus: false,
      revalidateOnReconnect: false,
      refreshWhenOffline: false,
      refreshWhenHidden: false,
    },
  );

Not sure if it fixes your request, but it worked for me.

Consider looking at swr-sync-storage if you want to persist the data on page refresh :)

[shuding]

#1017 is going to cover this very same use case in a more general way, which we are aiming to launch soon. 🎉

[shuding]

Closing this PR since #1017 was landed already. Docs: https://swr.vercel.app/advanced/cache.