-
-
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
Add full types to useMutation's returned callback #1014
Changes from 1 commit
a42105b
36ecc80
2bb34a9
3b56f57
b377eb3
1a44f22
d7e83f0
07a430b
b47b6fc
66dd9bb
dc9fc32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -68,7 +68,7 @@ type UseQueryResult<T> = { | |||||
isLoading: false; // Query is currently loading for the first time. No data yet. | ||||||
isFetching: false; // Query is currently fetching, but might have data from an earlier request. | ||||||
isSuccess: false; // Query has data from a successful load. | ||||||
isError: false; // Query is currently in "error" state. | ||||||
isError: false; // Query is currently in an "error" state. | ||||||
|
||||||
refetch: () => void; // A function to force refetch the query | ||||||
}; | ||||||
|
@@ -91,18 +91,40 @@ The query arg is used as a cache key. Changing the query arg will tell the hook | |||||
#### Signature | ||||||
|
||||||
```ts | ||||||
type MutationHook = () => [ | ||||||
( | ||||||
arg: any | ||||||
) => { | ||||||
unwrap: () => Promise<ResultTypeFrom<D>>; | ||||||
}, | ||||||
MutationSubState<D> & RequestStatusFlags | ||||||
]; | ||||||
type UseMutation<Definition> = () => [UseMutationTrigger<Definition>, UseMutationResult<Definition>]; | ||||||
|
||||||
type UseMutationTrigger<Definition> = ( | ||||||
arg: ArgTypeFrom<Definition> | ||||||
) => Promise<{ data: ResultTypeFrom<Definition> }> & { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
could be a bit more accurate thatn There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, went with this option: ) => Promise<{ data: ResultTypeFrom<Definition> } | { error: BaseQueryError | SerializedError }> & { |
||||||
arg: { | ||||||
endpointName: string; // The name of the given endpoint for the mutation | ||||||
originalArgs: ArgTypeFrom<Definition>; // Arguments passed to the mutation | ||||||
track?: boolean; // Whether the mutation is being tracked in the store | ||||||
startedTimeStamp: number; // Timestamp for when the mutation was initiated | ||||||
}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is kinda "implementation detail". Could we mark that as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep sounds good to me |
||||||
requestId: string; // A string generated by RTK Query | ||||||
abort: () => void; // A method to cancel the mutation promise | ||||||
unwrap: () => Promise<ResultTypeFrom<Definition>>; // A method to unwrap the mutation call and provide the raw response/error | ||||||
unsubscribe: () => void; // A method to manually unsubscribe from the mutation call | ||||||
}; | ||||||
|
||||||
type UseMutationResult<Definition> = { | ||||||
data?: ResultTypeFrom<Definition>; // Returned result if present | ||||||
endpointName?: string; // The name of the given endpoint for the mutation | ||||||
error?: any; // Error result if present | ||||||
fulfilledTimestamp?: number; // Timestamp for when the mutation was completed | ||||||
isError: boolean; // Mutation is currently in an "error" state | ||||||
isLoading: boolean; // Mutation has been fired and is awaiting a response | ||||||
isSuccess: boolean; // Mutation has data from a successful call | ||||||
isUninitialized: boolean; // Mutation has not been fired yet | ||||||
originalArgs?: ArgTypeFrom<Definition>; // Arguments passed to the latest mutation call | ||||||
startedTimeStamp?: number; // Timestamp for when the latest mutation was initiated | ||||||
status: 'uninitialized' | 'pending' | 'fulfilled' | 'rejected'; // @deprecated - A string describing the mutation state | ||||||
Shrugsy marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}; | ||||||
``` | ||||||
|
||||||
- **Returns**: a tuple containing: | ||||||
- `trigger`: a function that triggers an update to the data based on the provided argument | ||||||
- `trigger`: a function that triggers an update to the data based on the provided argument. The trigger function returns a promise with the properties shown above that may be used to handle the behaviour of the promise. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your non-American behavior here is silly! 🤣 |
||||||
- `mutationState`: a query status object containing the current loading state and metadata about the request | ||||||
|
||||||
#### Description | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,10 +87,85 @@ export type MutationActionCreatorResult< | |
> | ||
> | ||
> & { | ||
arg: QueryArgFrom<D> | ||
arg: { | ||
Shrugsy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** | ||
* The name of the given endpoint for the mutation | ||
*/ | ||
endpointName: string | ||
/** | ||
* The original arguments supplied to the mutation call | ||
*/ | ||
originalArgs: QueryArgFrom<D> | ||
/** | ||
* Whether the mutation is being tracked in the store. | ||
*/ | ||
track?: boolean | ||
/** | ||
* Timestamp for when the mutation was initiated | ||
*/ | ||
startedTimeStamp: number | ||
} | ||
phryneas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** | ||
* A unique string generated for the request sequence | ||
*/ | ||
requestId: string | ||
/** | ||
* A method to cancel the mutation promise. Note that this is not intended to prevent the mutation | ||
* that was fired off from reaching the server, but only to assist in handling the response. | ||
* | ||
* Calling `abort()` prior to the promise resolving will make it throw with an error like: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually have no idea what that would do, have you tested that? I assume it would resolve with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah my wording is definitely off. When I wrote it, in my head I was specifically thinking of the unwrapped promise, but that's not what I actually wrote. I'll re-word it to make sense There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-worded to refer to 'error state' rather than saying it will 'throw' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also fwiw, it behaves exactly like you thought for the 'wrapped' promise |
||
* `{ name: 'AbortError', message: 'Aborted' }` | ||
* | ||
* @example | ||
* ```ts | ||
* const [updateUser] = useUpdateUserMutation(); | ||
* | ||
* useEffect(() => { | ||
* const promise = updateUser(id); | ||
* promise | ||
* .unwrap() | ||
* .catch((err) => { | ||
* if (err.name === 'AbortError') return; | ||
* // else handle the unexpected error | ||
* }) | ||
* | ||
* return () => { | ||
* promise.abort(); | ||
* } | ||
* }, [id, updateUser]) | ||
* ``` | ||
*/ | ||
abort(): void | ||
/** | ||
* Unwraps a mutation call to provide the raw response/error. | ||
* | ||
* @remarks | ||
* If you need to access the error or success payload immediately after a mutation, you can chain .unwrap(). | ||
* | ||
* @example | ||
* ```ts | ||
* // codeblock-meta title="Using .unwrap" | ||
* addPost({ id: 1, name: 'Example' }) | ||
* .unwrap() | ||
* .then((payload) => console.log('fulfilled', payload)) | ||
* .catch((error) => console.error('rejected', error)); | ||
* ``` | ||
* | ||
* @example | ||
* ```ts | ||
* // codeblock-meta title="Using .unwrap with async await" | ||
* try { | ||
* const payload = await addPost({ id: 1, name: 'Example' }).unwrap(); | ||
* console.log('fulfilled', payload) | ||
* } catch (error) { | ||
* console.error('rejected', error); | ||
* } | ||
* ``` | ||
*/ | ||
unwrap(): Promise<ResultTypeFrom<D>> | ||
/** | ||
* A method to manually unsubscribe from the mutation call | ||
Shrugsy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
unsubscribe(): void | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { rest } from 'msw' | |
import { | ||
actionsReducer, | ||
expectExactType, | ||
expectType, | ||
matchSequence, | ||
setupApiStore, | ||
useRenderCounter, | ||
|
@@ -44,7 +45,7 @@ const api = createApi({ | |
}, | ||
}), | ||
}), | ||
updateUser: build.mutation<any, { name: string }>({ | ||
updateUser: build.mutation<{ name: string }, { name: string }>({ | ||
query: (update) => ({ body: update }), | ||
}), | ||
getError: build.query({ | ||
|
@@ -804,6 +805,67 @@ describe('hooks tests', () => { | |
) | ||
) | ||
}) | ||
|
||
test('useMutation hook callback returns various properties to handle the result', async () => { | ||
function User() { | ||
const [updateUser] = api.endpoints.updateUser.useMutation() | ||
const [successMsg, setSuccessMsg] = React.useState('') | ||
const [errMsg, setErrMsg] = React.useState('') | ||
const [isAborted, setIsAborted] = React.useState(false) | ||
|
||
const handleClick = () => { | ||
const res = updateUser({ name: 'Banana' }) | ||
expectType<{ | ||
endpointName: string | ||
originalArgs: { name: string } | ||
track?: boolean | ||
startedTimeStamp: number | ||
}>(res.arg) | ||
expectType<string>(res.requestId) | ||
expectType<() => void>(res.abort) | ||
expectType<() => Promise<{ name: string }>>(res.unwrap) | ||
expectType<() => void>(res.unsubscribe) | ||
|
||
// abort the mutation immediately to force an error | ||
res.abort() | ||
res | ||
.unwrap() | ||
.then((result) => { | ||
expectType<{ name: string }>(result) | ||
setSuccessMsg(`Successfully updated user ${result.name}`) | ||
}) | ||
.catch((err) => { | ||
setErrMsg( | ||
`An error has occurred updating user ${res.arg.originalArgs.name}` | ||
) | ||
if (err.name === 'AbortError') { | ||
setIsAborted(true) | ||
} | ||
}) | ||
} | ||
|
||
return ( | ||
<div> | ||
<button onClick={handleClick}>Update User and abort</button> | ||
<div>{successMsg}</div> | ||
<div>{errMsg}</div> | ||
<div>{isAborted ? 'Request was aborted' : ''}</div> | ||
</div> | ||
) | ||
} | ||
|
||
render(<User />, { wrapper: storeRef.wrapper }) | ||
expect(screen.queryByText(/An error has occurred/i)).toBeNull() | ||
expect(screen.queryByText(/Successfully updated user/i)).toBeNull() | ||
expect(screen.queryByText('Request was aborted')).toBeNull() | ||
|
||
fireEvent.click( | ||
screen.getByRole('button', { name: 'Update User and abort' }) | ||
) | ||
await screen.findByText('An error has occurred updating user Banana') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Appreciate you keeping the 🍌 s around 💯 |
||
expect(screen.queryByText(/Successfully updated user/i)).toBeNull() | ||
screen.getByText('Request was aborted') | ||
}) | ||
}) | ||
|
||
describe('usePrefetch', () => { | ||
|
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.
I don't know the best way to write this, but like this it is omitting
selectFromResult
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.
I've added in a pseudo-type for
UseMutationStateOptions
, but it's definitely getting very crowded. What are your thoughts on removing theDefinition
pseudo-generic from this section completely? My thoughts are that it's already an inaccurate generic compared to the real types, and it probably clutters the explanation more than it clarifies anything.