vercel / swr

React Hooks for Data Fetching
https://swr.vercel.app
MIT License
30.43k stars 1.21k forks source link

Data is not updated with `initialData` #284

Closed Kerumen closed 3 years ago

Kerumen commented 4 years ago

I'm on a Next.js app and here is my (simplified) code:

const IndexPage = ({ data: initialData }) => {
  const [filters, setFilters] = useState(defaultFilters)

  const onChange = () => {
    ...
    setFilters(newFilters)
  }

  const query = getQuery(filters)
  const { data } = useSWR(`/api/resorts?${query}`, fetcher, { initialData })

  return (...)
}

Index.getInitialProps = async ctx => {
  const query = getQuery(defaultFilters)
  const data: Resort[] = await fetcher(`${getHost(ctx)}/api/resorts?${query}`)
  return { data }
}

I have an initial set of filter (defaultFilters) on which I query and pass to useSWR with initialData on first render. When the user changes one filter, the key should change as it's a new query, but useSWR still returns the old data, from the first call.

When I remove initialData from useSWR it works but that's not what I want, I want to have SSR.

Am I doing something wrong?

sergiodxa commented 4 years ago

I replicated it here https://codesandbox.io/s/eager-lovelace-hgj9h

A fix right now is to grab mutate from useSWR results and call it immediately. In the CodeSandbox there is a fix running useEffect to call the mutate function and revalidate automatically.

Another fix, also in the CodeSandbox, is to change initialData to undefined if the dynamic part (in your case the query) is not the same as the initial one, to do this in your case you will need to pass the "initialQuery" (the one from getInitialProps) to the component as a prop and use it to detect if you should use or no initialData.

I think this makes sense, SWR is using the initialData if there is no a cached data already as the value of your current key, when you change the key is using the same initialData for the new key.

I think the bug here, if it's a bug and not intentional, is that SWR should detect the key changed and ignore the initialData, only use the cache.

Kerumen commented 4 years ago

Thanks for the suggestion! I applied the "undefined" fix.

I think the bug here, if it's a bug and not intentional, is that SWR should detect the key changed and ignore the initialData, only use the cache.

Indeed it seems to be a bug because it's not intuitive and not documented either..

pawelphilipczyk commented 4 years ago

I agree and I don't see any good use case for current behaviour. If for each key there is always initial data updated on the server side then why do I need using this hook at all?

majelbstoat commented 4 years ago

I also consider this a bug. In my use-case, I'm using a custom cache key that incorporates the viewer of the page, so that data is force refetched on login/logout. (This is in a useData hook that calls useSWR.) Because of this, it's not particularly easy for me to apply the undefined or mutate workaround because the key that I supply to useData() doesn't actually change, but the cache key changes in response to a state change elsewhere. Simplified example:

// data.ts
const useData(key: string, config?: ConfigInterface) => {
  const { viewer } = useContext(Auth0Context)
  const cacheKey = `${viewer?.id || ''}:${key}`
  const url = process.browser ? key : `https://localhost${key}`
  return useSwr(cacheKey, () => fetch(url).then((r) => r.json()), config)
}
// index.tsx
const IndexPage = ({ username, initialData }) => {
  const data = useData(`/api/v1/users/${username}`, { initialData })
  if (!data) return <h1>Loading</h1>
  return <Profile user={data} />
}

export const getServerSideProps = async ({ query }) => {
  const { username } = query
  const initialData = getUser(username)
  return {
    props: {
      username,
      initialData,
    },
  }
}

As with others, I've noticed that switching focus and refocusing does trigger a refetch, and that commenting out initialData means a refetch takes place immediately when the key changes, as I would expect.

majelbstoat commented 4 years ago

It looks like this might be a duplicate of https://github.com/zeit/swr/issues/230

shuding commented 4 years ago

I think the real problem here is "fallback" vs "cache". initialData was designed to be per hook and it was considered as "fallback". Let's say 2 components:

function A() {
  useSWR('data', fetcher, { initialData: 1 })
}
function B() {
  useSWR('data', fetcher, { initialData: 2 })
}

It will be very confusing if initialData (the fallback) is bound to the key, if the keys are the same / are changing.

Ideally, a better API design should be context (note this API doesn't exist):

<SWRConfig initialData={initialData}> // a key-value object
  <App/>
</SWRConfig>

And all the SWR hooks inside the tree should use it as the data source.

majelbstoat commented 4 years ago

@quietshu, so under that approach, and using with NextJS, we'd do getServersideProps (well, getInitialProps until gSP is supported on _app.tsx and _document.tsx), return initialData and then pass that to a SWRConfig defined in _app.tsx or _document.tsx. Is that right?

shuding commented 4 years ago

@majelbstoat yep.

pawelphilipczyk commented 4 years ago

OK so if I understand correctly currently the logic is that if I provide initialData for a hook then both for initial key and future keys per this hook it's considered a fallback if current key's data is not in cache.

In my understanding the key is often identifying data itself like for example user/profile/1234 or products/jeans. If I change the key to other user profile or product category I really do not expect the initialData try to provide defaults for me anymore.

I think this does make sense and is not confusing right? It's irrelevant what keys other hooks are using.

lkbr commented 4 years ago

Maybe I am failing to grasp things here or unaware of the inverse use cases, but I feel like the use case's described in this issue are more commonly the reason developers reach for the initialData feature.

Anyway, I solved my case by setting initialData to undefined when a new key differs from the "initial" key as @sergiodxa suggested. 🙏

ingoclaro commented 4 years ago

I don't really consider this a bug per se, it's actually how hooks work, right?. The function is executed each time the component re-renders, so on first render you will have: useSWR('/api/resorts?q=1', fetcher, { initialData }) and then when query changes: useSWR('/api/resorts?q=2', fetcher, { initialData }) with the same initialData.... so it doesn't change, even when providing a different key because you provided the same initialData.

If you do something like this you would get a different initial data each time:

const Page = ({ id }) => {
   let initialData = { message: `initial Message ${id}` }
   const { data, mutate } = useSWR(`/api/data?id=${id}`, fetcher, { initialData })

I think that the issue is that the initialData isn't revalidated, so even if you trigger a new swr call with a new key in this case, since initialData is present, that is used instead, so the api call never happens unless you manually trigger a revalidation... perhaps to avoid confusion it should revalidate the initialData by default?

other alternative is to use initialData as a default when data is empty instead of providing it to the swr hook (of course would need to change that const { data } into a let):

   if (!data) data = initialData
pawelphilipczyk commented 4 years ago

I don't feel this is the way hooks work in this case. Almost the same argument would be to say useState default value should be set to undefined by developer once you do first setState or you would get the unchanged state all the time.

Even more initialData is explicitly used as example for SSR. In this case the data is provided once and should be considered irrelevant once the key changes. Anytime I change the key I want to fetch new data and I don't care about initialData anymore.

If this is not a bug then maybe at least a suggestion for improvement in docs? Additionally some other prop could be added to api alongside initialData

majelbstoat commented 4 years ago

Whether or not it's a bug, it's clearly unexpected for quite a few people, and prevents one of the more common use-cases of server side rendering of dynamic routes without an unintuitive workaround. @shuding's proposal looks to be a good one (and maybe we could rename the argument to SWRConfig "cache" for clarity?), so hopefully something like that lands soon.

(revalidating the initial data by default would lead to a second unnecessary request on first load for server side rendered pages - I believe that's what the behaviour used to be and was changed, for this reason.)

pawelphilipczyk commented 4 years ago

How would @shuding proposal work in this case: I'm rendering page with getServerSideProps and have this initialData passed to my page component. The context wrapper in this case is somewhere else up in _app.js and I don't have access to it to pass this data.

I could add separate SWRConfig on rendered page but I feel like React hooks were a step towards reducing use of "wrappers" and maybe we could find a solution without need of that context.

lkbr commented 4 years ago

I was against this at first, but the more I think about it the more I realise it goes against what I fell in love with about useSWR: it's incredibly simple but insanely flexible.

I solve my use-case like this:

  const { data } = useSWR(key, fetcher, {
    initialData: key === initialKey ? initialData : undefined,
  })

initialKey is my initial algolia query formatted to a string that is passed to a simple fetch() which gets initialData in getServerSideProps. key is the updated version of the algolia query (whenever a user searches or changes a filter) that signals to useSWR that its time to take over. Whenever key doesn't equal initialKey we know initialData is no longer relevant.

Sure, the initialData option is not intuitive for the SSR use-case, but adding a new option for every unintuitive case (that can be solved in user-land) that we discover will slowly morph this package into the the very thing we are escaping (atleast, what I am escaping anyway 😄)

majelbstoat commented 4 years ago

@pawelphilipczyk: Next will be adding support for getServersideProps to _app at some point. Per discussions, it is "just a temporary restriction" (https://github.com/zeit/next.js/discussions/10874#discussioncomment-1085). If it works similarly to getInitialProps, _app.getServersideProps will receive the props from MyPage.getServersideProps, from where it can populate the cache.

@lkbr: I agree in general that we should limit complexity. However, SWR is suggested as the library to use for doing data fetching for Next, and Next supports SSR out of the box. (On the Next documentation, they are literally described on the same page!) SSR is not a minor use-case. And they're both created by zeit :)

Consider the case where we're fetching data from an API based on a site URL path parameter. For example with a page /profiles/[username], visiting/profiles/lkbr, my props are {username: "lkbr"} and we need to get data from /api/users/${username}, so that's my key. When I client side nav to /profiles/another, the props for the page change to {username: "another"}. How do I compare that key to the previous one? Do I now need to store a memoised reference to the original data in some kind of pseudo on-mount?

lkbr commented 4 years ago

Good point ^^

ingoclaro commented 4 years ago

these are great points! you totally convinced me. I actually was confusing initialData with a default value. As the name even implies, initialData should be used just once, probably only after the component is mounted. This would be much easier to have it implemented in swr than on userland, which requires going through extra hoops to store that initial key.

created a failing test for this:

it('should not use initial data when the key changes', async () => {
    const fetcher = jest.fn(() => 'new data')

    function Page() {
      const [key, setKey] = useState('initial-data-1')
      const { data } = useSWR(key, fetcher, {
        initialData: 'Initial'
      })
      return <div onClick={() => setKey('new-data-1')}>hello, {data}</div>
    }

    const { container } = render(<Page />)

    fireEvent.click(container.firstElementChild)

    expect(fetcher).toBeCalled()
    expect(container.firstChild.textContent).toMatchInlineSnapshot(
      `"hello, new data"`
    )
  })
lkbr commented 4 years ago

Sigh, I'm still on the fence 😅. The reason I keep nitpicking is because I don't like the idea of moving initialData out of hooks and into <SWRConfig />.

@majelbstoat I realise you've probably simplified your example for communication, so forgive me if I'm misunderstanding...

If you're using getServerSideProps wouldn't /profiles/[username] update when you navigated to to /profiles/another? As far as I understand, getServerSideProps blocks the client side page transition, fetches on the server then passes the props, then transitions.

@ingoclaro, "which requires going through extra hoops to store that initial key." You have to pass down initialData from the server anyway, couldn't you also pass down initialKey too?

majelbstoat commented 4 years ago

@lkbr I would have thought so, but that's not what appears to happen. AFAICT, initialData doesn't change within the hook after the first page load. (Which is the problem, I think. If both the key and the initialData changed, it would be fine. But the key changes and initialData doesn't.)

The solution doesn't have to be SWRConfig. I just jumped on it because I think it will work, and this currently prevents me from using SWR with SSR 😂

ingoclaro commented 4 years ago

@lkbr yea, would need to pass initial key and do the conditional to not use initialData if the key changes... doesn't sound very intuitive for me, when the library can do that automatically. Component/page now needs to know about a property just for the purpose of flipping the initialData...

mxmzb commented 4 years ago

Whether this is a bug or intended, I think it should be documented properly (and same goes for the currently nowhere mentioned useSWRPages).

matigda commented 4 years ago

I mean this solution really doesn't work for me. I'm building shop right now and there is hook useProduct. The problem is that when I go from one product to another and do this for some time, even though I pass {initialData: undefined} I still get the data from cache at first. It's really annoying, cause when user changes product url changes, but data stays the same.

majelbstoat commented 4 years ago

Yeah, I currently just don't use initialData for this exact reason @matigda. Which is a shame given that the NextJS provides simple server side rendering :/

matigda commented 4 years ago

but you are still using useSWR ? @majelbstoat if so how did you manage to work that out? because if you are not using initialData then you have 2 requests and can't really use that for SSR. Or did you just went with useState ?

majelbstoat commented 4 years ago

I just don't fetch data on the server, and I'm being patient hoping this gets resolved 🙂

matigda commented 4 years ago

IMO really the key here is to not bound call with the key, nor with the initialData, but make useSWR more like useState. If someone wants to change the key used by useSWR he should be able to do it by method returned from useSWR. I found another issue associated with bounding mutation to key. Can elaborate on it, but it may be just my misunderstanding of architectural decisions, so not sure whether to start discussion on github or ask it on stackoverflow.

But I think if there was something like updateKey returned from useSWR it would solve a lot of issues. Yeah, it would require something like useEffect({} => {updateKey(key)}, [key]); and it actually can be incorporated directly into library I guess.

Or maybe just another param that would actually be the key for given useSWR usage. I don't think using passed params as cache keys is good. For example I have two hooks - useCategory and useProduct ( both use useSWR underneath ) and if I call them both with the same param I end up with two hooks returning the same data. And I don't want to pass url to fetcher. My fetchers knows the urls ( but those urls sometimes need params ) and theirs role is to fetch and map data. Why that way? Because then I can use the same fetcher in my getInitialProps and pass data that is already mapped to my page. My UI is independent from backend and it just requires certain UI entities to work. So all the mapping is in fetchers. If i have to pass url to fetcher I have to do it twice. But my fetchers are created per resource so it doesn't really make sense to pass them urls.

I know the examples from documentation but they are unfortunately really basic. I don't mean to offend anyone. If I can help I gladly will.

majelbstoat commented 4 years ago

This issue has been open a while, and it seems pretty well discussed. @shuding, in your opinion, is this a behaviour that is likely to change? (Or else to otherwise support SSR with dynamic keys?) On pages with multiple requests, keeping track of all the initial queries is a bunch of confusing book keeping, and forgetting to put the undefined hack in causes pretty bad brokenness in terms of data inconsistency.

isaac-scarrott commented 4 years ago

I've made a custom hook/wrapper around useSwr that fixes this issue with Next.js if anyone is interested:

import { useEffect, useRef } from 'react'
import _useSwr from 'swr'

const useSwr = (key, fn, config) => {
  const hasMounted = useRef(false)

  useEffect(() => {
    hasMounted.current = true
  }, [])

  return _useSwr(key, fn, {
    ...config,
    initialData: hasMounted.current ? undefined : config.initialData,
  })
}

export default useSwr
pke commented 4 years ago

@isaac-scarrott I wonder why did you use useRef instead of useState?

isaac-scarrott commented 4 years ago

@pke I used useRef as this will not trigger a rerender where as useState will trigger a rerender. In this case we don't want to UI to update when we change the value of hasMounted to true, so by using useRef instead of useState when we update the ref value this will not trigger a rerender and created a very small performance increase.

pke commented 4 years ago

Nice trick, is this a recommended approach in general? @isaac-scarrott

isaac-scarrott commented 4 years ago

I would say yes it is unofficially recommended. Dan Abramov on Twitter explains it well here if you're familiar with classes in javascript (React or plain JS).

Also in this google search you can see a few articles endorsing/explaining it.

Romstar commented 4 years ago

I'm on a Next.js app and here is my (simplified) code:

const IndexPage = ({ data: initialData }) => {
  const [filters, setFilters] = useState(defaultFilters)

  const onChange = () => {
    ...
    setFilters(newFilters)
  }

  const query = getQuery(filters)
  const { data } = useSWR(`/api/resorts?${query}`, fetcher, { initialData })

  return (...)
}

Index.getInitialProps = async ctx => {
  const query = getQuery(defaultFilters)
  const data: Resort[] = await fetcher(`${getHost(ctx)}/api/resorts?${query}`)
  return { data }
}

I have an initial set of filter (defaultFilters) on which I query and pass to useSWR with initialData on first render. When the user changes one filter, the key should change as it's a new query, but useSWR still returns the old data, from the first call.

When I remove initialData from useSWR it works but that's not what I want, I want to have SSR.

Am I doing something wrong?

I ended up using this config option: revalidateOnMount: true.

initialData: initial data to be returned (note: This is per-hook)

revalidateOnMount: enable or disable automatic revalidation when component is mounted (by default revalidation occurs on mount when initialData is not set, use this flag to force behavior)

Example:

const { data: listOfUsers, mutate: setListOfUsers } = useSWR(API_ROUTES.ADMIN.USERS, { fetcher, initialData: [], revalidateOnMount: true });

and now my data is initialized as an empty array and the data is pulled after the first render which then causes the state to be updated and my array is populated with real data which then allows my app to work properly!

nfantone commented 4 years ago

Fell into the same rabbit hole as everybody else here.

I'm implementing a basic search results page where the first set of results is fetched server side (for SEO purposes). The way useSWR currently works with initialData means that while the very first list of results is returned just fine, new searches keep returning the same items. For some reason, I found that it won't even trigger network requests even if the key changes, in some cases.

@isaac-scarrott proposed solution actually worked for me 👍.

import { useEffect, useRef } from 'react';
import _useSwr from 'swr';

/**
 * Fetches data using a "stale-while-revalidate" approach.
 *
 * @see https://github.com/vercel/swr
 * @param {import('swr').keyInterface} key A unique key string for the request (or a function / array / `null`).
 * @param {import('swr').ConfigInterface} [options={}] An object of options for this SWR hook.
 * @returns {import('swr').responseInterface} SWR response map.
 */
function useSwr(key, config = {}) {
  // We pass `initialData` to `useSwr` on the first render exclusively
  // to prevent SWR from returning the same cached value if `key` changes
  const didMount = useRef(false);

  useEffect(() => {
    didMount.current = true;
  }, []);

  return _useSwr(key, {
    ...config,
    initialData: didMount.current ? undefined : config.initialData
  });
}

export default useSwr;

The only downside I see is that, unless using TS, you lose typings and intellisense on VSCode.

@lkbr

I solve my use-case like this:

  const { data } = useSWR(key, fetcher, {
    initialData: key === initialKey ? initialData : undefined,
  })

Right - but then you'd always be returning initialData for the same original key, no matter when you do it, which might have become stale at some point.

@Romstar As pointed out by @majelbstoat some comments ago, if you go down that route you'll be triggering a second request for basically the same data - preventing that is one of the upsides of using SSR in the first place.

(revalidating the initial data by default would lead to a second unnecessary request on first load for server side rendered pages - I believe that's what the behaviour used to be and was changed, for this reason.)

@shuding

@quietshu, so under that approach, and using with NextJS, we'd do getServersideProps (well, getInitialProps until gSP is supported on _app.tsx and _document.tsx), return initialData and then pass that to a SWRConfig defined in _app.tsx or _document.tsx. Is that right?

Yup.

IMHO, I don't feel like this would be a smart way to go about this moving forward. Not only it would break logic and data locality significantly, but as the current Next.js API stands, it would also completely void the ability to statically optimize pages in your app that could benefit from it, right?

danielrbradley commented 4 years ago

Here's a fully Typescript version of what @isaac-scarrott wrote for a workaround:

import { useEffect, useRef } from 'react';
import _useSwr from 'swr';
import { ConfigInterface, keyInterface, fetcherFn, responseInterface } from 'swr/dist/types';

/**
 * Patched version of SWR to work around bug where a key change is not detected when using initial data
 * @see https://github.com/vercel/swr/issues/284
 */
const useSwr = <Data, Error>(
  key: keyInterface,
  fn?: fetcherFn<Data>,
  config?: ConfigInterface<Data, Error>
): responseInterface<Data, Error> => {
  const hasMounted = useRef(false);

  useEffect(() => {
    hasMounted.current = true;
  }, []);

  return _useSwr<Data, Error>(key, fn, {
    ...config,
    initialData: hasMounted.current ? undefined : config?.initialData,
  });
};

export default useSwr;

I also agree with @nfantone that this needs addressing as it seems like it's going to continue causing significant issues going forward. I think we're needing agreement from the maintainers on what the expected behaviour so we can start working on a fix.

nfantone commented 4 years ago

I can't believe this bit me again today 😥. A customer raised an issue explaining that they "weren't seeing the new email entry in their table after creating it". And, lo and behold, it was from an older project where the workaround proposed here for initialData was not in place.

At this point, I believe this issue has been open for long enough and really needs to be addressed. I'm open to putting together myself a PR to integrate the didMount solution into swr, if there's an interest for it.

Let's hear from collaborators.

jacobedawson commented 4 years ago

For googlers who are puzzled when they have set initialData hoping for SWR to push fresh data when necessary (while also possibly fetching initialData in getServerSideProps), I found that setting revalidateOnMount: true (as suggested by @Romstar ) solved the issue of updating a page with e.g. a new item. It's worth a try if that's your issue, before going on to some of the more complex solutions suggested by other people here (at least it solved my update issue on page navigation).

gesposito commented 3 years ago

My current approach

export default function Page(props) {
  const [filter, setFilter] = useState(1);
  const [initialData, setInitialData] = useState(props.data);

  useEffect(() => {
    setInitialData(null);
  }, [filter]);

  return <></>;
}
rxb commented 3 years ago

@jacobedawson thank you for the tip! revalidateOnMount: true does revalidate.

My issue now is that I'm fetching the same data twice in a row on the client for every client-side page route, once in getInitialProps (which is thrown out) and once again right after that when swr revalidates on mount. I'm considering detecting if the page is being rendered SSR and only fetching if the render is on the server.

bardsol commented 3 years ago

I also ran into this issue thinking that initialData was a default data prop, like useState(data). But instead of attacking it at the key level, I decided to approach this at the fetcher function. You can create your own initialData at the fetcher level and only return it on the initial route. Downside is you still need to keep track of what was your initial key on mount.

const withCache = (fetcher, { refreshTimeout = 0, defaultData = {} } = {}) => async (route) => {
  if (cache.has(route) || defaultData[route]) {
    const data = cache.get(route) ?? defaultData[route];
    if (Date.now() - data.lastFetched < refreshTimeout) {
      return data;
    }
  }
  return await fetcher(route);
};

Her is an example of the usage:

const defaultData = useRef({ [keyTotheInitialRoute]: someDefaultInitialData });
const { data } = useSWR(key, withCache(fetcher, { refreshTimeout: 10000, defaultData: defaultData.current }))

This example expects a timestamp ({lastFetched}) on all data from the server.

olvert commented 3 years ago

I also ran into this problem and used the workaround from @danielrbradley which worked great. It uses some deprecated types though which I had to update. My version ended up looking like this:

import { useEffect, useRef } from 'react';
import _useSwr, { Key } from 'swr';
import { Fetcher, SWRConfiguration, SWRResponse } from 'swr/dist/types';

/**
 * @see https://github.com/vercel/swr/issues/284
 * @see https://github.com/vercel/swr/issues/284#issuecomment-706094532
 */
export const useSWR = <Data = never, Error = never>(
  key: Key,
  fn?: Fetcher<Data>,
  config?: SWRConfiguration<Data, Error>,
): SWRResponse<Data, Error> => {
  const hasMounted = useRef(false);

  useEffect(() => { hasMounted.current = true; }, []);

  return _useSwr<Data, Error>(key, fn, {
    ...config,
    initialData: hasMounted.current ? undefined : config?.initialData,
  });
};
skoob13 commented 3 years ago

There is the another workaround using cache. It allows to delete a cache entry synchronously before initialization of useSWR hook, so it removes the useless rerender from other solutions:

import useSWR, { cache } from 'swr';

export function usePagination(initialData) {
  const mounted = React.useRef(false);
  const key = '/list';

  if (cache.has(key) && !mounted.current) {
    cache.delete(key);
  }

  mounted.current = true;

  return useSWR(key, { initialData });
}

However, it doesn't delete other SWR cache entries for isValidation and error states. If you need that, there is cache.serializeKey method that returns additional keys.

felixcatto commented 3 years ago

I also ran into this problem. I have custom SSR implementation, no NextJs. What we want after we load the page

if (initialData && appFirstRender)  // do NOT perform revalidation
if (initialData && !appFirstRender) // perform revalidation
if (!initialData)                   // perform revalidation

But the problem is that SWR have no knowledge about is our app rendering first time or not. And i doubt that it a SWR concern.

My workaround

export const useSWR = (url, config = {}) => {
  const { isFirstRender } = useContext();
  const revalidateOnMount = !config.initialData || (config.initialData && !isFirstRender.current);
  return useSWROriginal(url, { ...config, revalidateOnMount });
};

And i added this code at top of my app

const isFirstRender = React.useRef(true);

React.useEffect(() => {
  isFirstRender.current = false;
}, []);

return <Context.Provider value={{ isFirstRender }}>...

What imporvements we can do at library side? I think SWR can support additional option - isFirstRender ref. We anyway need to provide it to SWR

const isFirstRender = React.useRef(true);
...
return <SWRConfig value={{ isFirstRender }}>...

but we don't need to write our custom useSWR hook, because this check will be made inside of library.

pablocubico commented 3 years ago

For any other googlers running into this, remember to be 100% certain that the key is the same. If you are using a custom fetcher, beware! I was struggling with this until I found out about the "multiple arguments" feature.

https://swr.vercel.app/docs/arguments#multiple-arguments

Example:

const fetcher = (url, access_token) => {
  return fetch(url, {
    headers: new Headers({
      ...,
      token: access_token,
    }),
    credentials: "same-origin",
  }).then((res) => res.json());
};

const { data: settings, mutate: setSettings } = useSWR(
  ["/api/settings", access_token],
  fetcher,
  {
    initialData: defaultSettings,
    revalidateOnMount: true,
  }
);

As the docs say: The function fetchWithToken still accepts the same 2 arguments, but the cache key will also be associated with token now.

Using multiple arguments and revalidateOnMount worked perfectly for me.

huozhi commented 3 years ago

Now with swr@beta, If you're using next.js, you can use custom cache API to initialize the very first state

import { createCache } from 'swr'

let cacheProvider

export default function IndexPage({ initialState }) {
  if (!cacheProvider) {
    cacheProvider = createCache(new Map(initialState))
  }
  return (
    <SWRConfig value={{ cache: cacheProvider.cache }}>
      <Index />
    </SWRConfig>
  );
}
export async function getServerSideProps() {
  return {
    props: {
      initialState: [ [key1, value1], [key2, value2], [key3, value3] ]  /* initial states */
    }
  };
}
shuding commented 3 years ago

As https://github.com/vercel/swr/issues/284#issuecomment-869174803 mentioned, the new Cache API will be the official replacement of initialData for many use cases. Please try it out! We have a more detailed docs here too: https://swr.vercel.app/advanced/cache.

patroza commented 3 years ago

I also ran into this problem. I have custom SSR implementation, no NextJs. What we want after we load the page

if (initialData && appFirstRender)  // do NOT perform revalidation
if (initialData && !appFirstRender) // perform revalidation
if (!initialData)                   // perform revalidation

But the problem is that SWR have no knowledge about is our app rendering first time or not. And i doubt that it a SWR concern.

My workaround

export const useSWR = (url, config = {}) => {
  const { isFirstRender } = useContext();
  const revalidateOnMount = !config.initialData || (config.initialData && !isFirstRender.current);
  return useSWROriginal(url, { ...config, revalidateOnMount });
};

And i added this code at top of my app

const isFirstRender = React.useRef(true);

React.useEffect(() => {
  isFirstRender.current = false;
}, []);

return <Context.Provider value={{ isFirstRender }}>...

What imporvements we can do at library side? I think SWR can support additional option - isFirstRender ref. We anyway need to provide it to SWR

const isFirstRender = React.useRef(true);
...
return <SWRConfig value={{ isFirstRender }}>...

but we don't need to write our custom useSWR hook, because this check will be made inside of library.

actually, that means you will do both a SSR call for json, and another fetch request by the client.

As #284 (comment) mentioned, the new Cache API will be the official replacement of initialData for many use cases. Please try it out! We have a more detailed docs here too: https://swr.vercel.app/advanced/cache.

How is this exactly supposed to work? What is the canonical way of solving this problem?

felixcatto commented 3 years ago

Nope, you are wrong. At first render revalidateOnMount: false, so no additional fetch. At second+ render because root component never unmounts, i have revalidateOnMount: true. So it works as expected.

patroza commented 3 years ago

Nope, you are wrong. At first render revalidateOnMount: false, so no additional fetch. At second+ render because root component never unmounts, i have revalidateOnMount: true. So it works as expected.

Im afraid with SSR that leads to two calls for second+:

  1. Nextjs does a SSR json call, I pass it on as initialData
  2. SWR ignores the initialData cause it has a cache entry already and then does a revalidate on mount

so two calls. I think the better solution to have only 1 call with NextJS+SWR+SSR is this solution: https://github.com/vercel/swr/issues/284#issuecomment-853730897