davidkpiano / react-redux-form

Create forms easily in React with Redux.
https://davidkpiano.github.io/react-redux-form
MIT License
2.06k stars 252 forks source link

Is it possible to get all the form values in a field validator? #849

Open jcheroske opened 7 years ago

jcheroske commented 7 years ago

The signature of a field validator is basically:

value => boolean isValid

I'm wondering how difficult it would be to add the entire model as a second argument:

(value, model) => boolean isValid

This would allow me to bridge more cleanly to validatorjs. Thanks for your time.

davidkpiano commented 7 years ago

If you're doing validation based on more than just one value, it would be a form-level validator. Look here for more info: http://davidkpiano.github.io/react-redux-form/docs/guides/validation.html

Form level validators, of course, have access to all of its fields' values.

jcheroske commented 7 years ago

I did see that in the docs, but that's not what I'm asking about. If I use a form level validator, the validation error exists only at the form level and is not associated with any specific fields, as far as I know. Being able to get the whole model in field level validators has value. Specifically, it would allow the same validator from validatejs to work correctly. I think then I could add very lightweight schema support with dvr to rrf. All other validators would port easily I think.

On Mon, Jun 19, 2017 at 1:49 PM David Khourshid notifications@github.com wrote:

Closed #849 https://github.com/davidkpiano/react-redux-form/issues/849.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/davidkpiano/react-redux-form/issues/849#event-1129878427, or mute the thread https://github.com/notifications/unsubscribe-auth/ADgUyqTfK3HueONx6ygUNTWtgsIVL2Phks5sFt68gaJpZM4N9yyt .

davidkpiano commented 7 years ago

I'll mark this as an enhancement. What would an ideal API for that look like to you?

jcheroske commented 7 years ago

I think just passing in an object as the second argument would work. Keys for the current model name (user.email) and for the entire model state would be a good start. That's all I would need right now. Something like:

{
  currentModel: 'user.email'
  parentModelValue: {
    email: 'foo@bar.com',
    name: 'Jon Doe
  }
}

Have you seen mobx-form? It has really sweet schema support, and supports three different validation libs via a plugins. When you use the validatejs plugin, the schema is just so straightforward and powerful. I think there are some good ideas there that would be so nice to have in this lib.

One more question: Is the RRF context object documented anywhere? Is there an HOC that grabs it?

davidkpiano commented 7 years ago

One more question: Is the RRF context object documented anywhere? Is there an HOC that grabs it?

It's just the Redux store, there's no specific RRF context object (at least not in V1).

Also, the potential issue with passing in the entire object is that validation based on other fields needs to also be dependent on other fields changing as well, so just naively passing the parent form values is a no-go (unless we want a lot of unnecessary rerenders).

jcheroske commented 7 years ago

It's just the Redux store, there's no specific RRF context object (at least not in V1).

Hmmm. How does a Control know what Form it's embedded in? Like, when a Control uses the .email type syntax?

jcheroske commented 7 years ago

In my mind, the complete model would be passed to the validators. This would potentially cause more work to be done in the validation subsystem, but the output of that work would still be an object with the validation results as it is now (keys and boolean values). Aren't the re-renders triggered by that object changing?

Maybe there could be a flag that controls whether or not the whole model state is passed into each validator, to keep the work done by the validators at a minimum? Most folks wouldn't need to turn it on I'm guessing.

davidkpiano commented 7 years ago

Oh, for .email yes, there is a model context that is read.

Here's what I'm thinking for a potential API (v2), which will be reactive-based:

// models (function) has the signature:
// (string | string[]) => Observable<Model | Model[]>
import { models } from 'react-redux-form';

// ...

<Control.text
  model="user.confirmPass"
  validate={models(['user.pass', 'user.confirmPass'])
    .map(([ pass, confirmPass ]) => pass === confirmPass)
  }
/>

By using Observables, we can ensure that this control is only re-rendered when either the user.pass or user.confirmPass models are changed.

There will also be a similar fields function that returns an Observable.

jcheroske commented 7 years ago

I really don't want to enter Rx land just to declare form structure and behavior! In the same way that, yes, Rx gives you massive power, and you can do anything with a lib like redux-observable, most of the time you don't need it, and are better off just using async/await or promises. See redux-logic for a sweet take on a side-effect lib, that gives you the option of writing Rx if you need to, but doesn't force you into it.

I'm still not understanding why you'll get a re-render if the whole state is passed to the validator. The output of the validator is a boolean. If that boolean doesn't change, then don't re-render. But I'm kinda dense and am probably missing the obvious...

davidkpiano commented 7 years ago

I really don't want to enter Rx land just to declare form structure and behavior!

Then what would your ideal API be? IMO the one I proposed is very concise.

I'm still not understanding why you'll get a re-render if the whole state is passed to the validator

Let's say you're validating that two passwords match, user.pass and user.confirmPass and the validator that checks both of these models belongs to user.pass.

If you tell RRF (somehow) that you only want to "listen" to only pass and confirmPass, then the validator will run whenever only pass or confirmPass changes, as expected.

If you don't tell RRF and instead you pass the entire state, the validator will need to be run any time any part of the state changes, because the validation function becomes a black box that doesn't immediately know which fields it's supposed to care about.

jcheroske commented 7 years ago

Right, I get that the validator will run with every state change. But that seems different than a re-render. The validator may re-run, but if its return value (true/false) remains constant, then you don't have to re-render, do you?

davidkpiano commented 7 years ago

The validator may re-run

Right, I don't want that to happen. Validators should only run when they need to. Validation can be expensive.

jcheroske commented 7 years ago

Gotcha. What about adding a flag, so that whole-model validation could be enabled when desired?

davidkpiano commented 7 years ago

That increases the API surface area. Going back to my initial proposal, the above can easily be abstracted to something like...


import { validateWith } from 'react-redux-form'; // future API?

// ... in render

<Control.text
  model="user.pass"
  validate={validateWith('user.confirmPass',
    (pass, confirmPass) => pass === confirmPass
  )}
/>
jcheroske commented 7 years ago

I love it! In most cases, I think Rx is an implementation detail.

I think I could write something that would split up a validatejs rules string, and special-case the same validator into that structure.

Other than the model name, is anything else being passed through the context?

mathieuseguin commented 7 years ago

I'm having the same issue where I have two controls (both invalid on page load) and I need to enable the second control only once the first control becomes valid. I like the proposal and I believe it would work great for my use case too!

mathieuseguin commented 7 years ago

In the meantime I've put this hack together where I'm passing the whole state to my component then get the model from the Fieldset (because in my case I don't know the model in advance), so that I can get the value of the first field and do the comparison

import React, { Component } from 'react'
import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { Fieldset, Control } from 'react-redux-form'
import _get from 'lodash.get'

class MyComponent extends Component {
  render () {
    return (
      <Fieldset
        model=".my_nested_obj"
        ref={(fieldset) => {
          if (!this.model) {
            this.model = fieldset.model
            this.forceUpdate()
          }
        }}
      >
        <Control.text
          model=".value1"
        />

        <Control.text
          model=".value2"
          disabled={() => {
            if (!this.model) return
            const { my_nested_obj } = _get(this.props.state, this.model)
            return my_nested_obj.value1.length == 0
          }}
        />
      </Fieldset>
    )
  }
}

MyComponent.propTypes = {
  'state': PropTypes.object.isRequired
}

const mapStateToProps = (state, ownProps) => {
  return {
    'state': state
  }
}

export default connect(mapStateToProps)(MyComponent)
mathieuseguin commented 7 years ago

I was able to get a slightly better implementation using context:

import React, { Component } from 'react'
import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { Fieldset, Control } from 'react-redux-form'
import _get from 'lodash.get'

class MyComponent extends Component {
  render () {
    return (
      <Fieldset
        model=".my_nested_obj"
      >
        <Control.text
          model=".value1"
        />

        <Control.text
          model=".value2"
          disabled={() => {
            const { my_nested_obj } = _get(this.props.state, this.model)
            return my_nested_obj.value1.length == 0
          }}
        />
      </Fieldset>
    )
  }
}

MyComponent.contextTypes = {
  'model': PropTypes.any,
}

MyComponent.propTypes = {
  'state': PropTypes.object.isRequired
}

const mapStateToProps = (state, ownProps) => {
  return {
    'state': state
  }
}

export default connect(mapStateToProps)(MyComponent)

But it would be even better if the resolveModel wrapper was exposed, as requested here: https://github.com/davidkpiano/react-redux-form/issues/751

em-ka commented 6 years ago

Hi @davidkpiano, are there any plans / progress with the new validateWith API? Thanks.