vercel / swr

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

The `isValidating` flag is no longer changing? #224

Closed Svish closed 4 years ago

Svish commented 4 years ago

From version 0.1.16 the isValidating flag doesn't seem to change anymore, it just stays at false, even when there is a request going on. Has the "API" of it changed after #186? Do I need to read it differently or something? Is it because I have wrapped useSWR in my own hook?

export default function useRequest(request, { initialData, ...config } = {}) {
  const { data: response, ...returned } = useSWR(
    request && JSON.stringify(request),
    () => axios(request),
    {
      ...config,
      initialData: initialData && {
        status: 200,
        statusText: 'InitialData',
        headers: {},
        data: initialData,
      },
    }
  );

  return {
    ...returned,
    data: response && response.data,
    response,
  };
}

Is there something in the way I get and pass on the returned values from useSWR here which is causing the isValidating flag not to change? If so, how would you suggest I do it instead?

Adding the following where I use my hook, just results in a single log message of undefined.

  useEffect(() => console.log(isValidating), [isValidating]);
Svish commented 4 years ago

Just tested replacing ...returned with isValidating, error, revalidate, which does make it work again. But I'm guessing this will also "undo" the re-render reduction you guys added in #186?

Is there a way I can write this useSWR wrapper so it doesn't "break" this re-render reduction?

export default function useRequest(request, { initialData, ...config } = {}) {
  const { data: response, isValidating, error, revalidate } = useSWR(
    request && JSON.stringify(request),
    () => axios(request),
    {
      ...config,
      initialData: initialData && {
        status: 200,
        statusText: 'InitialData',
        headers: {},
        data: initialData,
      },
    }
  );

  return {
    isValidating,
    error,
    revalidate,
    data: response && response.data,
    response,
  };
}
davecoates commented 4 years ago

This looks like it's because useSWR returns an object with getters on it... if you spread the return of that you lose the getters and the only property you'll get is revalidate.

You can keep the getters by mutating the returned object to add your own properties:

export default function useRequest(request, { initialData, ...config } = {}) {
  const returned = useSWR(
    request && JSON.stringify(request),
    () => axios(request),
    {
      ...config,
      initialData: initialData && {
        status: 200,
        statusText: 'InitialData',
        headers: {},
        data: initialData,
      },
    }
  );
  returned.response = returned.data
  return returned
}

I'm not entirely sure if there's issues with this approach though.

It is rather unexpected though - I had the same issue but I was spreading the entire response of useSWR so even data was undefined for me.

Svish commented 4 years ago

@davecoates Yeah, your workaround seem to work for me too, but not sure if the result of it is different from what I had already. And no idea if either of our solutions break/keep the optimization they've implemented with these getters. 🤔

davecoates commented 4 years ago

Yeah sorry for your case you'd want to add a getter on response as well so that it doesn't prematurely access data (my use case was just adding some extra options to the return that didn't need to read the values returned by useSWR)

eg. instead of returned.response = returned.data; do:

Object.defineProperties(returned, { 
    response: {
        get() { 
            return this.data;
       }
    },
});
Svish commented 4 years ago

Not sure that would work since I also replace data with response.data. I.e. I don't only add response, but replace data with something from the response 😕

What I'm trying to do here is that data should be the actual data, since that's what's expected when using the hook, but since I'm wrapping axios, then data returned from useSWR is actually the axios response object. So I need to do a little bit of a switcheroo before I return things from my useRequest wrapper. 🤔

drews256 commented 4 years ago

Bumping this issue.

While following the steps in the readme and not wrapping the SWR request I am seeing isValidating return undefined.

Using version 0.1.16, downgrading to 0.1.15 fixes the issue.

shuding commented 4 years ago

When using the spread operator, getter of the returned object can't be copied:

const { data, ...returned } = useSWR(...)

console.log(returned.isValidating) // will be undefined

Currently, we don't have a easy fix for it. I'll suggest not using ... for now:

const swr = useSWR(...)
const { data } = swr

console.log(swr.isValidating) // will change over time
shuding commented 4 years ago

The getter improvement reduces unnecessary re-renders dramatically. I'll keep this issue open and look for a better solution, or add a notice on the documentation.

cnzh2992 commented 4 years ago

Can we add enumerable: true in defineProperties to make it present?