final-form / react-final-form

🏁 High performance subscription-based form state management for React
https://final-form.org/react
MIT License
7.39k stars 481 forks source link

After submitting the form (firing redux action) the final-form interprets response from backend as validation errors #845

Open nad182 opened 4 years ago

nad182 commented 4 years ago

Are you submitting a bug report or a feature request?

Bug report/feature request

What is the current behavior?

When form values are received on the backend, the server responds with the updated data, which react-final-form interprets as a submission error (the invalid prop from react-final-form says true, even when the form was submitted successfully). This makes it very challenging to use react-final-form with redux (not for tracking local form state, but for dispatching redux actions).

What is the expected behavior?

Do not interpret data returned from backend as validation error. Perhaps, only do so if there's an error field in the data sent from the backend.

Sandbox Link

What's your environment?

Other information

The issue was explained in more detail on stackoverflow. As a workaround, I'm doing the following (so the return would be undefined, because it's an arrow function in curly brackets):

<Form
  onSubmit={(data: SomeProps) => {
    fireAction(data)
  }}>

instead of the normal

<Form onSubmit={fireAction}>
Hyperkid123 commented 4 years ago

@nad182 this is not a bug but intended behavior. Check the docs: https://final-form.org/docs/react-final-form/types/FormProps#onsubmit.

Just don't return any function/object/promise from the onSubmit handler and you will be fine.

nad182 commented 4 years ago

Thanks for the the reply. As mentioned in my issue this is a bug/features request. I know it's by design, but that doesn't make it right (if your checked the Stackoverflow link, I'm referencing the same doc you mentioned). I'm not sure if you noticed in the issue description, but I'm dispatching a redux action onSubmit, which returns data from the backend. This data is needed to update redux store. So not returning anything is not an option.

Hyperkid123 commented 4 years ago

So not returning anything is not an option.

Can you elaborate? Why can't the callback look something like this:

const dispatch = useDispatch()
const handleSubmit = (values) => {
  dispatch(reduxAction(values)) 
}
nad182 commented 4 years ago

Because it's an unnecessary boilerplate. It really should be Form onSubmit={reduxAction}

And my thunk listens to the SUCCESS action, where it passes the data next to the reducer.

Hyperkid123 commented 4 years ago

Because it's an unnecessary boilerplate. It really should be

I am sorry that is just silly. onSubmit={reduxAction} is just a shorthand for onSubmit={(...args) => reduxAction(...args)} which is a short hand for

onSubmit={(...args) => {
  return reduxAction(...args)
}}

If you think it will save you some code evaluation in a production build, then it won't. It will be transpiled to es5 variant of the last example. I don't think that changing a completely valid design because of calling completely normal and valid JS code "unnecessary boilerplate" is something I want to really talk about. :peace_symbol:

nad182 commented 4 years ago

Are you sure you're fully reading my messages? Or are just picking stuff out of the context? πŸ™‚

Even when firing an action your way it will cause the same issue because of the return statement. The backend returns data, which is passed to redux reducer with a thunk. But I think I've said it around 4 times already, so I guess, I don't think that is something I want to repeat over and over again ✌️

czabaj commented 3 years ago

As noted here,

Rejection is reserved for communications or server exceptions.

This API might be not intuitive at first, but has its purpose and could be safely handled with something like this

onSubmit={(values) => asyncMethod(values).then(() => null, error => convertErrorResponseToReduxErrorsObjectOrNull(error))}

just create helper functions for your application and you should be good to go.

@nad182 could you please close this issue?