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

Reset submission error when the field is changed #5962

Merged
merged 3 commits into from
Mar 8, 2021

Conversation

alanpoulain
Copy link
Contributor

@alanpoulain alanpoulain commented Feb 26, 2021

Fixes #5938.

Use https://github.com/ignatevdev/final-form-submit-errors to reset the submission error when the field is changed.

I'm not using the SubmitErrorsSpy component but a new hook instead to make it work in the FormWithRedirect component.

I've also opened a PR in https://github.com/ignatevdev/final-form-submit-errors to add the hook there: ignatevdev/final-form-submit-errors#6.

@@ -41,7 +41,7 @@ describe('FormWithRedirect', () => {

expect(renderProp.mock.calls[1][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(2);
expect(renderProp).toHaveBeenCalledTimes(3);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using useFormState triggers a rerender.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a problem... This feature impacts the performance of all forms to allow a feature that isn't so common...

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encouraging!

@@ -51,7 +53,10 @@ const FormWithRedirect: FC<FormWithRedirectProps> = ({
initialValues,
initialValuesEqual,
keepDirtyOnReinitialize = true,
mutators = arrayMutators as any, // FIXME see https://github.com/final-form/react-final-form/issues/704 and https://github.com/microsoft/TypeScript/issues/35771
mutators = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd create a defaultMutators outside of the component, to avoid that the mutators value change at each render

*/
const useResetSubmitErrors = () => {
const { mutators } = useForm();
const { values } = useFormState({ subscription: { values: true } });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it weird that you need to call useFormState twice, especially since you've already called useForm. Maybe that's why you had to update the FormWithRedirect unit test.

I assume you could do the same with form.subscribe(), without even calling useFormState once.
Could you try that and see if it removes the extra render?

@@ -41,7 +41,7 @@ describe('FormWithRedirect', () => {

expect(renderProp.mock.calls[1][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(2);
expect(renderProp).toHaveBeenCalledTimes(3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a problem... This feature impacts the performance of all forms to allow a feature that isn't so common...

@alanpoulain
Copy link
Contributor Author

@fzaninotto is it better now?

@fzan
Copy link
Contributor

fzan commented Mar 2, 2021

sorry to jump in the discussion, but i wanted to show my appreciation to your work @alanpoulain
excellent imho

Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Thanks!

@Vgor07
Copy link

Vgor07 commented Mar 16, 2021

I am getting an error message after upgrading to a version with this change.

TypeError: form.mutators.resetSubmitErrors is not a function

    in ReactFinalForm (created by FormWithRedirect)
    in FormContextProvider (created by FormWithRedirect)
    in FormWithRedirect

I am using mutators in FormWithRedirect to implement some data manipulation logic, and the form doesn't work anymore. Should I open a bug or there is a solution?

@djhi
Copy link
Contributor

djhi commented Mar 17, 2021

If you added custom mutators, make sure you include the one we need as well. We should document that though.

Besides please don't comment on PR like that, open issues instead

@djhi
Copy link
Contributor

djhi commented Mar 17, 2021

We need array mutators and https://github.com/ignatevdev/final-form-submit-errors which has been introduced in this very PR

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

Successfully merging this pull request may close these issues.

Server validation errors not removed after field change
5 participants