final-form / react-final-form

🏁 High performance subscription-based form state management for React
https://final-form.org/react
MIT License
7.39k stars 482 forks source link

Warning when using material-ui radio group component #392

Open mschipperheyn opened 5 years ago

mschipperheyn commented 5 years ago

Are you submitting a bug report or a feature request?

Not sure.

What is the current behavior?

When I use a Material UI Radio Group, and don't pass type="radio" as a Field property I get a warning:

index.js:2178 Warning: Failed prop type: The prop `questions[0].questionId` is marked as required in `Questionnaire`, but its value is `undefined`.

The radio group works as expected in this scenario.

When I add the type="radio" attribute, the value is passes as undefined and the radio group stops working.

What is the expected behavior?

No warning

Sandbox Link

Data structure for this value un checked (type="radio" not present)

answerId: null
questionId: "4ff412dd-c7af-488c-8661-098cf081910d"

data structure checked

answerId: "3f787b28-190d-4879-b99c-67f7c60bb31f"
questionId: "4ff412dd-c7af-488c-8661-098cf081910d"

Code

<Field
    name={`${name}.answerId`}
    label={question.question}
    component={RadioGroupField}
    options={mapAnswers(question.answers)}
/>
class RadioGroupField extends React.PureComponent {
    render() {
        const {
            input,
            label,
            options,
            required,
            disabled,
            helperText,
            horizontal,
            meta: { touched, error, submitFailed },
            className,
        } = this.props;
        return (
            <FormControl
                required={required}
                fullWidth
                margin="normal"
                error={!!((touched || submitFailed) && error)}
            >
                <FormLabel component="label">{label}</FormLabel>
                <RadioGroup
                    {...input}
                    className={cx(className, {
                        [styles.horizontal]: horizontal,
                    })}
                >
                    {options.map(opt => (
                        <FormControlLabel
                            key={opt.value}
                            value={opt.value}
                            disabled={disabled}
                            control={<Radio />}
                            label={opt.label}
                        />
                    ))}
                </RadioGroup>
                <FormHelperText>{(touched && error) || helperText}</FormHelperText>
            </FormControl>
        );
    }
}

What's your environment?

"react-final-form": 3.6.6 "react-final-form-arrays": "1.0.6", "react": "16.6.0",

Other information

index.js:2178 Warning: You must pass `type="radio"` prop to your Field(userAnswers[1].answerId) component.
Without it we don't know how to unpack your `value` prop - "undefined".
mschipperheyn commented 5 years ago

This issue also occurs with

"react-final-form": 4.0.2
"react-final-form-arrays": "2.0.1",
Gzbox commented 5 years ago

I also encountered the same problem, I don’t know what the reason is. image

--version

"final-form": "^4.13.0",
"react-final-form": "^5.0.1",
Grsmto commented 5 years ago

From what I see in the code, this is because react-final-form expects you to handle radio/checkbox inputs in a different way than what you're doing: it expects you to have 1 field component per radio input whereas you're trying to abstract that in a RadioGroupField component. I think this is a bug in the warning logic here, we can see it in the undefined that's being thrown. The value is actually computed properly by the form.

amok commented 5 years ago

Example usage

const RadioFormField = ({
  input: { checked, value, name, onChange, ...otherInput },
  meta,
  ...other
}) => (
  <Radio
    {...other}
    name={name}
    value={value}
    checked={checked}
    onChange={onChange}
    inputProps={otherInput}
  />
);

const Group = () => (
  <FormControl component="fieldset">
    <RadioGroup>
      <FormControlLabel control={<Field type='radio' color='secondary' name='n' value='none' component={RadioFormField} />} label="None" />
      <FormControlLabel control={<Field type='radio' color='secondary' name='n' value='pending' component={RadioFormField} />} label="Pending" />
      <FormControlLabel control={<Field type='radio' color='secondary' name='n' value='approved' component={RadioFormField} />} label="Approved" />
      <FormControlLabel control={<Field type='radio' color='secondary' name='n' value='rejected' component={RadioFormField} />} label="Rejected" />
      <FormControlLabel control={<Field type='radio' color='secondary' name='n' value='expired' component={RadioFormField} />} label="Expired" />
    </RadioGroup>
  </FormControl>
)
erikras commented 5 years ago

type="radio" does a special thing of adding a checked value in fieldState.input iff the value prop === the actual value of the field in the form state. This is to make regular <input>s work.

Grsmto commented 5 years ago

Hey Erik! The logic you explained works fine, but that's not what this issue is about, this is about the warning. Removing or fixing this warning would allow both behaviour to work fine I think.

jamesmoss commented 5 years ago

I'm migrating from redux-form and this was the first issue I ran into when testing our existing custom radio fields. I feel like having to add type="radio" prop to the Field component isn't great API design - as a developer building forms I'm already passing in component={CustomRadioField}, isn't that enough to indicate it's a radio field. We don't have to pass in type for any other types and it's easy to miss off, causing this warning and in some instances the field to stop working. After studying the source I can't think of a nicer way to change the API right now though and I can see why it was done this way.

In the meantime, I have found a workaround for other people hitting this issue. If you skip passing a SyntheticEvent to the fieldState.input.onChange function and instead pass the new value directly you don't get the warning any more.

Previously your input element might have looked like this where you're passing input.onChange directly to the onChange prop:

<input
  type="radio"
  name={ input.name }
  value="bar"
  checked={ input.value === 'bar' }
  onChange={ input.onChange }
  ...etc
/>

Now we pull out the value from the SyntheticEvent and pass it instead:

<input
  type="radio"
  name={ input.name }
  value="bar"
  checked={ input.value === 'bar' }
  onChange={ (event) => input.onChange(event.target.value) }
  ...etc
/>