Possibility of adding cancel function to UseQueryOptions #1671
Replies: 5 comments 8 replies
-
I would tend to agree that the current approach is not ideal. We’ve had issues reported for both cases you described (async and TS). Maybe we could add the cancel option in a minor and remove the “cancel-on-the-promise” in the next major. Could you provide examples for:
some situations why this might not work:
|
Beta Was this translation helpful? Give feedback.
-
The reason for today’s api is so you can cancel specific instances of a fetch function that have been called. All, some, or one. If you extract out the cancel logic, you lose that fidelity that you can only get inside of the current scope of Query function execution. Yes, this means you can’t pass an async syntax function directly, but it’s simple enough to wrap into it with Promise.resolve()
Some pseudo code from my phone while laying in bed ;)
() => {
// create the cancel token
const promise = Promise.resolve().then(async () => {
// pass the cancel token
const stuff = await axios.get()
return stuff
}
promise.cancel = token.cancel
return promise
}
Tanner Linsley
…On Jan 23, 2021, 5:23 AM -0700, Dominik Dorfmeister ***@***.***>, wrote:
I would tend to agree that the current approach is not ideal. We’ve had issues reported for both cases you described (async and TS). Maybe we could add the cancel option in a minor and remove the “cancel-on-the-promise” in the next major. Could you provide examples for:
• axios without axios-cancel
• fetch w/ Abortcontroller
some situations why this might not work:
• you could have one subscriber with the option and one without - what happens then?
• will this approach work with queryClient.cancelQueries(key)?
• Will this approach work if you define this option on the queryDefaults in the QueryClientProvider?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Beta Was this translation helpful? Give feedback.
-
I would like to propose another alternative: have react-query provide its own // Fetch usage - look ma, I can use async functions at the top level and still support cancellation!
const query = useQuery('todos', async ({ signal }) => {
const res = await fetch('/todos', {
method: 'get',
// Pass the signal to your request
signal,
});
const json = await res.json();
let data = null;
if (!signal.aborted) {
// do expensive CPU transformation of data
data = expensiveTransform(json);
}
return data;
}); // axios usage
import axios from 'axios'
const query = useQuery('todos', ({ signal }) => {
// Create a new CancelToken source for this request
const CancelToken = axios.CancelToken
const source = CancelToken.source()
signal.addEventListener('abort', () => {
source.cancel('Query was cancelled by React Query')
})
return axios.get('/todos', {
// Pass the source token to your request
cancelToken: source.token,
})
}) // XMLHttpRequest usage
const query = useQuery('todos', ({ signal }) => {
return new Promise((resolve, reject) => {
var oReq = new XMLHttpRequest();
oReq.addEventListener('load', () => {
resolve(JSON.parse(oReq.responseText));
});
signal.addEventListener('abort', () => {
oReq.abort();
reject();
});
oReq.open('GET', 'http://www.example.org/example.txt');
oReq.send();
});
}); |
Beta Was this translation helpful? Give feedback.
-
I like this idea as long as we can make sure it doesnt require polyfills for all of our standard environments, eg most browsers, recent node versions, and React Native.
If we can guarantee that and also make the change non breaking, go for it. We can then deprecate the old way in a major in the future.
…On Oct 9, 2021, 7:50 AM -0600, James Wyatt Cready-Pyle ***@***.***>, wrote:
I can imagine a few ways to solve the issue of environments not having AbortController:
1. Explicitly call out the requirement in the docs/readme and provide links to polyfills (easiest).
2. Include a dependency on polyfill (easy but extra dependencies are strictly worse than fewer).
3. Only support this method of cancellation in environments that have support for AbortController. If it is available an AbortController is made and an AbortSignal instance is passed to the queryFn. If not then no AbortSignal is passed to the queryFn. The TS type for the signal property passed to the queryFn would simply be signal?: AbortSignal | null. It's a little more work for both the library and the consumer who would be forced to always check for truthy-ness (or just use !).
a. Another option here would be to keep the type as signal: AbortSignal and just fake the interface for environments w/o support (e.g. it would never actually abort) with a warning in the docs/readme about how environments w/o support will simply never see their AbortSignals actually become aborted:
const noopDescriptor = { value: () => {} };
function FakeAbortSignal() {}
Object.defineProperties(FakeAbortSignal.prototype, {
addEventListener: noopDescriptor,
removeEventListener: noopDescriptor,
aborted: { value: false }
});
Personally I'd go with 1 and then 3.i, but I'd be happy with any of those options as they'd all be superior to the existing solution. Here are just a few polyfills I've found:
• https://github.com/mo/abortcontroller-polyfill
• https://github.com/mysticatea/abort-controller
• https://github.com/benlesh/abort-controller-polyfill
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Beta Was this translation helpful? Give feedback.
-
Looks like axios v0.22.0 also has support for signal: axios/axios#3305 |
Beta Was this translation helpful? Give feedback.
-
The current way that cancellations are handled is by having the user attach a cancel function to the promise itself. One downside with this approach is that the user must attach the
cancel
function to the outermost promise, which means that it discourages using the async/await syntax for theQueryFunction
. Another minor inconvenience for typescript users is that there is nocancel
function on a Promise, so you have to create a new interface just for this.For example, this works:
But this does not:
I was thinking that maybe there could be an optional
cancel
function added to theUseQueryOptions
as an alternate way of handling query cancellation.For example:
Beta Was this translation helpful? Give feedback.
All reactions