gcanti / tcomb-form

Forms library for react
https://gcanti.github.io/tcomb-form
MIT License
1.16k stars 136 forks source link

Errors carry over between forms #352

Closed volkanunsal closed 7 years ago

volkanunsal commented 7 years ago

Version

0.9.9

Expected behaviour

Given I have a form that has validation errors on a field named foo, I switch to a different form that has a field with the same name, there should be no error on the field on the new form because it's a new form that is not validated yet.

Actual behaviour

The fields with the same name on the new form do contain error messages. This is likely because the Component, which contains the hasError state, is shared between form components. Perhaps the best solution would be to clone the components that getFormComponent returns.

gcanti commented 7 years ago

Actually I consider this a feature rather than a bug. I assume that the 2 forms are kind of a union so seems a desirable behaviour. Moreover, having shared components is a sweet perf optimisation provided by react, I'd prefer to keep it. Maybe we can find a workaround. What's your use case? I mean, why your forms have the same fields but are not related?

volkanunsal commented 7 years ago

@gcanti I see what you're saying, and I agree with you. The forms are sign-in and sign-up forms, so they're kinda related, but the clearing the error messages on a new form would be desirable for my users, I think. Maybe I can namespace the type structs like this:

const SignUpFormT = t.struct({
  sign_up: t.struct({
    name: t.String,
    email: Email,
    password: Password,
    password_confirmation: Password
  })
})

const SignInFormT = t.struct({
  sign_in: t.struct({
    email: Email,
    password: Password
  })
})
volkanunsal commented 7 years ago

@gcanti Yup, that seems to work. The only problem now is that path array seems to be always empty. I'm not sure why this is...

options={{
   error: getValidationErrorMessage(value, path, context) {
       // => path is always an empty array
   }
}}
gcanti commented 7 years ago

@volkanunsal that is because now you are nesting

const options = {
  error(value, path, context) { // <= this is executed for the "root" form, path = []
    console.log('root', value, path, context)
  },
  fields: {
    sign_up: {
      error(value, path, context) { // <= this is executed for the nested sign_up type, path = ["sign_up"]
        console.log('sign_up', value, path, context)
      }
    }
  }
}
volkanunsal commented 7 years ago

@gcanti Ah, that makes sense. Thanks for pointing that out.