Warning when using null value on TextInput

[mediafreakch]

What you were expecting:

When my API returns null as the value for a <TextInput />, it should not lead to any warnings.

What happened instead:

When a record is returned by the API with a value of null, this leads to a warning when used on a text input:

Warning: value prop on input should not be null. Consider using an empty string to clear the component or undefined for uncontrolled components.

and

Warning: A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component.

Steps to reproduce:

1. Go to the sandbox provided
2. Open any post to edit
3. Observe console output (you may need to filter for should not be null)

Related code:

https://codesandbox.io/s/hopeful-worker-u1dxlq?file=/src/posts/PostEdit.tsx

Other information:

• Setting the defaultValue of the input, or defaultValues of the form to an empty string (''), doesn't make the warning go away
• From the migration docs it's clear that the behaviour changed in regard of how null values are treated by react-form-hook compared to react-final-form. But it's unclear to me how to mitigate this warning.

Environment

• React-admin version: 4.1.2
• Last version that did not exhibit the issue (if applicable): 3.19.x
• React version: 17

[antoinefricker]

Thanks for your for submitting this. We have been able to reproduce those warnings.

[winniecarole]

Hi I would like to solve this issue .Can I try it?

[slax57]

Hi I would like to solve this issue .Can I try it?

@winniecarole Of course! We gladly accept PRs 🙂 . Just make sure to read the contribution guidelines if you haven't already 😉

[winniecarole]

thank

[mediafreakch]

Maybe someone else should take this bug?

[slax57]

Taking some time to come back on this issue, I noticed that if your dataProvider returns undefined instead of null, then the warning disappears and everything works as expected (you don't need to add someOptionalInput: '' in your defaultValues).
In the end I'm more and more convinced this is not a bug (the warning is self-explanatory and justified).
Maybe we could document this gotcha in the migration guide though.
Hence I'll relabel this as a documentation issue.

[mediafreakch]

Just one other thought on this: undefined is not a valid JSON value, so APIs would need to omit the corresponding properties entirely. This doesn't contradict the following section in the docs, but it's not exactly plausible either:

Caution: A Data Provider should return the same shape in getList and getOne for a given resource.
https://marmelab.com/react-admin/DataProviderWriting.html

May I suggest that this is dealt with on react-admin input level (TextInput, BooleanInput, ...) instead? Another argument to support this is that there's no way for the developer to know whether react-admin/react-form-hook needs a controlled or uncontrolled input (as suggested by the warning)

[magnattic]

In the end I'm more and more convinced this is not a bug (the warning is self-explanatory and justified). Maybe we could document this gotcha in the migration guide though. Hence I'll relabel this as a documentation issue.

While I understand this reasoning, this leads to major headaches for some users though. Our API returns a lot of null values for fields where no content exists. We would have to turn all of those into empty strings manually in the dataProvider, just so that they can be used in TextInputs. Sure, it's possible, but seems like a lot of work for something that react-admin could easily help us with.

Is there any reason not to treat a null value like an empty string when null is not a valid option? (like in TextInputs)
This seems like a major convenience feature that not only helps people that upgraded from v3, but also everyone else that has null values in their API.

Maybe we could have a prop on the useInput level that makes it easy to add this? Something like treatNullAsEmpty? And set this in all components where null is no sensible option, like TextInput and BooleanInput.

If a change in the default behavior of those components is acceptable, I would consider working on a PR for this. WDYT?

[magnattic]

In the meantime we are using custom wrapper components like this to work around the problem:

import { TextInput, TextInputProps } from 'react-admin';

/**
 * Simple wrapper for the TextInput component that allows null values.
 *
 * When a null value is provided by the data provider, the TextInput component will use an empty string instead.
 * Respectively, when the value is an empty string while saving, it will be turned into null instead.
 */
export const NullableTextInput = ({
  format = (value) => value,
  parse = (input) => input,
  ...props
}: TextInputProps) => (
  <TextInput
    {...props}
    format={(value) => format(value || '')}
    parse={(input) => parse(input === '' ? null : input)}
  />
);

[alanpoulain]

For information I have added back the sanitizeEmptyValues prop in API Platform Admin (related PR: api-platform/admin#460) because having null values sent to the API is the right behavior in most cases (that's why it's even true by default, like React Admin 3).
I think it would be probably better to have it in React Admin, WDYT?
Maybe it would be necessary to have it to false by default though, to avoid BC break now that the behavior has changed.

[fzaninotto]

We're re-adding sanitizeEmptyValues in #8188 to address this issue on submission. We still need to address the format problem.