Bug Fix: Add state.change to useField's onChange dependencies.

[jkdowdle]

First off, I love the project and appreciate being able to use it.

In my project I have what may be an unconventional form where the fields that are rendered on the screen depend on what the user selects. The form as well as all the selectable categories are visible at the same time. I found a problem when I was migrating to version 5.x in my unit tests where once a subset was 'selected' a second time a textarea onChange event no longer updated the fields value.

I was trying to debug my own code and found no problems so I started playing with the code under the hood and I found that state.change was not part of the dependencies of the onChange useCallback. I added it and my test started passing as expected.

Adding state.change as a dependency is all the pull request does, but if there is a reason that state.change should not be in the dependencies (maybe performance?) then I could work on creating a reproducible example.

[codecov]

Codecov Report

Merging #477 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #477   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines         222    222           
  Branches       57     57           
=====================================
  Hits          222    222

Impacted Files	Coverage Δ
src/useField.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22abf13...ecd5daa. Read the comment docs.

[erikras]

That is very odd. The value of state.change is only ever being set to this, which only contains the name bound via closure. Are you changing the name prop to your <Field>? We've got some name-changing tests, but maybe they're not robust enough...

[jkdowdle]

Let me take another look at my test and make sure the name is changing, it should be and I am pretty sure it is.

I did think that was odd as well because I know name is in the dependency array. I will attempt to reproduce it and take a look at those tests. Hopefully I will have something soon.

Update: The name is changing in my test.

[jkdowdle]

@erikras

Ok here is reproducible example. I had some odd behavior with Codesandbox, so here is a repo. I actually had to add more elements than I expected but it still should be easy to follow.

https://github.com/jkdowdle/react-final-form-issue

git clone https://github.com/jkdowdle/react-final-form-issue.git 
cd react-final-form-issue
yarn 
yarn test

I first tried to use react-testing-library rerender utility to change to a different field name prop, but that did work as expected. I haven't looked at the unit tests yet, but I would imagine it was similar to that?

Also notice the difference between the modified form state.

[erikras]

Published in v5.1.1.

[jkdowdle]

Thanks a ton @erikras!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.