fix: do not overwrite application provided abort signals

[ccouzens]

If the application has provided an abort signal to fetchOptions when initializing the Client, then that abort signal shouldn't be overwritten when makeFetchSource sets up its own abort controller.

In my case, this will allow me to easily set timeouts on requests from my web server to my graphql server.

---

I wanted to test this, but I can't see how without intrusive changes to the test suite.

Now we're using AbortSignal.any it validates that the arguments are actually abort signals. The mock functions are no longer sufficient. But the best way of getting an abort signal is through AbortControllers. But the test suite removes access to AbortController in a couple different files. For example here https://github.com/urql-graphql/urql/blob/92a101a5497040641306a783b56d5fc3ee0917b1/scripts/vitest/setup.js#L2

Summary

Set of changes

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 732f54f07ce0d23f174dac51121734b6a6f287f1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@urql/core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[JoviDeCroock]

Hmm, I looked into browser support for AbortSignal.any and it's a bit problematic 😅 only 89% https://caniuse.com/?search=AbortSignal.any - we could get around this by adding a listener for the abort on the supplied one (and ensure we properly clean it up)

[kitten]

I think with the current support status of AbortController.any, I'm not really in favour of this, since it trades it one problem for another. I find unexpected support window changes more problematic than the signal being ignored.

Generally, fetchOptions only exists for simple use-cases, and isn't intended for how it's sometimes used: specifying a custom fetch function that wraps fetch for the sake of replacing what exchanges are intended for and using fetchOptions.signal to "smooth over" what fetchSource does when a teardown signal is received (which also does "more" due to backpropagation, if that's used with reexecuteOperation) is basically an anti-pattern.

I'd be more inclined to say this is acceptable, if this at least did signal.addEventListener('abort', () => { abortController.abort() }) since that'd remove the trade-off at least.

But I still don't see a good argument for passing fetchOptions.signal in the first place, so another option here is to simply restrict the input type to explicitly mark it as unsupported

[ccouzens]

But I still don't see a good argument for passing fetchOptions.signal in the first place, so another option here is to simply restrict the input type to explicitly mark it as unsupported

I've raised the following PR for this.

https://github.com/urql-graphql/urql/pull/3788

I've kept it in draft while I explore some of the other suggestions, but feel free to merge if it's likely to be the preferred option.