Skip to content

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

djhi
Copy link
Collaborator

@djhi djhi commented Jul 28, 2025

Problem

The mutation hooks currently put the mutationMode and params 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

  • stories
  • tests

Additional Checks

  • The PR targets master for a bugfix or a documentation fix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • The PR includes one or several stories (if not possible, describe why)
  • The documentation is up to date

Also, please make sure to read the contributing guidelines.

Copy link
Member

@fzaninotto fzaninotto left a 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);
Copy link
Member

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);
Copy link
Member

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);
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Development

Successfully merging this pull request may close these issues.

2 participants