Skip to content
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

Field validation not triggered after reset #2838

Closed
bbenezech opened this issue Apr 20, 2017 · 18 comments
Closed

Field validation not triggered after reset #2838

bbenezech opened this issue Apr 20, 2017 · 18 comments

Comments

@bbenezech
Copy link
Contributor

Very easy to reproduce:

https://codesandbox.io/s/o22j1lOZB

Modify age, then reset, then modify age, then submit.
Field validation on Username and Email are bypassed.
UPDATE_SYNC_ERRORS is not triggered after reset like it is when a validated field is dirty.

It looks like the conditions updateSyncErrorsIfNeeded are incorrects.

This is a major issue IMO. I have no easy workaround.
@erikras I know you must be terribly busy but can you have a look?

@VdeVentura
Copy link

Hello there @bbenezech!

Where did u got that sandbox from ?

I certainly can reproduce the behavior on it, but I can't do it on the codesanbox provided in the docs:

https://codesandbox.io/s/PNQYw1kVy

perhaps someone screwed up while setting it up ?

@bbenezech
Copy link
Contributor Author

bbenezech commented Apr 24, 2017

@irvingv8 The only difference is the absence of validation on age.
If the modified field is validated, it works, if not, it doesn't. Sorry I should have made that clearer.

I looked at the conditionals in updateSyncErrorsIfNeeded, not sure I can fix it, I have no clear idea what is going on here.

@VdeVentura
Copy link

Ok! I Believe I fixed it! I'll submit a pull request, will be my first contribution to OSS <3! hopefully I did it well, even though this was an easy one

@VdeVentura
Copy link

@erikras is there any guide/steps on how to contribute to redux-forms? It was little bit hard for me to get to test what i modified, I find the rebuild examples script, but it was going over all of the examples taking a LOT of time to rebuild each of them and I just wanted to test a single one.

I ended up deleting all the other examples, running the script and testing...

Maybe the script could take the name of a example to rebuild ?

@timhwang21
Copy link
Contributor

I actually ran into this exact issue myself a while back. Really appreciate the PR, and I'm willing to help get this moved along!

You probably want to add a test here. Specifically, something like it should not erase sync errors if they exist. The test structure is pretty self-explanatory.

One question I have: I'm wondering about the order of things happening.

  1. Validation seems to happen when new props are received. This function checks to see if values are deeply equal, and skips validation if so.
  2. Your PR seems to be simultaneously setting the values to initialValues, and syncErrors to the existing syncErrors. Wouldn't these errors be outdated, so to speak?
  3. The issue is only happening when "Age" is the only field populated when the form is reset. In other words, validation is being run after resetting when "Name" or "Email" field values change upon reset. It's not that errors aren't being properly persisted.

It seems the correct fix would lie in making either updateSyncErrorsIfNeeded or validateIfNeeded in the main file more robust.

@VdeVentura
Copy link

VdeVentura commented Apr 24, 2017

Ty for the guidance @timhwang21, now that you mention it, it totally makes sense, I went straight to the fact that syncErrors was out of the state right after the reset, but you are right, there is something more to it as it only happens when the non validated field is the one populated.

I'll take another shot at it in a couple of hours and see if I can get to culprit of the trouble.

And about:

Your PR seems to be simultaneously setting the values to initialValues, and syncErrors to the existing syncErrors. Wouldn't these errors be outdated, so to speak?

values -> initialValues was already there.
And sorry but i didn't understand the "Wouldn't these errors be outdated, so to speak?", problem I found was that after the reset syncErrors wasn't in state anymore, so I'm not setting syncErrors to the existing ones (cause they don't exist), but I might be wrong it's my first time looking at the core.

@timhwang21
Copy link
Contributor

Sure, what I meant is that values and errors are being populated at the same time in reset. Validation happens on the new values AFTER reset. The errors you are propagating are the errors from BEFORE reset. Hence any new errors caused by the reset won't be saved. Though, in the edge case at play here, if the reset causes errors then set validation would probably be called anyways. So it might work but strikes me as an indirect solution.

There was a PS from the last release that changes accessing errors from props to a property set on this, I had to stop digging but it seems this may be a regression due to that fix.

@erikras
Copy link
Member

erikras commented May 4, 2017

Good news, guys. I found the bug. This was a tricky one.

@VdeVentura
Copy link

Would be great If you could share some details, I'd like to get started helping people on this repo, sorry for the Pull Request, It was my first time looking at the core. Is there any way I can contact you if I have any doubts about it ?

@erikras
Copy link
Member

erikras commented May 9, 2017

Published fix as v6.7.0.

@irvingv8 Twitter or email (see package.json)

@callmeberzerker
Copy link

This is still not working, no action for UPDATE_SYNC_ERROR is being dispatched & no validation occurs.

@arman-mukatov
Copy link

arman-mukatov commented Jun 13, 2017

I confirm, the problem is not solved yet, after the reset, verification is not carried out..

p.s. v6.8.0

@VdeVentura
Copy link

@arman-mukatov could you provide a codesanbox sample to test ? I can no longer reproduce this issue with 6.8.0

@HiroAgustin
Copy link

I have this same problem using version 7.0.3

@danielrob
Copy link
Collaborator

I won't re-open this bug because the original reproduction sandbox + repro steps do not work against the same sandbox upgraded to 7.0.3. If you are still experiencing an issue like this, please create a new formal issue report.

@ashishlal91
Copy link

ashishlal91 commented Mar 22, 2018

I am having same problem... validation does not work after reset form...
how do I get out of this rid...

@h0wXD
Copy link

h0wXD commented Apr 16, 2018

Same problem happening here. We have an edit mode and read only mode and have an issue with resetting the form.
We have two different behaviors happening
Behavior 1 (CORRECT): Enter edit mode, update any form value, leave edit mode (reset() is called). formSyncErrors contains all errors of the wrong fields.
Behavior 2 (WRONG): Enter edit mode, don't update any form value, leave edit mode (reset() is called).
formSyncErrors contains no fields, but before reset call it did, so it's being reset somewhere.

for now we found a workaround:
//reset();
initialize(originalData);

This is working, since initialize will trigger the validation UPDATE_SYNC_ERRORS message.

p.s. may require enableReinitialize: true, keepDirtyOnReinitialize: true

@lock
Copy link

lock bot commented Apr 16, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants