react-request icon indicating copy to clipboard operation
react-request copied to clipboard

change `fetching` to be set to `true` on mount for non-lazy requests

Open ianstormtaylor opened this issue 7 years ago • 3 comments

I was wondering if you'd consider a slight change for non-lazy behavior. Right now it seems like you need to do:

<Fetch url="/me">
  {({ fetching, failed, data }) => {
    if (fetching) return ...
    if (failed) return ...
    if (!data) return null
    return ...
  }}
</Fetch>

There's an extra case where the component has just mounted, but hasn't yet started the request where fetching == false but data == null. Apollo seems to avoid this, such that you can handle the fetching and failed cases up front, and then assume you have data after that, which just reduces a little of the boilerplate.

Would you be down for that change?

ianstormtaylor avatar May 25 '18 00:05 ianstormtaylor

Sure! PRs welcome :v:

jamesplease avatar May 25 '18 00:05 jamesplease

I started working around this by creating my own Fetch wrapper that would detect this case and pass this along as fetching={true}. However, because I relying on fetching to show a loading state, I was seeing this activated when refreshing the data as well (when we have data cached but a network call is refreshing that data, i.e. cache-and-network).

I ultimately landed on a solution that passes through a new indicator called waitingForData that just checks that data === null. This seems to avoid the moment where we've mounted but not yet fetched (fetching === false && data === null) and showing a spinner when refreshing data (fetching === true && data !== null).

frolic avatar Jun 26 '18 16:06 frolic

I'm thinking that the fix here will be changing this line to be

fetching: !this.isLazy()

I'm working on a docs update now, but this will likely be in the next major release (v4)

Update: PR opened over in #190 . There are a lot of broken tests, so I'll likely fix it up closer to whenever a v4 release is pending (maybe a month or two)

jamesplease avatar Jul 13 '18 15:07 jamesplease