dai-shi / react-suspense-fetch

[NOT MAINTAINED] A low-level library for React Suspense for Data Fetching
MIT License
296 stars 8 forks source link

Support cancellation #42

Closed tom-sherman closed 2 years ago

tom-sherman commented 2 years ago

If a component rerenders the store will refetch without cancelling the in-flight fetch.

Just spam the button on this demo: https://codesandbox.io/s/react-suspense-fetch-spam-ibqdlp?file=/src/App.js

Is it possible to add query cancellation to this library? I'm assuming not but I could be wrong

tom-sherman commented 2 years ago

I think the only way of achieving this would be to add some form of query key a la React Query which would change the API to something like store.get(key, input). The FetchStore could then pass a signal into the FetchFunc.

Putting the key as the second param would be a non breaking change and would also allow it to be optional: store.get(input, key)

dai-shi commented 2 years ago

Thanks for opening a new discussion.

Is it possible to add query cancellation to this library?

It's actually possible if we define an API. I'm not sure how intuitive the usage is. I can try implementing it and you can feel it.

Putting the key as the second param would be a non breaking change

Hmm, yeah, this would work and fairly easy to use. I just don't prefer the idea of key... I would create a new store for each key (but, we lose shared cache). Or maybe just an option to the store (assuming we always want "one" active fetch).

When do you want not to cancel fetching?

tom-sherman commented 2 years ago

When do you want not to cancel fetching?

This is an interesting question. I think calling .get() across multiple component instances wouldn't trigger cancellation. Conversely maybe it makes sense to always cancel when calling .get() inside the same component instance.

I don't know how this would look, altho it for sure wouldn't use user-provided keys as these would be the same across all component instances.

tom-sherman commented 2 years ago

For example:

function User({ id }) {
  store.get(id)
}

// This should cancel on each new `id` passed as a prop
<User id={id} />

// This should allow fetching both users in parallel, but if id1 or id2 changes then the relevant query should be cancelled
<>
  <User id={id1} />
  <User id={id2} />
</>
dai-shi commented 2 years ago

Thanks for your ideas. Hmm, I think key would still be sub-optimal in such complex cases. I'd try all-or-nothing strategy for a start.

btw, remember we should always prefetch() once before invoking get() multiple times. So, prefetch() is the timing for cancellation, not get().

tom-sherman commented 2 years ago

Hmm, I can't get my head around how prefetch orchestrates cancelation 🤔

Do you mean that cancelation should only trigger by calling prefetch() with different inputs? I don't understand why get() shouldn't work in a similar way

dai-shi commented 2 years ago

get() invokes prefetch() internally, but that's like an escape hatch. (For example, we might not have a chance to prefetch for the initial render.)