Skip to content

Fix passing fetch options to built-in fetchers - #2085

Open
axelboc wants to merge 1 commit into
mainfrom
fix-fetchers
Open

axelboc wants to merge 1 commit into
mainfrom
fix-fetchers

Conversation

@axelboc

@axelboc axelboc commented Oct 15, 2026

Copy link
Copy Markdown
Contributor
  • With the basic fetcher, default headers (passed to createBasicFetcher) were being overridden by local headers (passed to the actual fetcher).
  • With the axios fetcher, local headers as well as method and body were simply not used. This was a complete oversight.

fetchOpts: Omit<RequestInit, 'signal'> = {},
fetchOpts: {
headers?: Record<string, string>;
} & Omit<RequestInit, keyof FetcherOptions> = {},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still give the flexibility to pass fetch options like credentials, etc. but not the options that only make sense on a per-request basis (FetcherOptions).

Passing headers is explicitly allowed in a Record format so they can be merged easily

Comment on lines +18 to +19
export interface FetcherOptions extends Pick<RequestInit, 'method' | 'body'> {
headers?: Record<string, string>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FetcherOptions now includes only the options that are actually used by the providers.

Comment on lines +104 to +108
method,
headers: { ...defaultHeaders, ...headers },
body,
signal: abortSignal,
...moreFetchOpts,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we know exactly what's going on and users can't mistakenly pass an option to createBasicFetcher that makes no sense or gets overridden.

@axelboc
axelboc requested a review from loichuder October 15, 2026 15:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant