Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix null value support in inputs #8262

Merged
merged 32 commits into from Oct 17, 2022
Merged

Fix null value support in inputs #8262

merged 32 commits into from Oct 17, 2022

Conversation

slax57
Copy link
Contributor

@slax57 slax57 commented Oct 13, 2022

Problem:

Because of https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/form/getFormInitialValues.ts#L16, null values in records get transformed into empty strings ''.
We should not change the records value like this, especially if the user does not change them in a form.

This also prevents to submit null as value.
Actually, adding a parse prop like parse={value => (value === '' ? null : value)} actually works, but only if the value is changed by the user.

Solution:

  • no longer modify the values in getFormInitialValues, and instead add a default format function to all inputs (via useInput) to change the formatted value to an empty string in that case, but not the actual (controlled) value.
  • also add a default parse function to all inputs to parse all empty strings to null, because that's what most backends expect anyway

Additional info

  • Also fixes Null default value becomes blank string #8264
  • sanitizeEmptyValues is now rendered mostly useless when using only RA input components (such as TextInput, NumberInput, ...) ; it remains useful however if you are using custom inputs that do not use useInput
  • This PR introduces a (very minor) BC break: custom inputs that do not use useInput will need to format their value to '' when they receive undefined or null by themselves, otherwise this will raise console warnings about controlled/uncontrolled inputs

@slax57 slax57 added the RFR Ready For Review label Oct 13, 2022
@slax57 slax57 added WIP Work In Progress and removed RFR Ready For Review labels Oct 14, 2022
@@ -7,7 +7,7 @@
export const sanitizeEmptyValues = (values: any, record: any = {}): any => {
const sanitizedValues = {};
Object.keys(values).forEach(key => {
if (values[key] === '') {
if (values[key] == null || values[key] === '') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you updated this function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For react-admin inputs (such as TextInput), when the field is undefined from the dataProvider:

  • if you touch the field (e.g. change it to 'foo' then back to '')
  • if your field has a defaultValue but you clear it
    => In both cases we will send "field": null, although the field was absent from what the dataProvider returned.

This is because sanitizeEmptyValues only triggers on '' but not on null, however our components now return null.

IMO in this case sanitizeEmptyValues should still remove the values from what's been sent (hence my change of the implementation is needed).

I'll add a story to play with this to the branch in any case so that you can see for yourself 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Null default value becomes blank string
2 participants