-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 cache types #1961
Fix cache types #1961
Conversation
isValidating?: boolean | ||
}) => void | ||
export type StateUpdateCallback<Data = any, Error = any> = ( | ||
state: State<Data, Error> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reuses the State
type, which will add isLoading
which was missing in the original type definition.
Lines 167 to 172 in 09d80c5
export type State<Data, Error> = { | |
data?: Data | |
error?: Error | |
isValidating?: boolean | |
isLoading?: boolean | |
} |
|
||
export interface Cache<Data = any> { | ||
get(key: Key): Data | null | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed null
as it seemed like it's no longer being used.
export interface Cache<Data = any, Error = any> { | ||
get(key: Key): State<Data, Error> | undefined | ||
set(key: Key, value: State<Data, Error>): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From release notes:
So you will have to do the following change to your code,
get
:- cache.get(key) + cache.get(key)?.dataAnd
set
:- cache.set(key, data) + cache.set(key, { ...cache.get(key), data })
@@ -442,7 +442,8 @@ export const useSWRHandler = <Data = any, Error = any>( | |||
mergeObjects( | |||
{ | |||
error: state.error, | |||
isValidating: state.isValidating | |||
isValidating: state.isValidating, | |||
isLoading: state.isLoading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLoading
was not being copied over originally, but I believe it should be.
@@ -106,13 +107,9 @@ export const createCacheHelper = <Data = any, ExtendedInfo = {}>( | |||
) => | |||
[ | |||
// Getter | |||
() => cache.get(key) || {}, | |||
() => (cache.get(key) || {}) as State<Data, any> & Partial<ExtendedInfo>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was necessary to make ExtendedInfo
work:
Lines 67 to 77 in 09d80c5
const [get, set] = createCacheHelper< | |
Data, | |
{ | |
// We use cache to pass extra info (context) to fetcher so it can be globally | |
// shared. The key of the context data is based on the first page key. | |
$ctx: [boolean] | [boolean, Data[] | undefined] | |
// Page size is also cached to share the page data between hooks with the | |
// same key. | |
$len: number | |
} | |
>(cache, infiniteKey) |
I couldn't figure out a way to do it without type assertion.
I did notice that this line below had a type error (Type 'Data' is not assignable to type 'Data[]'
), but I couldn't figure out how to resolve this. This line already had a different type error even before this PR.
Line 147 in 09d80c5
!config.compare(originalData[i], pageData)) |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1dca126:
|
@@ -144,7 +144,7 @@ export type MutatorCallback<Data = any> = ( | |||
|
|||
export type MutatorOptions<Data = any> = { | |||
revalidate?: boolean | |||
populateCache?: boolean | ((result: any, currentData: Data) => Data) | |||
populateCache?: boolean | ((result: any, currentData?: Data) => Data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a suggested fix for this issue in src/utils/mutate.ts
: #1961 (comment)
With the 2.0.0 update (#1863), each value in
cache
now holds the current states.Type changes
State
type whenever necessarynull
from a list of possible values of the cachecreateCacheHelper
to be compatible withinfinite/index.ts
useSWRConfig
.MutatorOptions['populateCache']
to allowundefined
forcurrentData
on function argument.Non-type changes
use-swr.ts
:isLoading
was not being copied over inonStateUpdate
. This became obvious as I saw that theState
type containedisLoading
.