-
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
feat: Hook for remote mutations #1450
Conversation
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 76d7864:
|
How about const { mutate, trigger, reset, isMutating } = useSWRMutation('/api/user', (extraArgument) => {})
// fetcher only receive extraArgument from trigger
trigger(extraArgument) trigger would have batter typescript intellisense and for some non-restful endpoints, they actually may not need the key args |
Yeah that makes the design so much simpler and I have thought about it. But I wasn't sure, and this design is mostly for consistency. |
IMO The benefit of receiving keys is that user could configure a general fetcher in context rather than writing it at every hook instance. But in useSWRMutation user cannot set a general mutate function in context (or should we allow user to do that ?). The mutate function could access the keys directly in every hook instance instead of receiving it from its arguments. |
I feel that it's still able to reuse the mutator function: const { trigger } = useSWRMutation('/api/user', post)
trigger({
name: 'yo'
}) And function post(API, data) {
return fetch(API, {
method: 'POST',
body: JSON.stringify(data)
}).then(res => res.json())
} Which is a general utility to post data. |
I am sorry. my last reply is somehow misleading. My expectation// if user already have a mutator function like this
interface User {
name: string
age: number
}
const updateUser = (param: User) => post('/api/user', param)
const { trigger } = useSWRMutation('/api/user', updateUser)
// ts error here since age is string
trigger({ name: '123' , age: '12'}) It would be batter if swr could make use of user-defined types and let typescript do type check on trigger. Possible solutionSince we would not use SWRConfig to set a general mutator function <SWRConfig
value={{
fetcher: post
}}>
</SWRConfig> The key does not have to be passed to mutator function We could improve the type of trigger by letting mutator function only receive extraArgument. In this way it doesn't need to upgrade the typescript version. |
Yeah this can maybe fix the typing problem, but my point was that this breaks the consistency in API design. |
It could do the trick. But it would require user to do some extra work interface User {
name: string
age: number
}
const updateUser = (param: User) => post('/api/user', param)
// developer need to type the mutator function explicitly so that trigger could get correct inferred type
const { trigger } = useSWRMutation('/api/user', (key: string, extraArguments: User) => updateUser(extraArguments))
trigger({ name: '123' , age: '12'}) |
A question: do you consider supporting configure onSuccess, onError, etc globally? Like what useSWR does. |
@anothertempore Yes those should be supported! |
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.
Looks great! Amazing work
* (wip) initial impl. * fix test * (wip) fix deps * initial implementation * fix linter * fix state reset * avoid reset race condition * fix race conditions * code tweaks * code tweaks * return bound mutate * apply review comments * fix tsconfig * type fixes * fix lint errors * code tweaks * fix type error * update types * inline serialization result * merge main and update argument api * add tests * fix tests * update typing * update state api * change error condition
Closes #1262.
Motivation
There are a lot of existing discussions and feature requests about doing requests with side effects on the remote side manually in SWR as well as a better mutation API design (e.g. #93, #116, #157, #550, #1082), and we do see some valid use cases for it.
As a conclusion, we need something that:
isMutating
foruseSWR
Breakings
useSWRMutation
For
revalidate
,populateCache
,optimisticData
androllbackOnError
, check https://github.com/vercel/swr/releases/tag/1.2.0 and successor releases.Unlike
useSWR
, there is no moreisValidating
andmutate
returned from this hook.trigger
During a mutation, the
isMutating
state will betrue
, and any race conditions ofuseSWR
anduseSWRMutation
will be carefully avoided.reset
To-dos
useSWR
hook with the same key. This can be another option to consider.isMutating
state are working as expected.trigger
prevents race conditions with normal GET requests.reset
.reset
andtrigger
calls.populateCache
(feat: AddpopulateCache
option tomutate
#1729).Polish event callbacks includingonDiscarded
for this mutation hook.useSWRMutation
and ignore the others.