[1.6.7] Side-effect in Firefox / Form.validateField method

[CugeDe]

Testing environment

• OS: Archlinux 5.0.0 x86_64 | Archlinux 5.0.1 x86_64
• NPM: v6.4.1 | v6.7.0 | v6.9.0
• node: v10.15.3 | v11.11.0
• react-advanced-form: v1.5.* | v1.6.7
• Browser: Google Chrome (working but not always) | Firefox (not working at all)

What

Current Behaviour

• Provide a Form component with a Select field component (created with createField) and using React-Select component. This select field is multiple.
• Select one of the available values
• The field is updated and renders the selected value
• Select additional values
• The field is not updated and ignores the selected values

Expected Behaviour

• The field should not ignore additional values

Investigation result

• When an additional value is selected Form.validateField is called (line 488).
  See here

Here, forceProps is always false.

On Chrome:

const fields = explicitFields || this.state.fields
let fieldProps = forceProps
      ? explicitFieldProps
      : R.path(explicitFieldProps.fieldPath, fields)

// This branch is always taken in my case
if (fieldProps) {
    console.log(fieldProps.value)
    // -> Array of two values (IMO, this means that the field values were
    // updated BEFORE the validation...)
}

fieldProps = fieldProps || explicitFieldProps

On Firefox:

const fields = explicitFields || this.state.fields
let fieldProps = forceProps
      ? explicitFieldProps
      : R.path(explicitFieldProps.fieldPath, fields)

// This branch is always taken in my case
if (fieldProps) {
    console.log(fieldProps.value)
    // -> One value (IMO, this means that the field values were not
    // updated, kinda logical because validation should allows it or not)
}


fieldProps = fieldProps || explicitFieldProps

More

I was not able to reproduce the problem on Debian unstable (my only other testing environment).

Here's a code sandbox (See here) showing the bug I have on Archlinux with Firefox.

On the sandbox, I can't even select a single value and the bug also happens when the isMulti = false.

Resolution

I don't really understand why you are using R.path(explicitFieldProps.fieldPath, fields).
I tried removing the line and it works (same as setting forceProps to true).

[kettanaito]

Hello, @CugeDe. Thanks for reporting the issue!

I don't really understand why you are using R.path(explicitFieldProps.fieldPath, fields).

There are two types of field updates that may happen:

1. Update on existing field state. This transient update grabs the latest state of the field from the form's state, and uses it as the input. It's often used to prevent concurrency issues when using a field state at the specific point of time, which might have changed before/during field update operation.
2. Update on explicit field state. This kind of updates emphasize that the update operation is bound to the current (often intermediate, not yet set to the form's state) field state, and must not attempt to resolve the latest field state since it would be obsolete at the moment of update invocation.

Validation itself may also happen in both of these ways above. For example, usually validation is performed using the first update type, and operates on the existing field state. This allows to make validation calls debounced while having synced value updates of a field while typing (otherwise a debounced validation call would execute with the obsolete state of a field, a state when the validation call was dispatched). And it may also happen using the second update type, operating on the explicit field state. This happens during reactive props resolving, where a subsribed property of a field state resolves and must be validated at the same time.

---

Regarding to your issue

1. Is it reporoducible on the example of react-select usage?
2. Which version of react-select are you using?
3. Could you please post me the implementation of your custom Select field?

Thanks.

[CugeDe]

Hello @kettanaito. Thanks for answering!

If I understand, R.path(...) is used for the first type of field update, to grab the latest state. Is that right ?
Because, in my case, when the field receives its new value in explicitFieldProps, the field's value remains unchanged because forceProps is set to false, then the grabbed value (from the latest state) is set again.

1. In this CodeSandbox, which uses you example implementation of react-select, it happens (on my distro and with firefox).

2. react-select 2.4.2

3. My implementation is the following (it is derived from yours):

import React, { Component } from 'react';
import { createField } from 'react-advanced-form';
import { Label, FormGroup, FormFeedback } from 'reactstrap';
import PropTypes from 'prop-types';
import ReactSelect from 'react-select';

const styles = {
  multiValue: (base, state) => {
    return state.data.isFixed ? { ...base, backgroundColor: 'gray' } : base;
  },
  multiValueLabel: (base, state) => {
    return state.data.isFixed ? { ...base, fontWeight: 'bold', color: 'white', paddingRight: 6 } : base;
  },
  multiValueRemove: (base, state) => {
    return state.data.isFixed ? { ...base, display: 'none' } : base;
  }
};

class Select extends Component {
  static propTypes = {
      label: PropTypes.string,
      options: PropTypes.arrayOf(
          PropTypes.shape({
              value: PropTypes.oneOfType([
                  PropTypes.string.isRequired,
                  PropTypes.number.isRequired
              ]),
              label: PropTypes.string.isRequired,
          }),
      ).isRequired,
  }

  /* Handler for "react-select" onChange event */
  handleChange = (selectedOption, { action, removedValue }) => {
      /* Dispatching "react-advanced-form" field change handler to update the field record */
      switch (action) {
        case 'remove-value':
        case 'pop-value':
          if (removedValue.isFixed) {
            return;
          }
          break;
        default:
          break;
      }

      this.props.handleFieldChange({ nextValue: selectedOption });
  }

  getFieldErrors(asyncErrors, field) {
      let errors = false;
      if (asyncErrors && asyncErrors[field] && asyncErrors[field].errors) {
          errors = asyncErrors[field].errors;
      }
      return errors;
  }

  render() {
    const { fieldProps, options, fieldState, name, label, asyncErrors } = this.props;
    var { errors } = fieldState;

    var customErrors = this.getFieldErrors(asyncErrors, name);
    if (customErrors) {
      if (!errors)
        errors = [];
      customErrors.forEach(error => {
        errors.push(error);
      });
    }

    return (
        <FormGroup row>
            { label && 
                <Label for={ name } className="col-12 align-self-center mb-0">{ label }</Label>
            }
            <div className="col-12">
                <ReactSelect {...fieldProps} isClearable={!options.some(v => !v.isFixed)} styles={styles} onChange={this.handleChange}/>
                {errors && errors.map((error, index) => (
                  <FormFeedback key={index} className="d-block">{error}</FormFeedback>
                ))}
            </div>
        </FormGroup>
    )
  }
}

export default createField({
  serialize(selectedValue) {
    if (Array.isArray(selectedValue)) {
      return selectedValue.map((element, idx) => { return element.value; });
    }
    else
      return selectedValue.value;
  },
  enforceProps({ props: { options, isMulti } }) {
    /* Declare which props to propagate from "<Select>" props to "fieldProps" */
    return {
      options,
      isMulti,
    }
  },
})(Select)

Thanks.

[kettanaito]

Thank you for a fast reply.

Yes, R.path is a utility that grabs a deep value from a nested object. It's commonly used in the library, including the usage of the first type of updates I've mentioned. At this point I think the change/validation events must be correct, and forceProps should be used in a rather exclusive scenarios.

Strangely enough, your sandbox works as expected when I test it on Firefox (66.0.2 x64) and Chrome (73.0.3683.86 x64).

To narrow down the problem, could you please assist me in this questions:

1. Does the same behavior happens with the Input field? For example, when you type and validate, do you receive up-to-date values in the UI?
2. Does this behavior happen on a simple Select component (not using react-select)?
3. Could you please explain me why do you prevent change in certain cases within your custom change handler:

  /* Handler for "react-select" onChange event */
  handleChange = (selectedOption, { action, removedValue }) => {
      /* Dispatching "react-advanced-form" field change handler to update the field record */
      switch (action) {
        case 'remove-value':
        case 'pop-value':
          if (removedValue.isFixed) {
            return;
          }
          break;
        default:
          break;
      }

      this.props.handleFieldChange({ nextValue: selectedOption });
  }

When the if (removedValue.isFixed) condition resolves it will circuit the handler function and this.props.handleFieldChange won't be executed (thus, no next value would be provided). For simplicity's sake, I would recommend you to remove that switch statement for now while we debug, and see if it affects the issue behavior.

I really doubt that there is a bug in such fundamentals (although I don't deny it entirely), respective tests seems to pass successfully. Unfortunately, I can't test it on a Linux distributive right now. Maybe it's somehow related to the environment/browser. To be honest, I'm not sure at this point what is the cause of the issue.

[BrandonGillis]

Hello, I was able to reproduce the exact same bug with the code sandbox provided by using Google Chrome 73.0 on Android 8.0. When selecting a value on my mobile, the select is not updated. I tested it with only react-select without React-advanced-forms, and it's working as expected.

EDIT :
This is a new codesandbox using default HTML5 select : https://codesandbox.io/s/mm6kzrm99

As we can see, when we try to select a value in the HTML5 select, the value is selected and then unselected automatically (visually) but if we submit the form the selected value is returned.

By the way in the BasicSelect field (html5 select), the onChange event is not called. My implementation of html5 select is probably the issue though.

[CugeDe]

Your questions

1. The same does not happen with the Input field. Everything works fine.

2. This behavior does not happen on a simple Select component, it only happens when I use react-advanced-forms with react-select.

3. In react-select, some values can be considered fixed and then not removable. The following code will just remove the button used to remove the value in the field to avoid removal, but it is only a visual prevention.

const styles = {
  multiValue: (base, state) => {
    return state.data.isFixed ? { ...base, backgroundColor: 'gray' } : base;
  },
  multiValueLabel: (base, state) => {
    return state.data.isFixed ? { ...base, fontWeight: 'bold', color: 'white', paddingRight: 6 } : base;
  },
  multiValueRemove: (base, state) => {
    return state.data.isFixed ? { ...base, display: 'none' } : base;
  }
};

Programmatically removing these fixed values is still possible. Doing it in handleChange ensures that fixed values won't be removed by discarding the removal action before updating the values with:

 this.props.handleFieldChange({ nextValue: selectedOption });

---

More

The fact that you are grabbing the value from the field to update the field value in the form makes me think that there could be an event triggered too early or too late in the 'change' event processing.

---

@BrandonGillis, got the same on your codesandbox.

[kettanaito]

Thank you both for providing more details. In case it's a concurrency issue, there is a high change it will be solved by #354, on which I'm working at the moment. Expect the next minor release somewhere in April (sorry, I don't have time to dive into it earlier).