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
Removes redundant dependencies in append and remove of useFieldArray #2780
Removes redundant dependencies in append and remove of useFieldArray #2780
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 2abef78:
|
hmmm actually I am going to hold this PR, I think to remove that ref object is the correct approach. |
Ok, I believe you will find a better way to eliminate this dependency, as it is not needed in |
append: React.useCallback(append, [name, errors, fields]), | ||
remove: React.useCallback(remove, [name, errors, fields]), | ||
append: React.useCallback(append, [name, errors]), | ||
remove: React.useCallback(remove, [name, errors]), |
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.
please fix remove as well with ref
.
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.
It was not used, as you see all tests are still green. Unless I am missing something?
I am not sure also where errors
dep is used in these functions? Maybe it can be removed as well?
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 are right, that was my bad!
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.
Do you want me to remove errors
dep as well. then?
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.
it's used in batchStateUpdate
.
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 will clean this update to only have name
as dep. you can leave that to me, will sort out tonight.
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 for the PR.
Co-authored-by: Bill <bluebill1049@hotmail.com>
dame lint failed, you may have to pull the latest and run |
Fixed! :) |
Fixes #2773
In
append
methodfillEmptyArray
cares just about the length of the array, so thefields
dep was not needed there at all.In
remove
dependencyfields
was not used.All tests are still green. Probably we should add some test cases, so these dependencies are not re-added to the methods in
useFieldArray
.