Issue 1933 - Cancellation of requests when no longer needed #2275
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit aa869c7:
|
CaptainPants
changed the title
|
|
||
| /** | ||
| * Enable this to pass an instance of AbortSignal to your fetcher that will be aborted when there no useSWR hooks remaining that are listening for the result of the fetch (as | ||
| * a result for example of navigation, or change of search input). This allows you to cancel a long running a request if the result is no longer relevant, e.g. when doing a search. | ||
| */ | ||
| abortDiscardedRequests?: boolean |
There was a problem hiding this comment.
I would like for this feature to be enabled by default. Not the other way around.
Let's emphasize what the new users want rather than reverse compatibility since the risk of problems is low.
Would a new user who has no prior version of swr installed prefer this on by default or not? I think yes but I'm only one user.
There was a problem hiding this comment.
Lest setting this to true give someone a false impression, I think this config should be removed altogether.
- Setting
abortDiscardedRequeststotrueis insufficient for aborting discarded requests. Both of these also must be true
a. The request API supports cancellation withAbortSignal(not all do)
b. You remembered to pass thesignalto that API - Simply ‘not passing the
signalto the request API’ is sufficient to disable the cancellation of discarded requests.
I think we should not offer this config, but rather always pass signal to the fetcher function.
If you're worried about constructing a bunch of AbortControllers that nobody will ever use (don't) you could always create a Proxy and only construct the AbortController when someone gets the signal property.
Removing personal email.
CaptainPants
force-pushed
the
issue-1933
branch
from
3b64fe5
to
aa869c7
Compare
|
Is there anything missing from this preventing it from being merged? |
|
This would be very helpful to have! Any ideas when will this get released? |
|
Exciting features! looking forward to launch. |
| */ | ||
| export interface FetcherOptions { | ||
| signal?: AbortSignal | ||
| [key: string]: unknown |
There was a problem hiding this comment.
I'd omit an index signature because it may hide typos for the valid options (only signal in this case). It also prevents us from adding properties without introducing a potential breaking change.
First attempt at #1933 - adding a cancellation mechanism to fetchers.
Looking for feedback on the approach and if there is any huge landmines I'm missing. I won't be offended if this is just a big 'no'.
Still working on how to appropriately test this.
TODO: