-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Make all mutations react to their declaration time options changes #10857
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
Conversation
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.
1% code, 99% stories and tests, we're good ;)
]; | ||
const dataProvider = { | ||
getList: (resource, params) => { | ||
console.log('getList', resource, params); |
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.
you should not leave the console.log here (same in delete and in other hook tests)
]; | ||
const dataProvider = { | ||
getList: (resource, params) => { | ||
console.log('getList', resource, params); |
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 think you should remove these logs and the console.log mocking on the test. These may have been useful during development, but now the story is explicit enough (and testable enough) not to rely on logs.
Besides, the test aren't reverting the mock on console.log.
]; | ||
const defaultDataProvider = { | ||
getList: (resource, params) => { | ||
console.log('getList', resource, params); |
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.
same, console.log is useless in tests
… may pass invalid parameters to the mutation ## Problem After #10857, when passing parameters at declaration time to a mutation hook in `optimistic` or `undoable` mode, those params may be modified by the side effects before the actual mutation is called. For instance, if you pass the `selectedIds` from the `ListContext` to a `useUpdateMany` hook and use the `useUnselectAll` hook to reset the `selectedIds` in the `onSuccess` side effect, then the actual mutation will be called with an empty array of ids. ## Solution Keep the params in an additional ref that is freezed once the `mutate` callback is called.
Problem
The mutation hooks currently put the
mutationMode
andparams
they receive at declaration time in a ref but never update them from their props.Solution
Add effects that sync the refs with the props.
How To Test
Additional Checks
master
for a bugfix or a documentation fix, ornext
for a featureAlso, please make sure to read the contributing guidelines.