redux-form: SET_SUBMIT_SUCCEEDED called from empty onSubmit and after stopSubmit with errors

I’m not returning a Promise from my onSubmit, instead dispatching an action to be later picked up in a saga.

The saga calls, startSubmit('formName'), and then at some point later calling stopSubmit('formName', errors) (or just stopSubmit('formName') if everything was ok).

In the case of a failure, the errors are being properly picked up and applied to the form, however, I’m seeing the @@redux-form/SET_SUBMIT_SUCCEEDED action coming through as well, which results in an inconsistent form state. The submit failed, but @@redux-form/SET_SUBMIT_SUCCEEDED is setting submitSucceeded to true even though I signalled a failed submission.

I’m trying to signal a failure, but the action sequence I’m getting is,

screen shot 2017-01-26 at 12 04 25

What’s more, I get the @@redux-form/SET_SUBMIT_SUCCEEDED action creeping in even if I do nothing at all in onSubmit, i.e. with an empty onSubmit function that does nothing and doesn’t even return a Promise I see the following action,

screen shot 2017-01-26 at 13 02 52

In terms of correctness, correct me if I’m wrong, but I feel like this action shouldn’t be being called after an empty onSubmit that doesn’t return a Promise. In that case shouldn’t nothing have happened? A Promise wasn’t returned, and startSubmit() wasn’t called so why is Redux Form flipping submitSucceeded to true?

About this issue

  • Original URL
  • State: open
  • Created 7 years ago
  • Reactions: 7
  • Comments: 41 (8 by maintainers)

Most upvoted comments

Proposal

A new function, like handleSubmit, called dispatchSubmit, that you use like this:

<form onSubmit={this.props.dispatchSubmit}>
  ...
</form>

It will check that sync validation and async validation all passes, and if and only if the form is valid, it will dispatch an action that looks something like:

{
  type: '@@redux-form/DISPATCH_SUBMIT', // 'SUBMIT' is taken already... 😕
  payload: { 
    // all your form values
  }
}

The redux-form reducer will ignore this action entirely. It will be up to all your fancy async middlewares to dispatch startSubmit(), stopSubmit(), etc. to update the form’s submitting state manually.

Would that satisfy everyone’s needs?

@erikras This looks interesting, though still feels a lot like hack and adds complexity. If there is still interest, I might get a PR going for the dispatchSubmit.

In case it’s useful, here is my workaround.
I just return a Promise, and handle resolution/rejection in the main submitFormSaga:

Action creator

export const ADDRESS_SUBMIT_REQUEST = 'ADDRESS_SUBMIT_REQUEST'
export const addressSubmitRequest = (form, values, promise) => ({
  type: ADDRESS_SUBMIT_REQUEST,
  form,
  values,
  promise
})

Form

const onSubmit = (values, dispatch, props) => new Promise((resolve, reject) => {
  dispatch(addressSubmitRequest(
    // submitFormSaga needs to know which form to submit...
    props.form,
    // ... and which values to submit the form with
    {
      ...values,
      // These are additional settings allowing me to abstract my AddressForm, nothing interesting in this scope, but still useful to think of
      id: props.addressId,
      type: props.addressType,
      saveUser: props.onSubmitSaveUser,
    },
    // Pass the "promise" (actually an object containing the resolve and reject methods)
    { resolve, reject },
  ))
})

const AddressForm = props => (
  <form 
      className={classNames('AddressForm', props.className)} 
      onSubmit={props.handleSubmit}
  >
     // ...
  </form>

export default compose(
  connect((state, props) => ({
    ...props,
  })),
  reduxForm({
    validate,
    onSubmit,
  }),
)(AddressForm)

sagas/form.js

// This saga was greatly inspired by snippets found on issues regarding RF with sagas
// See https://github.com/redux-saga/redux-saga/issues/161 for quite different but interesting solutions

export function* submitFormSaga(form, submitSaga, submitSagaArgs, promise) {
  const {resolve, reject} = promise
  // Start submitting, won't trigger setSubmitSucceeded()
  yield put(startSubmit(form))
  try {
    // Call the submitSaga, which takes care of the actual API flow etc.
    yield call(submitSaga, submitSagaArgs)
    // Allow resetting the form with a simple param
    if (submitSagaArgs && submitSagaArgs.resetForm) {
      yield put(reset(form))
    }
    // I was using this line before, but actions happened in a way I couldn't manage
    // Now resolving/rejecting the Promise will take care of dispatching stopSubmit()
    // yield put(stopSubmit(form))
    // Resolve the promise, dispatching setSubmitSucceeded()
    resolve()
  } catch (err) {
    // Handle errors depending on type
    let errors = null
    if (err instanceof SubmissionError) {
      error = err.errors
      // Same as above, resolving/rejecting the Promise will take care of dispatching stopSubmit()
      // yield put(stopSubmit(form, err.errors))
    } else {
      error = { _error: err }
      // yield put(stopSubmit(form, { _error: err }))
      logException(err)
    }
    // Finally reject the promise.
    reject(error)
  }
}

FWIW, here’s the watcher saga (which calls submitAddressForm which itself calls submitFormSaga) and the data submit saga (called inside submitFormSaga):

// Watcher
export function* watchAddressSubmit() {
  yield takeLatest(userActions.ADDRESS_SUBMIT_REQUEST, submitAddressForm)
}
// Submit handler
export function* submitAddressForm(action) {
  yield call(submitFormSaga, action.form, saveAddress,
    // saveAddress arguments
    {
      form: action.form,
      ...action.values,
    },
    // Promise passed from onSubmit dispatched action.
    action.promise,
  )
}

// Data handling saga
export function* saveAddress(data) {
  try {
    const addressable_id = yield getAddressableId(data.type)

    const address = {
      ...data,
      addressable_type: data.type,
      addressable_id,
    }
    const response = yield call(Api.user.saveAddress, address)
    // Here we can call more sub-sagas depending on the parameters we've passed
    if (data.saveUser) {
      let userData = {
        ...data,
        id: yield select(getUserId),
      }
      yield call(saveUser, userData)
    }
    yield put(userActions.addressSubmitSuccess(response))
  } catch (e) {
    yield put(userActions.addressSubmitFailure(e))
    logException(e)
  } finally {
    // Mark the generator as "done"
    return true
  }
}

In result, here’s the actions dispatched, in the expected order:

screen shot 2018-03-06 at 17 19 46

This might not be the most straightforward way of doing it, but at least :

  • I can fully manage RF Forms from my Sagas
  • I don’t have to adapt my codebase to what redux-form-actions or other packages force you to do
  • My RF form handling feature is only located in a single saga for the RF related part. Even if I had to adapt my sagas to handle form submissions + their Promises, once you get it, it’s really easy to reproduce
  • As my logic is almost totally abstracted in my sagas, I can re-use my AddressForm by passing it different props depending on the use case, without needing to implement new sagas if the level of complexity stays reasonable

I honestly don’t know enough about RF to try to implement the feature so we don’t have to use workarounds, and I’m lacking time, but if someone is willing to work with me on this, that’d be great

Here is the PR for this behavior https://github.com/erikras/redux-form/pull/4015 All existing tests are passing and changes should be backwards compatible. Would be great to hear opinions on the matter and if there are any pitfalls I am unaware of. If everything is OK in approach and implementation, create tests for added functionality and ready to go.

I’m also facing this issue. Using actions to handle async submission with redux-logic, similar to the above-mentioned redux-saga use cases.

@erikras your dispatchSubmit proposal looks like it would provide the functionality to handle the form easily.

This proposal would be perfect for my current project. Any news on this?

For all of you using redux-saga, perhaps check out this tool I just built to convert actions into a Promise for libraries like redux-form.

Trying to handle both sync and remote/async submit workflows via a single onSubmit handler, and forking the two behaviours based on the return type from that function feels like a slightly fuzzy public api. Is there any benefit to be gained by having something more explicit?

Afaiu returning a promise from this handler is quite hostile to approaches that use libs that fully abstract such workflows like redux-observable and redux-saga.

Perhaps an onSubmit for sync workflows that is sensitive to errors being thrown, and an onSubmitAsync (or whatever) for async workflows where all of the submit handling is expected to be handled remotely via action creators?

I’ve got time and a desire to work on this and prepare a PR if the maintainers think there’s some benefit to be gained here …

I feel like there’s almost 100% perfect saga support as is (and indeed redux-thunk support, since it suffers from the same issue).

I setup my form like so,

const submit = values => ({type: 'SUBMIT_FORM', values})

const withForm = reduxForm({
  onSubmit(values, dispatch) {
    dispatch(submit(values))
  },
  ...
})

and then whether the submit action creator is handled exclusively by a thunk, or triggers a saga, the approach is similar. The logic (in this case a saga) looks like,

export function* submitWorker({ values }) {
  yield put(startSubmit('myForm'))
  yield put(someApiCall(values))

  const { success } = yield race({
    success: take('API_CALL_SUCCESS'),
    failure: take('API_CALL_FAILURE'),
  })

  if (failure) {
    yield put(stopSubmit('myForm', { _error: 'Failed' }))
  } else {
    // Handle success
  }
}

So remote controlling the form like this with action creators basically almost already works 100%, and I don’t see how you’d achieve better separation of concerns. The problem being just the (fairly minor) inconsistency in the form’s state that I mentioned at the top of this thread with submitSucceeded flipped to true - since as you rightly pointed out the submit handling logic is erroneously assumed synchronous.

This feels like something that could be fixed?

So I’m another one who tried to do form submission in “redux way” (my middleware is redux-observable). I found proposed solutions hard to understand and maintain, but original one is really what we need.

Since I have just a few relatively simple forms, I ended up with rewriting code with formik and own redux integration. Integration is not really hard, and because formik uses TypeScript - it integrates better with project written in TypeScript.

I wish someday redux-form can be used with pure redux approach without pain, and replaced flow with TypeScript.

Hi Guys,

Any progress on this? I just started using redux-form and I have run into the same issue. I have found a sort of way to handle it, but I am not sure it is the best way. it looks like a little hack to me. If someone is interested it is this library: https://github.com/salsita/redux-form-actions

this takes advantage of the fact onSubmit can handle a promise as well.

I think it would be a much cleaner approach if one could define which action the form

  • triggers when a submit happens
  • the form listens on failure and success events to update it’s status

Thanks