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

Handling side effects depending on "isValid" field state #344

Open
ekozhura opened this issue Nov 29, 2017 · 13 comments
Open

Handling side effects depending on "isValid" field state #344

ekozhura opened this issue Nov 29, 2017 · 13 comments

Comments

@ekozhura
Copy link

Hey, I'm trying to run some side effects like logging when field state is changing after validation. I use mobx reactions (maybe there are better options) to observe isValid property and react on it:

  const logger = reaction(
    () => this.props.field.isValid,
    isValid => {
      const { name } = this.props.field;
      LoggerService(`${name} status: ${isValid ? LoggerStatus.ValueOk : LoggerStatus.ValueError}`);
    }
  );

Works perfectly fine but on form submit some kind of validation reset is happening:

field1 status: ok
field1 status: error
field2 status: ok
field2 status: error

Whereas I prefer to get actual field state on submit:

field1 status: error
field2 status: error

I suspect I'm doing something wrong, maybe you can suggest a better solution?

Thanks!

@foxhound87
Copy link
Owner

I created a codesandbox with your code but I don't know how to replicate your issue.

mobx reaction will execute the function only after the data expression returns a new value for the first time.

The packages has also built-in observe/intercept methods that which might be helpful

@ekozhura
Copy link
Author

ekozhura commented Dec 1, 2017

Oh wow, thanks for reply. I will try to prepare a sandbox (my bad, i should do it before creating an issue). I checked your codesandbox example and noticed something strange - submit doesn't trigger reaction at all, whereas in my case it does. Probably i'm doing something wrong.

@foxhound87
Copy link
Owner

I changed the field to email and worked (its isValid needs to change).

Previously was needed mutate the state of both password and passwordConfirm fields.

@ekozhura
Copy link
Author

ekozhura commented Dec 4, 2017

@foxhound87 i slightly modified your example and i think i've been able to reproduce my issue:
https://codesandbox.io/s/rjpy25y50p I put logger reaction inside my Input component (to make it more generic) and changed dvr validation mode to vjf and used my custom validations.

@ekozhura
Copy link
Author

ekozhura commented Dec 11, 2017

@foxhound87 Me, again. Obviously my issue was caused by resetValidation action executed inside validate method. resetValidation "resets" all observables used in computed checkValidationErrors and as a result, isValid is set to true. Then validators are executed and if some of them failed - isValid is again is set to false. But what is interesting, if validation functions are synchronous, even though resetValidation is executed but seems has no effect on checkValidationErrors, and on isValid respectively.

Note that, in my sandbox example I use async function isRequired.

I spent few hours on debugging and still get confused by this behavior.

@foxhound87
Copy link
Owner

why are you using async/await on isRequired? I think it will work by removing it.

@ekozhura
Copy link
Author

@foxhound87 You can remove async/await and just wrap the result in a Promise - you've got the same result. Not using asynchronous validators at all won't work for me because I need to use them (e.g., emails, phone numbers, VATs etc are checked on a server side).

And as I understand, resetValidation actually resets errors only if there is a serie of async actions (as in my case), while for sync validators resetValidation is completely ignored (I assume this is due to mobx optimizations). Can you explain what is the point of calling this method in a validateField action?

@foxhound87
Copy link
Owner

@ekozhura resetValidation should never be ignored, it should run every time some fields or the form is validated.

@ekozhura
Copy link
Author

@foxhound87 it should and it does run, but the value of checkValidationErrors isn't recomputed - you can check it if you debug. That's why isValid is also not recomputed when using sync validation. The fact is, resetValidation acts differently when you use either async or sync validations. The whole point of this issue is that I can't figure out why the behavior differs.

@foxhound87
Copy link
Owner

foxhound87 commented Dec 29, 2017

@ekozhura validationAsyncData is mutated by resetValidation (on every validation) and should recompute checkValidationErrors automatically. This beahvior is still not clear to me also, I need to look more, let me know if you discover something more. Thank You.

@ekozhura
Copy link
Author

@foxhound87 I'm not sure if I get it right, but I think this is somehow related to mobx optimization. Take a look at doc:

actions will batch mutations and only notify computed values and reactions after the (outer most) action has finished. This makes sure intermediate or incomplete values produced during an action are not visible to the rest of the application until the action has finished.

In case of sync validation, we have a serie of actions, which mutate the same observable. Maybe that's our case.

I tried to recreate a validation flow in a sandbox without mobx-react-form, using just mobx: https://codesandbox.io/s/ol3on1o7z9 Click few times on validate button and check console.

As you can see, if sync validator returns true and we proceed with async validator, then we have a property isInValid recomputed 2 times. Try to change a returned value of validator method and see the difference.

What do you think?

@foxhound87
Copy link
Owner

@ekozhura can you make your sandbox working?

@ekozhura
Copy link
Author

ekozhura commented Apr 4, 2019

@foxhound87 now it works, check please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants