Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data is always undefined first, even when cached #144

Closed
jstcki opened this issue Nov 21, 2019 · 3 comments · Fixed by #148
Closed

data is always undefined first, even when cached #144

jstcki opened this issue Nov 21, 2019 · 3 comments · Fixed by #148

Comments

@jstcki
Copy link

jstcki commented Nov 21, 2019

Whenever a key is used that has already cached data, data will be undefined anyway first and then immediately cause a re-render with the cached data.

I've set up an example here:
https://codesandbox.io/s/determined-almeida-3nwpv

You'll see that even for cached results, the sequence will be like this:

CLICKED
data: undefined, isValidating: false <-- causes an unnecessary render with undefined data!
data: Hello Bar, isValidating: false
data: Hello Bar, isValidating: true
data: Hello Bar, isValidating: false

… where I would expect a sequence like this:

CLICKED
data: Hello Bar, isValidating: false <-- Even this step could be optional?
data: Hello Bar, isValidating: true
data: Hello Bar, isValidating: false
@sergiodxa
Copy link
Contributor

This is because SWR uses useEffect to read from cache I believe, I know when you enable Suspense it will try to read from cache right away.

@shuding
Copy link
Member

shuding commented Nov 22, 2019

Amazing reproduction example! Thanks @herrstucki.

The major reason is that when changing the key, it's still the same hook. For example:

const [state] = useState(key)

Only the initial key will affect the state. If the key has changed, it returns undefined (because of course it can't just return the previous state of the previous key), and uses setState to update it (inside useEffect), so the first render will be undefined, and set to the correct data in a second render.

However I do think this is fixable (https://github.com/zeit/swr/blob/master/src/use-swr.ts#L544-L547), by just trying to return the cached data (actually the initialData value, it also handles the hydration case), rather than undefined.

Will let you know when this gets implemented 👍

@jstcki
Copy link
Author

jstcki commented Nov 22, 2019

Thanks @quietshu!

I tried to figure out how things work by looking at the source but to be honest I didn't quite understand it at first glance 😅

I'm not quite sure why a changed key even needs to update state in the first place … reading from cache is synchronous anyway, no? Maybe a simplified pseudo-code example of what I mean:

function useSWR(key, fetcher) {
  const [isValidating, setValidating] = useState(false);

  let currentData;
  if (cache.has(key)) {
    currentData = cache.get(key); // synchronous read
  }
  
  useEffect(() => {
    if (needsRevalidating(key)) {
       // will update cache, cache will notify "subscribers" via context or whatever
      triggerRevalidating(key)
      setValidating(true)
    }
  }, [key, setValidating])

  
  return { data: currentData, isValidating };
}

@shuding shuding mentioned this issue Nov 26, 2019
pacocoursey pushed a commit that referenced this issue Dec 2, 2019
* fix the case that the arg array is empty

* fix hash primitive types

* add prefetch to the readme

* improve wording

* return cached state and remove hydration, fixes #144

* remove useHydration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants