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

submitting edit form creates fields with empty string ("") value in a record instead of keeping them uninitialized #7972

Closed
SerhiyAndryeyev opened this issue Jul 13, 2022 · 10 comments

Comments

@SerhiyAndryeyev
Copy link

What you were expecting:
I have SimpleForm form with a but subtitle does not exist in fetched data. When a user saves the form without changing the "subtitle" I expect this field to be empty or undefined because the user did not modify it.

What happened instead:
When the form is submitted the data field "subtitle" gets an empty string ("") value. This breaks the integrity of our data because its field is not supposed to be assigned in this way. In the previous version (v3) of react-admin the field was not created if it is not explicitly filled by a user

Steps to reproduce:
original data loaded to form:

{
  "userId": 1,
  "id": 3,
  "title": "ea molestias quasi exercitationem repellat qui ipsa sit aut",
  "body": "et iusto sed quo iure\nvoluptatem occaecati omnis eligendi aut ad\nvoluptatem doloribus vel accusantium quis pariatur\nmolestiae porro eius odio et labore et velit aut"
}

The data after submit(pay attention to "subtitle" field added:

{
  "userId": 1,
  "id": 3,
  "title": "ea molestias quasi exercitationem repellat qui ipsa sit aut",
  "body": "et iusto sed quo iure\nvoluptatem occaecati omnis eligendi aut ad\nvoluptatem doloribus vel accusantium quis \nmolestiae porro eius odio et labore et velit aut",
  "subtitle": ""
}

Related code:
the code of the form below. This is actually from react-admin tutorial

export const PostEdit = props => (
    <Edit mutationMode="pessimistic">
        <SimpleForm>
            <TextInput disabled source="id" />
            <ReferenceInput source="userId" reference="users">
                <SelectInput optionText="name" />
            </ReferenceInput>
            <TextInput source="title" />
            <TextInput source="subtitle" />
            <TextInput multiline source="body" />
        </SimpleForm>
    </Edit>
);
  • CodeSandbox:

Other information:
Using parse method to convert empty strings to undefined does not help because empty string value ("") is also a valid value for some fields.
I tried to implement detection of dirty fields using a combination of useFormState and transform callback but it seems strange that this functionality is not supported out of the box

@SerhiyAndryeyev SerhiyAndryeyev changed the title submitting edit form creates empty string fields in a record that were not in original record submitting edit form creates fields with empty string ("") value in a record instead of leaving them empty Jul 13, 2022
@SerhiyAndryeyev SerhiyAndryeyev changed the title submitting edit form creates fields with empty string ("") value in a record instead of leaving them empty submitting edit form creates fields with empty string ("") value in a record instead of leaving them uninitialized Jul 13, 2022
@SerhiyAndryeyev SerhiyAndryeyev changed the title submitting edit form creates fields with empty string ("") value in a record instead of leaving them uninitialized submitting edit form creates fields with empty string ("") value in a record instead of keeping them uninitialized Jul 13, 2022
@djhi
Copy link
Collaborator

djhi commented Jul 13, 2022

Related to #7782?

@SerhiyAndryeyev
Copy link
Author

SerhiyAndryeyev commented Jul 13, 2022

a bit similar but not. In my case API simply doesn't have some values. But they are assigned to "" when data is sent back on submit

@slax57
Copy link
Contributor

slax57 commented Aug 22, 2022

Hi @SerhiyAndryeyev
I have been trying to reproduce your issue, but the codesandbox you provided leads to too many errors, and it is not easy for me to work with it.
Could you rather create a new codesandbox based from https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple and change the minimum amount of code to reproduce?
This would also allow us to see all request and responses made from and to the dataProvider, because the logs are enabled by default in our example.
Thanks

@martdavidson
Copy link

martdavidson commented Aug 22, 2022

I'm seeing the same, we have a NumberInput in an ArrayInput and it's now submitting an empty string instead of nothing in v4 compared to v3, which of course breaks our graphql backend. Using parse has no impact unless the input is interacted with.

If Serhiy doesn't come through with a codesandbox, I'll see if I can come up with one.

@SerhiyAndryeyev
Copy link
Author

Hi, @slax57
I modified the CommentEdit.tsx file(line 135), and also added log on update call in dataProvider. You can see that the "newField" is set to "" after submit.

https://codesandbox.io/s/react-admin-demo-issue-0hted9?file=/src/comments/CommentEdit.tsx
Screenshot 2022-08-23 133423

@dylanlt
Copy link

dylanlt commented Sep 6, 2022

Same as #8060 ?

@SerhiyAndryeyev
Copy link
Author

Same as #8060 ?

yes, the same

@slax57
Copy link
Contributor

slax57 commented Sep 8, 2022

Hi guys,
Sorry I took so long to answer.

So indeed, this is actually a breaking change from v3 to v4, due to the change of the forms library.
As explained here, react-hook-form does not handle undefined values (because it uses uncontrolled inputs by default).

native input will not return undefined, and this library doesn't support undefined as well

Native web inputs don't return undefined, only empty strings in case of an empty value.

This is what is implemented in react-admin.

If you need to workaround this, you can use the parse prop on a per-input basis, or the transform prop on the <Create>, <Edit>, or <SaveButton> components.
See the docs here: https://marmelab.com/react-admin/Upgrade.html#sanitizeemptyvalues-has-been-removed

Since we do not plan to change this, I'm going to close this issue.

@slax57 slax57 closed this as completed Sep 8, 2022
@pablos44
Copy link

pablos44 commented Sep 9, 2022

Probably there was misunderstanding of the prolem. We for instance work with mongodb which doesnt have fixed data scheme. Some values (randomly) are simply not fetched to the form. So I expected react admin to handle correctly the case when those fields stay untouched on save. So they should not be written back to DB. Unfortunatelly because the way react-hook-form was integrated it became impossible in v4.
To overcome this problem it is not suficcient to parse values you also need to rack which values are untouched (not dirty). Unfortunatelly the transform of the Edit does not have an access to the Form state (cant use useFormState), so this context need somehow to be transfered from inside the form to the ancestor Edit conrol.
I made some quick and dirty solution , but I believe it can be made in much more ellegant way inside reacr-admin edit or other related contexts

const SafeEditForm = ({ children }: { children: ReactNode }) => {
   
    const dirtyFields = useRef({});
    
    const handleTransform: TransformData = (data, options: any) => {
        const { previousData } = options;
        const updates = filter(dirtyFields.current, data);
        const final = merge({ ...previousData }, updates);
        return final;
    }

    const WrapChildren = useCallback(() => {
        const { dirtyFields: dirty } = useFormState();
        dirtyFields.current = dirty;
        return <>{children}</>
    }, [children])


    return (
        <Edit transform={handleTransform}>
            <Form>
                .....

            </Form>
        </Edit>
    )
}


I hope to see it reflected in some future version of ra

@fzaninotto
Copy link
Member

@pablos44 I think you're complaining to the wrong guy.

You're right that the behavior of our forms about default values have changed between v3 and v4. In v3, react-final-form automatically sanitized empty inputs, and it created problems for some users. in v4, react-hook-form doesn't sanitize empty inputs, and it's also not the expected behavior for all. What I mean is that there is not one-size-fits-all solution for this problem.

We try to avoid adding custom logic on top of the frameworks we use, and we embrace their philosophy as much as possible. So if react-hook-form doesn't sanitize empty values by default, so does react-admin.

If you think this is a flaw, it's probably a flaw in the underlying form framework, and it should be fixed there, not here. So I invite you to open an issue on the react-hook-form repository about your problem.

Unfortunatelly because the way react-hook-form was integrated it became impossible in v4.

If there is a way to pass the form state (and not only the form data) to handleSubmit in react-hook-form, please tell me how. React-hook-form's handleSubmit API is pretty simple and doesn't allow that.

Also, you can use a form resolver to transform the data before it's submitted based on a context object that you manipulate.

We're doing our best to communicate about this gotcha (cf #8156), but I'm afraid it can't be fixed in react-admin.

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

No branches or pull requests

7 participants