Conditional field level validation not working as expected

[nealoke]

Are you submitting a bug report or a feature request?

Bug

What is the current behavior?

1. Open codesandbox
2. Click on username field but don't fill anything in (so the error shows up)
3. Fill in e-mail
4. fill in age (above 18)
5. Toggle required by clicking the button above the form
6. Try to submit the form

What is the expected behavior?

Expected is that after toggling the required off, the field that currently has the error (the username) should update itself and be valid as the validate prop now returns undefined.

Now it is required to touch the field again which is not a good user experience and in my case breaks the possibility to submit the form (see other information)

Sandbox Link

(I've used the codesandbox example of Field-Level Validation)
https://codesandbox.io/s/8ZwVQ2Zr

What's your environment?

(unchanged from codesandbox)
react 15.5.3
react-dom 15.5.3
react-redux 5.0.4
redux 3.6.0
redux-form 6.6.3
redux-form-website-template 0.0.41

Other information

This is needed when performing field level validation which is based on other factors. In my form (other project) the field also get's an overlay which is semitransparent (but blocks the user from touching the field).

The error is keeping them from submitting because the error persists even though the field is not mandatory anymore.

[nealoke]

Just as a visualization how it goes on my application. When one of the toggle switches is active (aka has the blue color) the field needs to be required.

The first error is as it should work, then when switching off both toggle switches the error persists which is unexpected. Then after toggeling one of the switches back on (and thus expecting the error) it removes the error (as you can see in the redux devtools)

@@redux-form/UPDATE_SYNC_ERRORS that is triggerd and causing the reverse of the expected behaviour

[nealoke]

I guess this could be solved by adjusting the createField file

componentWillReceiveProps(nextProps) {
	if (this.props.name !== nextProps.name) {
	// unregister old name
	this.context._reduxForm.unregister(this.name)
	// register new name
	this.context._reduxForm.register(
		prefixName(this.context, nextProps.name),
		'Field'
		)
	}
	
	// Check if validate changed (?)
	// This is how I would solve it but i'm not so good at js
	// as you guys so don't shoot me :)
	if (this.props.validate !== nextProps.validate) {
		// Update the validate in redux
	}
}

[gustavohenke]

To me, it seems like a legit bug.
Would you like to try fixing this? Feel free to go ahead and send us a PR 😉 If @erikras or I find a better way to fix it then the way you suggested, we can discuss in the PR.

[nealoke]

@gustavohenke I would love to help you guys fix this but i'm not so experienced in contributing to github repos. What are the steps I need to take? :)

I guess it is

• Fork repo
• Adjust code
• Test
• Add pull request

Let me know if I miss something :)

[gustavohenke]

Yeah, that's pretty much it.
But you could perhaps go TDD and write small passing test, then write small code that passes test, and keep looping and refactoring until your code is done 😉

[nealoke]

@gustavohenke I would love to do TDD but don't have any experience with testing (yet). I'll do my best though ;)

[erikras]

Fix published in v7.0.0.

[avocadowastaken]

So sad that this change killed descriptive syntax like this

<div>
  <Field name="username" validate={x => !isValidUsername(x) && "Please enter username"}/>
  <Field name="password" validate={x => !isValidPassword(x) && "Please enter password"}/>
  
  /* another 20 fields ... */
</div>

[duro]

@umidbekkarimov I too am sad. I'm still trying to figure out how to allow for error message definition in line to the field. cc: @erikras

I reported that this kind of approach broke in #3288 but was told it's a feature. Bummer.

[danielrob]

@umidbekkarimov you've caught me dead in my tracks... and is a different issue in my mind to what @duro is raising (which uses an higher order function) and to what I had read/understood in any of the reports on this so far. Unfortunately though... you might be right... This is pretty sad...

I think this at the very least requires another update to the docs, because it means that you can never inline specify your validate function regardless of whether it's an HOC or not...

[PavelPolyakov]

@danielrob @erikras
but do we really want this to happen? this removes very useful feature of the inline declaration. and in javascript we love this inline stuff, aren't we?

I think this should be rethought.

separating the validation definition from the field looks not nice and actually makes it harder to maintain your redux-forms.

[danielrob]

@PavelPolyakov I've over extended my day job on this one :D. I would say that being able to change the validation function seems like a very desireable feature (the point of this change). Perhaps you can put forward some ideas/a PR on some possible ways to have the best of both worlds?

[avocadowastaken]

@duro @PavelPolyakov Oh, I'm glad that I'm not the only one here!

Here gist with workaround that i used to migrate to redux-form@7.

Perhaps you can put forward some ideas/a PR on some possible ways to have the best of both worlds?

I have some thoughts on this 🤔.

[nealoke]

Sorry to see that you guys don't like the change but to me it is still preferred. Not only because of my use case but also because I think it is way more DRY to create a separate function. Especially when you will need it in the future you can simply import it again. Just my 2 cents.

Of course a workaround to have both is preferred 👍

[timhwang21]

I agree that creating separate functions is the better approach, but this change also means that arrays of rules have to be defined separately. To me that's where it crosses the line between DRY and unwieldy 😄

For what it's worth, we've solved this exact problem by sidestepping the need to change rules entirely. We did this by making our rules composable, and defining higher-order "conditional" rules. For example:

// generic higher order rule to apply validation conditionally
const applyIf = condition => rule => (value, allValues) => 
  condition(value, allValues) && rule(value, allValues);

// some validation
const hatesBob = (value, allValues) => 
  value === 'Bob' && 'Go away, Bob';
}

// will error on name === 'Bob' if blockBob (maybe some toggle switch) is true
// the validation function itself doesn't need to change!
<Field
  name="name"
  component="input"
  validation={applyIf((value, allValues) => allValues.blockBob === true)(hatesBob)}
/>
<Field
  name="blockBob"
  component="toggle" // just run with it here...
/>

[danielrob]

@timhwang21 very nice! Finally I don't think this does break arrays, deepEquals caters for e.g. [1,2,3] being equal to [1,2,3] and the submit validation example in the docs is not broken and uses an array.

[timhwang21]

Ah I shouldn't have spoken before looking into the code more (still on 6.6.3) but I saw the linked PR added a shallow equality check, my bad!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.