-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 values cause warning in forms #8212
Conversation
Object.keys(finalInitialValues).forEach(key => { | ||
if (finalInitialValues[key] === null) { | ||
finalInitialValues[key] = ''; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work for nested values, like author.name
for instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the fix requires a bit of recursion:
export default function getFormInitialValues(
defaultValues: DefaultValue,
record?: Partial<RaRecord>
) {
const finalInitialValues = merge(
{},
getValues(defaultValues, record),
record
);
// replace null values by empty string to avoid controlled/ uncontrolled input warning
return mapValuesDeep(finalInitialValues, value =>
value === null ? '' : value
);
}
const mapValuesDeep = (obj, callback) =>
mapValues(obj, v =>
isPlainObject(v) ? mapValuesDeep(v, callback) : callback(v)
);
But I'm afraid it could have nasty side effects. Why should we modify the inner properties of embedded objects - some of which my not be modifiable? Besides, the sanitizeEmptyValues
doesn't work for nested properties, so the empty string would be sent back to the server.
All in all, this is another corner case, which already exists today, and I don't think this PR makes it worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from @slax57 review seems good!
Closes #7782