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

Issues with focusOnFirstError and onSubmit #3827

Open
4 tasks done
Sourcekot opened this issue Aug 14, 2023 · 2 comments
Open
4 tasks done

Issues with focusOnFirstError and onSubmit #3827

Sourcekot opened this issue Aug 14, 2023 · 2 comments

Comments

@Sourcekot
Copy link

Prerequisites

What theme are you using?

mui

Version

5.12.0

Current Behavior

Follow-up to #3757: Sadly my problems were only partially fixed and some new problems now occur:

Codesandbox: codesandbox.io/s/solitary-breeze-7ykd2f

  1. focusOnFirstError is only reached when I'm pressing submit for a second time.
  2. onSubmit is only called once, try the following in the codesandbox: Fill the field and press submit, it will show "ExtraError field filled.". Now clear the field and press submit again and the message doesn't change (should have been "ExtraError field not filled."). The console.log in the first line of onSubmit is never reached.
  3. If the first error is inside an array the error in focusOnFirstError will be "undefined". See codesandbox: codesandbox.io/s/quizzical-butterfly-zsh7v7
    Add a subentry and press submit. It will somehow close after the first submit so add a subentry again and submit again. This time you can see "undefined" in the console as the error object from the focusOnFirstError function.

One more thing: Why is the ErrorListTemplate reached every time I change something in the form? LiveValidate is not enabled so I thought this shouldn't happen.

Expected Behavior

  1. focusOnFirstError should be called at each submit
  2. onSubmit should be called on each submit
  3. focusOnFirstError should work with arrays.

Steps To Reproduce

see Current Behavior section.

Environment

- OS: Microsoft Windows 10 Pro, Version	10.0.19044 Build 19044
- Node: 16.15.0
- npm: 8.18.0

Anything else?

No response

@Sourcekot Sourcekot added bug needs triage Initial label given, to be assigned correct labels and assigned labels Aug 14, 2023
@nickgros
Copy link
Contributor

nickgros commented Aug 18, 2023

@Sourcekot There's a lot going on here, so I'll need your help refining the reproduction to figure out what bugs there are, and where you might just need to make a few changes to your code to do what you want.

  1. Are you able to use the customValidate prop instead of extraErrors? I think this would make more sense since it should run after the validator, but before onSubmit. I think this might make many of your problems go away. Docs link
  2. If you do need to use extraErrors, It's not clear to me why you're setting extraErrors in your onSubmit function, while also setting extraErrorsBlockSubmit to true--the first submit will always go through, because your extra errors don't exist yet. Can you store your formData in state, and set extraErrors when the form data changes (using onChange)? You might need to mess with programmatic validation to get everything to work in the correct order, which is why I would recommend using customValidate if possible.
  3. If the first error is inside an array the error in focusOnFirstError will be "undefined".

This definitely looks like a bug in RJSF to me!

  1. Why is the ErrorListTemplate reached every time I change something in the form?

Because it's in the body of the function component, and not wrapped in useEffect, or other lifecycle hook, the console.log statement runs every time the ErrorListTemplate re-renders. This is fundamental to how React FCs work. I'm not worried about excessive re-renders of this component, unless it's causing performance issues.

@nickgros nickgros added help wanted awaiting response validation and removed needs triage Initial label given, to be assigned correct labels and assigned labels Aug 18, 2023
@Sourcekot
Copy link
Author

Sourcekot commented Aug 21, 2023

@Sourcekot There's a lot going on here, so I'll need your help refining the reproduction to figure out what bugs there are, and where you might just need to make a few changes to your code to do what you want.

  1. Are you able to use the customValidate prop instead of extraErrors? I think this would make more sense since it should run after the validator, but before onSubmit. I think this might make many of your problems go away. Docs link
  2. If you do need to use extraErrors, It's not clear to me why you're setting extraErrors in your onSubmit function, while also setting extraErrorsBlockSubmit to true--the first submit will always go through, because your extra errors don't exist yet. Can you store your formData in state, and set extraErrors when the form data changes (using onChange)? You might need to mess with programmatic validation to get everything to work in the correct order, which is why I would recommend using customValidate if possible.

Sadly I think thats not a possibility. I've got some validation that need to fetch some information from my database. That's why I need extraErrors to do those async calls and if I remember it correctly the customValidate function can't be async. That's why I have it in my onSubmit. But maybe I'm not doing this correctly as I haven't found much about the async validation in the documentation.
In my onSubmit I'm running those async checks which are then put into the extraErrors state. In short it looks something like this:

const errors = await someAsyncValidation();
setExtraErrors(errors);

It can't be in the onChange as I don't want those validations to be done on every single change. They should only be executed if the user presses the submit button.

Regarding extraErrorsBlockSubmit: That might have been a misunderstanding on my side. As it was added in my previous issue (#3757) I thought I had to add it to fix my issue.

Currently I see two options to somehow fix my issue but both aren't good solutions:

a) I have to do my async call in the onChange which is not really an option as I don't want to call those that often or
b) keep it in onSubmit and don't use focusOnFirstError (preferable but still not great)

Is there any other way for me to set the extraErrors prior to the focusOnFirstError without calling it on every change?
I think the best solution would be for the customValidate prop to allow async functions.

EDIT: I had no time to try it yet but could I maybe use customValidate with an inner async function like this?:

const customValidate = (...) => {
  (async () => {
    my async stuff
  })();
}

Currently I was using the onSubmit like this:

const onSubmit = useCallback(async (formdata: IChangeEvent, event: React.FormEvent) => {
    my submit function including async calls
  }, [...]);
  1. Why is the ErrorListTemplate reached every time I change something in the form?

Because it's in the body of the function component, and not wrapped in useEffect, or other lifecycle hook, the console.log statement runs every time the ErrorListTemplate re-renders. This is fundamental to how React FCs work. I'm not worried about excessive re-renders of this component, unless it's causing performance issues.

But isn't it the suggested way to add the templates? In the documentation (Doc Link) it is done pretty similar to my example. Could you give me an example of how it should be done with useEffect?

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

No branches or pull requests

2 participants