-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[UI v2] feat: Have UX related mutation callbacks be handled in the UX layer #16136
Conversation
86f5d57
to
de7c909
Compare
de7c909
to
92b7271
Compare
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.
By moving the toast as a callback to onSuccess
caught the test to mock the delete endpoint
92b7271
to
a9a1a08
Compare
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.
Nice! This makes way more sense!
server.use( | ||
http.delete("http://localhost:4200/api/variables/:id", () => { | ||
return HttpResponse.json({ status: 204 }); | ||
}), | ||
); |
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 is a general enough mock that I think adding it to ui-v2/tests/mocks/handlers.ts
might make sense.
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.
Thanks! Updated in latest push
a9a1a08
to
d549842
Compare
… layer of seperation of concern
d549842
to
a35ae10
Compare
What does this PR do and why?
This PR handles all UX related mutation callbacks to be handled on the UX layer.
useMutation
is divided into 2 separate concerns: 1️⃣ Data, 2️⃣ UX.Upon success on the data layer, it is not guaranteed that the UX related callbacks will be invoked.
This is why after calling a defined
useMutation
, there are callback options to pass, in which all UX related callbacks should be handled there.Here is a good article of how
useMutation
is architected with seperation of concerns:https://tkdodo.eu/blog/mastering-mutations-in-react-query#some-callbacks-might-not-fire
Overall,
useMutation
should be designed with 2 considerations:1️⃣ When initially creating a
mutation
, handle all data / cache related callbacks here2️⃣ When calling the
mutation
in a UX component, handle the UX updates (eg: re-routing, toasts) thereChecklist
<link to issue>
"mint.json
.Related to #15512