Skip to content

Commit c490b19

Browse files
authored
Improve fetchBaseQuery default headers handling (#5112)
* Handle formData headers properly * Add some additional tests from other issues * Add handling for `accept` headers
1 parent 7b7faea commit c490b19

File tree

5 files changed

+381
-10
lines changed

5 files changed

+381
-10
lines changed

packages/toolkit/src/query/fetchBaseQuery.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ function stripUndefined(obj: any) {
105105
return copy
106106
}
107107

108+
// Only set the content-type to json if appropriate. Will not be true for FormData, ArrayBuffer, Blob, etc.
109+
const isJsonifiable = (body: any) =>
110+
typeof body === 'object' &&
111+
(isPlainObject(body) ||
112+
Array.isArray(body) ||
113+
typeof body.toJSON === 'function')
114+
108115
export type FetchBaseQueryArgs = {
109116
baseUrl?: string
110117
prepareHeaders?: (
@@ -252,21 +259,36 @@ export function fetchBaseQuery({
252259
extraOptions,
253260
})) || headers
254261

255-
// Only set the content-type to json if appropriate. Will not be true for FormData, ArrayBuffer, Blob, etc.
256-
const isJsonifiable = (body: any) =>
257-
typeof body === 'object' &&
258-
(isPlainObject(body) ||
259-
Array.isArray(body) ||
260-
typeof body.toJSON === 'function')
262+
const bodyIsJsonifiable = isJsonifiable(config.body)
263+
264+
// Remove content-type for non-jsonifiable bodies to let the browser set it automatically
265+
// Exception: keep content-type for string bodies as they might be intentional (text/plain, text/html, etc.)
266+
if (
267+
config.body != null &&
268+
!bodyIsJsonifiable &&
269+
typeof config.body !== 'string'
270+
) {
271+
config.headers.delete('content-type')
272+
}
261273

262-
if (!config.headers.has('content-type') && isJsonifiable(config.body)) {
274+
if (!config.headers.has('content-type') && bodyIsJsonifiable) {
263275
config.headers.set('content-type', jsonContentType)
264276
}
265277

266-
if (isJsonifiable(config.body) && isJsonContentType(config.headers)) {
278+
if (bodyIsJsonifiable && isJsonContentType(config.headers)) {
267279
config.body = JSON.stringify(config.body, jsonReplacer)
268280
}
269281

282+
// Set Accept header based on responseHandler if not already set
283+
if (!config.headers.has('accept')) {
284+
if (responseHandler === 'json') {
285+
config.headers.set('accept', 'application/json')
286+
} else if (responseHandler === 'text') {
287+
config.headers.set('accept', 'text/plain, text/html, */*')
288+
}
289+
// For 'content-type' responseHandler, don't set Accept (let server decide)
290+
}
291+
270292
if (params) {
271293
const divider = ~url.indexOf('?') ? '&' : '?'
272294
const query = paramsSerializer

packages/toolkit/src/query/tests/buildHooks.test.tsx

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { userEvent } from '@testing-library/user-event'
3535
import type { SyncScreen } from '@testing-library/react-render-stream/pure'
3636
import { createRenderStream } from '@testing-library/react-render-stream/pure'
3737
import { HttpResponse, http, delay } from 'msw'
38-
import { useEffect, useMemo, useState } from 'react'
38+
import { useEffect, useMemo, useRef, useState } from 'react'
3939
import type { InfiniteQueryResultFlags } from '../core/buildSelectors'
4040

4141
// Just setup a temporary in-memory counter for tests that `getIncrementedAmount`.
@@ -1586,6 +1586,80 @@ describe('hooks tests', () => {
15861586
expect(screen.queryByText('Request was aborted')).toBeNull()
15871587
})
15881588

1589+
// Based on issue #5079, which I couldn't reproduce but we might as well capture
1590+
test('useLazyQuery calling abort() multiple times does not throw an error', async () => {
1591+
const user = userEvent.setup()
1592+
1593+
// Create a fresh API instance with fetchBaseQuery and timeout, matching the user's example
1594+
const timeoutApi = createApi({
1595+
reducerPath: 'timeoutApi',
1596+
baseQuery: fetchBaseQuery({
1597+
baseUrl: 'https://example.com',
1598+
timeout: 5000,
1599+
}),
1600+
endpoints: (builder) => ({
1601+
getData: builder.query<string, void>({
1602+
query: () => ({ url: '/data/' }),
1603+
}),
1604+
}),
1605+
})
1606+
1607+
const timeoutStoreRef = setupApiStore(timeoutApi as any, undefined, {
1608+
withoutTestLifecycles: true,
1609+
})
1610+
1611+
// Set up a mock handler for the endpoint
1612+
server.use(
1613+
http.get('https://example.com/data/', async () => {
1614+
await delay(100)
1615+
return HttpResponse.json('test data')
1616+
}),
1617+
)
1618+
1619+
function Component() {
1620+
const [trigger] = timeoutApi.endpoints.getData.useLazyQuery()
1621+
const abortRef = useRef<(() => void) | undefined>(undefined)
1622+
const [errorMsg, setErrorMsg] = useState('')
1623+
1624+
const handleChange = () => {
1625+
// Abort any previous request
1626+
abortRef.current?.()
1627+
1628+
// Trigger new request
1629+
const result = trigger()
1630+
1631+
// Store abort function for next call
1632+
abortRef.current = () => {
1633+
try {
1634+
result.abort()
1635+
} catch (err: any) {
1636+
setErrorMsg(err.message)
1637+
}
1638+
}
1639+
}
1640+
1641+
return (
1642+
<div>
1643+
<input data-testid="input" onChange={handleChange} />
1644+
<div data-testid="error">{errorMsg}</div>
1645+
</div>
1646+
)
1647+
}
1648+
1649+
render(<Component />, { wrapper: timeoutStoreRef.wrapper })
1650+
1651+
const input = screen.getByTestId('input')
1652+
1653+
// Trigger multiple rapid changes that will call abort() multiple times
1654+
await user.type(input, 'abc')
1655+
1656+
// Wait a bit to ensure any errors would have been caught
1657+
await waitMs(200)
1658+
1659+
// Should not have any error messages
1660+
expect(screen.getByTestId('error').textContent).toBe('')
1661+
})
1662+
15891663
test('unwrapping the useLazyQuery trigger result does not throw on ConditionError and instead returns the aggregate error', async () => {
15901664
function User() {
15911665
const [getUser, { data, error }] =

packages/toolkit/src/query/tests/buildInitiate.test.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { setupApiStore } from '@internal/tests/utils/helpers'
22
import { createApi } from '../core'
33
import type { SubscriptionSelectors } from '../core/buildMiddleware/types'
44
import { fakeBaseQuery } from '../fakeBaseQuery'
5+
import { delay } from '@internal/utils'
56

67
let calls = 0
78
const api = createApi({
@@ -14,6 +15,14 @@ const api = createApi({
1415
return { data }
1516
},
1617
}),
18+
incrementKeep0: build.query<number, void>({
19+
async queryFn() {
20+
const data = calls++
21+
await delay(10)
22+
return { data }
23+
},
24+
keepUnusedDataFor: 0,
25+
}),
1726
failing: build.query<void, void>({
1827
async queryFn() {
1928
await Promise.resolve()
@@ -86,6 +95,19 @@ describe('calling initiate without a cache entry, with subscribe: false still re
8695
})
8796
})
8897

98+
test('successful query with keepUnusedDataFor: 0', async () => {
99+
const { store, api } = storeRef
100+
calls = 0
101+
const promise = store.dispatch(
102+
api.endpoints.incrementKeep0.initiate(undefined, { subscribe: false }),
103+
)
104+
expect(isRequestSubscribed('increment(undefined)', promise.requestId)).toBe(
105+
false,
106+
)
107+
108+
await expect(promise.unwrap()).resolves.toBe(0)
109+
})
110+
89111
test('rejected query', async () => {
90112
const { store, api } = storeRef
91113
calls = 0

packages/toolkit/src/query/tests/createApi.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,7 @@ describe('additional transformResponse behaviors', () => {
712712
meta: {
713713
request: {
714714
headers: {
715+
accept: 'application/json',
715716
'content-type': 'application/json',
716717
},
717718
},
@@ -733,7 +734,9 @@ describe('additional transformResponse behaviors', () => {
733734
value: 'success',
734735
meta: {
735736
request: {
736-
headers: {},
737+
headers: {
738+
accept: 'application/json',
739+
},
737740
},
738741
response: {
739742
headers: {

0 commit comments

Comments
 (0)