kettanaito / react-advanced-form

Functional reactive forms. Multi-layer validation, custom styling, field grouping, reactive props, and much more.
https://redd.gitbook.io/react-advanced-form
MIT License
217 stars 24 forks source link

Changing values with form.setValues() in an onChange() handler removes reactive props #364

Open johnpreed opened 5 years ago

johnpreed commented 5 years ago

Environment

What

Current behavior

When 2 fields that have a reactive prop (required) on the same field, and that field has an onChange handler that calls this.form.setValues(), the reactive props are removed.

Expected behavior

Calling setValues() from an onChange handler for a field upon which other fields react should not remove reactive props.

Use Case

The reason I was doing this was because I wanted to clear the values of 2 text boxes when I unchecked a checkbox on my form and both of those text boxes were required if the checkbox was checked.

How

I altered the SingleTarget reactive props example from your codebase to exhibit the problem. See below. Note that if you remove fieldThree from setValues() and only alter firstName the issue doesn't occur. Seems like it only happens if you modify more than one form field. Also, the fields that you modify with setValues() don't have to be the ones with the reactive props - they can be any other 2 fields.

import React from 'react'
import { Form } from 'react-advanced-form'
import { Input } from '@examples/fields'
import Button from '@examples/shared/Button'

export default class RxPropsSingleTarget extends React.Component {
  handleChange = () => {
    this.form.setValues({
      firstName: "",
      fieldThree: ""
    })
  }

  render() {
    return (
      <React.Fragment>
        <h1>Single target</h1>

        <Form onSubmitStart={this.props.onSubmitStart} ref={(form) => this.form = form}>
          <Input
            name="firstName"
            label="First name"
            hint="Required when `lastName` has value"
            required={({ get }) => {
              return !!get(['lastName', 'value'])
            }}
          />
          <Input name="lastName" label="Last name" onChange={this.handleChange} />
          <Input
            name="fieldThree"
            label="Some field three"
            hint="Required when `lastName` has value"
            required={({ get }) => {
              return !!get(['lastName', 'value'])
            }}
          />
          <Button>Submit</Button>
        </Form>
      </React.Fragment>
    )
  }
}
kettanaito commented 5 years ago

Thank you for reporting, @johnpreed!

Please give me some time, I will get into this issue's scenario on my local and will update shortly after with more technical details.

Meanwhile I am re-writing the state update algorithm to use patched state updates (#354). This may resolve this issue in case it's a concurrency problem (updateA contains next value of reactive prop, update B contains next value without react prop). No release deadline, there's quite a lot of work to verify after such change. I will try your scenario on that branch at first, to see if it resolves it.

kettanaito commented 5 years ago

Quick update: Testing your use case on the latest branch I can conclude that the reactive prop (required) is updated properly when typing into lastName. However, as you can see on the screenshot below, subscribed fields must not be set as "valid" once they are cleared using setValues().

I will add this scenario into existing scenario to make sure its test passes.

image