Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not override falsy (null) fetcher #2245

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions _internal/types.ts
Expand Up @@ -209,29 +209,29 @@ export interface SWRHook {
): SWRResponse<Data, Error>
<Data = any, Error = any, SWRKey extends Key = null>(
key: SWRKey,
fetcher: Fetcher<Data, SWRKey> | null
fetcher: Fetcher<Data, SWRKey> | null | undefined
): SWRResponse<Data, Error>
<Data = any, Error = any, SWRKey extends Key = null>(
key: SWRKey,
config: SWRConfiguration<Data, Error, Fetcher<Data, SWRKey>> | undefined
): SWRResponse<Data, Error>
<Data = any, Error = any, SWRKey extends Key = null>(
key: SWRKey,
fetcher: Fetcher<Data, SWRKey> | null,
fetcher: Fetcher<Data, SWRKey> | null | undefined,
config: SWRConfiguration<Data, Error, Fetcher<Data, SWRKey>> | undefined
): SWRResponse<Data, Error>
<Data = any, Error = any>(key: Key): SWRResponse<Data, Error>
<Data = any, Error = any>(
key: Key,
fetcher: BareFetcher<Data> | null
fetcher: BareFetcher<Data> | null | undefined
): SWRResponse<Data, Error>
<Data = any, Error = any>(
key: Key,
config: SWRConfiguration<Data, Error, BareFetcher<Data>> | undefined
): SWRResponse<Data, Error>
<Data = any, Error = any>(
key: Key,
fetcher: BareFetcher<Data> | null,
fetcher: BareFetcher<Data> | null | undefined,
config: SWRConfiguration<Data, Error, BareFetcher<Data>> | undefined
): SWRResponse<Data, Error>
}
Expand Down
1 change: 1 addition & 0 deletions _internal/utils/helper.ts
Expand Up @@ -12,6 +12,7 @@ export const UNDEFINED = (/*#__NOINLINE__*/ noop()) as undefined

export const OBJECT = Object

export const isNull = (v: any): v is null => v === null
export const isUndefined = (v: any): v is undefined => v === UNDEFINED
export const isFunction = <
T extends (...args: any[]) => any = (...args: any[]) => any
Expand Down
16 changes: 10 additions & 6 deletions _internal/utils/normalize-args.ts
@@ -1,15 +1,19 @@
import { isFunction } from './helper'
import { isFunction, isNull } from './helper'

import type { Key, Fetcher, SWRConfiguration } from '../types'

export const normalize = <KeyType = Key, Data = any>(
args:
| [KeyType]
| [KeyType, Fetcher<Data> | null]
| [KeyType, Fetcher<Data> | null | undefined]
| [KeyType, SWRConfiguration | undefined]
| [KeyType, Fetcher<Data> | null, SWRConfiguration | undefined]
): [KeyType, Fetcher<Data> | null, Partial<SWRConfiguration<Data>>] => {
return isFunction(args[1])
| [KeyType, Fetcher<Data> | null | undefined, SWRConfiguration | undefined]
): [
KeyType,
Fetcher<Data> | null | undefined,
Partial<SWRConfiguration<Data>>
] => {
return isFunction(args[1]) || isNull(args[1])
? [args[0], args[1], args[2] || {}]
: [args[0], null, (args[1] === null ? args[2] : args[1]) || {}]
: [args[0], undefined, args[1] || {}]
}
5 changes: 3 additions & 2 deletions _internal/utils/resolve-args.ts
Expand Up @@ -2,6 +2,7 @@ import { mergeConfigs } from './merge-config'
import { normalize } from './normalize-args'
import { useSWRConfig } from './use-swr-config'
import { BUILT_IN_MIDDLEWARE } from './middleware-preset'
import { isUndefined } from './helper'

// It's tricky to pass generic types as parameters, so we just directly override
// the types here.
Expand All @@ -11,7 +12,7 @@ export const withArgs = <SWRType>(hook: any) => {
const fallbackConfig = useSWRConfig()

// Normalize arguments.
const [key, fn, _config] = normalize<any, any>(args)
const [key, fn, _config] = normalize(args)

// Merge configurations.
const config = mergeConfigs(fallbackConfig, _config)
Expand All @@ -24,6 +25,6 @@ export const withArgs = <SWRType>(hook: any) => {
next = middleware[i](next)
}

return next(key, fn || config.fetcher, config)
return next(key, isUndefined(fn) ? config.fetcher : fn, config)
} as unknown as SWRType
}
4 changes: 2 additions & 2 deletions _internal/utils/with-middleware.ts
Expand Up @@ -16,9 +16,9 @@ export const withMiddleware = (
return <Data = any, Error = any>(
...args:
| [Key]
| [Key, Fetcher<Data> | null]
| [Key, Fetcher<Data> | null | undefined]
| [Key, SWRConfiguration | undefined]
| [Key, Fetcher<Data> | null, SWRConfiguration | undefined]
| [Key, Fetcher<Data> | null | undefined, SWRConfiguration | undefined]
) => {
const [key, fn, config] = normalize(args)
const uses = (config.use || []).concat(middleware)
Expand Down
4 changes: 2 additions & 2 deletions test/unit/utils.test.tsx
Expand Up @@ -11,7 +11,7 @@ describe('Utils', () => {
const opts = { revalidateOnFocus: false }

// Only the `key` argument is passed
expect(normalize(['key'])).toEqual(['key', null, {}])
expect(normalize(['key'])).toEqual(['key', undefined, {}])

// `key` and `null` as fetcher (no fetcher)
expect(normalize(['key', null])).toEqual(['key', null, {}])
Expand All @@ -20,7 +20,7 @@ describe('Utils', () => {
expect(normalize(['key', fetcher])).toEqual(['key', fetcher, {}])

// `key` and `options`
expect(normalize(['key', opts])).toEqual(['key', null, opts])
expect(normalize(['key', opts])).toEqual(['key', undefined, opts])

// `key`, `null` as fetcher, and `options`
expect(normalize(['key', null, opts])).toEqual(['key', null, opts])
Expand Down
22 changes: 22 additions & 0 deletions test/use-swr-fetcher.test.tsx
Expand Up @@ -126,4 +126,26 @@ describe('useSWR - fetcher', () => {
rerender(<Page fetcher={false} />)
screen.getByText('data:')
})

it('should be able to pass null to the fetcher even config.fetcher is provided', () => {
const key = createKey()
const fn = jest.fn()

function Page() {
const { data } = useSWR(key, null, {
fetcher: fn
})

return (
<div>
<p>data:{data}</p>
</div>
)
}

renderWithConfig(<Page />)
screen.getByText('data:')

expect(fn).not.toHaveBeenCalled()
})
})
19 changes: 19 additions & 0 deletions test/use-swr-middlewares.test.tsx
Expand Up @@ -57,6 +57,25 @@ describe('useSWR - middleware', () => {
expect(mockConsoleLog.mock.calls.length).toBe(2)
})

it('should pass null fetcher to middleware', () => {
const key = createKey()
const mockConsoleLog = jest.fn(s => s)
const loggerMiddleware: Middleware = useSWRNext => (k, fn, config) => {
mockConsoleLog(fn)
return useSWRNext(k, fn, config)
}
function Page() {
const { data } = useSWR(key, null, {
use: [loggerMiddleware]
})
return <div>hello, {data}</div>
}

renderWithConfig(<Page />)
screen.getByText('hello,')
expect(mockConsoleLog.mock.calls[0][0]).toEqual(null)
})

it('should support `use` option in context', async () => {
const key = createKey()
const mockConsoleLog = jest.fn(s => s)
Expand Down