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

React-admin changes the value of a field with typeof value === 'object'. #4277

Closed
wmwart opened this issue Jan 13, 2020 · 3 comments · Fixed by #6643
Closed

React-admin changes the value of a field with typeof value === 'object'. #4277

wmwart opened this issue Jan 13, 2020 · 3 comments · Fixed by #6643

Comments

@wmwart
Copy link

wmwart commented Jan 13, 2020

What you were expecting:
Expected to return the value passed from the server.

What happened instead:
Instead, React Admin changes the value that the server passed.

Steps to reproduce:
This issue is a copy of #4037 one since it was not completed and closed.
My server returns a field with a typeof value === 'object'.

The problem appears if there is an array in the returned object. The admin admin changes it, tries to add a parameter with the identifier values (this is not accurate) of the array elements. This is probably somehow related to the preparation of data for arrayInput, however, the value of my field is not a resource, which means that I do not need to change my data.

  1. Open codesandbox
  2. Go to CommentEdit.js
  3. Edit code :
...
<TextInput
    source="body"
    parse={v => JSON.parse(v)}
    format={v => JSON.stringify(v)}
    validate={minLength(10)}
    fullWidth={true}
    multiline={true}
/>
...
  1. Open CromeDeveloperTools -> Sources: https://l91qk8j2r7.csb.app/node_modules/ra-ui-materialui/lib/form/SimpleForm.js
  2. Set breackpoint on line 61
    props.save(finalValues, finalRedirect);
  3. In the Body Input enter a value:
{
   "FunctionalGroup": [
      {
         "uaIDref": [
            "2104"
         ],
         "_Name": "Текущие параметры",
         "_ID": "33"
      },
      {
         "uaIDref": [
            "2100"
         ],
         "_Name": "Текущие параметры пониженной точности",
         "_ID": "34"
      }
   ],
   "_Name": "Прибор 1",
   "_ID": "32"
}
  1. Press Save
  2. Look at the breackpoint, parameter finalValues

I think this problem is very important. The reliability of transmitted data of any type must remain unchanged. In addition, this prevents you from making your own components for situations more complex than in the documentation.

Environment

  • React-admin version: 3.1.2
@djhi
Copy link
Contributor

djhi commented Jan 17, 2020

Would you mind forking the sandbox and modify it so that we don't have to ? Thanks!

@wmwart
Copy link
Author

wmwart commented Jan 20, 2020

Hi, @djhi.
Good news. I found a problem. The problem is ra-data-graphql-simple in getResponseParser.js

The fact is that ra-data-graphql-simple changes the input data from the server for the needs of react-admin. However, he does this for all fields, regardless of their context. And it turns out that even if the field does not have __typename (has the graphql data type "SCALAR"), the script processes it the same way as the nested type (potentially Resource). This is definitely not necessary.

Unfortunately, making changes to your repository is problematic for me. But I'm asking for help making changes to getResponseParser.js:

import { GET_LIST, GET_MANY, GET_MANY_REFERENCE } from 'ra-core';

const sanitizeResource = data => {
    const result = Object.keys(data).reduce((acc, key) => {
        if (key.startsWith('_')) {
            return acc;
        }

        const dataKey = data[key];

        if (dataKey === null || dataKey === undefined) {
            return acc;
        }

        if (Array.isArray(dataKey)) {
            if (typeof dataKey[0] === 'object') {
                return {
                    ...acc,
                    [key]: dataKey.map(sanitizeResource),
                    [`${key}Ids`]: dataKey.map(d => d.id),
                };
            } else {
                return { ...acc, [key]: dataKey };
            }
        }

        if (typeof dataKey === 'object') {
            return {
                ...acc,
                ...(dataKey &&
                    dataKey.id && {
                        [`${key}.id`]: dataKey.id,
                    }),

                // [key]: sanitizeResource(dataKey), Additional check here

		[key]: (dataKey.__typename) ? sanitizeResource(dataKey) : dataKey, 
            };
        }

        return { ...acc, [key]: dataKey };
    }, {});

    return result;
};

export default introspectionResults => aorFetchType => response => {
    const data = response.data;

    if (
        aorFetchType === GET_LIST ||
        aorFetchType === GET_MANY ||
        aorFetchType === GET_MANY_REFERENCE
    ) {
        return {
            data: response.data.items.map(sanitizeResource),
            total: response.data.total.count,
        };
    }

    return { data: sanitizeResource(data.data) };
};

This change will solve my problem. But I am concerned about changing data from the server before this data gets into the component. This is a potentially dangerous practice, as I, as a developer, do not expect this behavior. I think that all changes to the source data should occur in the component that uses them. What do you think about this?

@fzaninotto
Copy link
Member

Just putting @wmwart's suggestion here as a diff:

// in packages/ra-data-graphql-simple/src/getResponseParser.js
@@ -31,7 +31,7 @@ const sanitizeResource = data => {
                     dataKey.id && {
                         [`${key}.id`]: dataKey.id,
                     }),
-                [key]: sanitizeResource(dataKey),
+                [key]: (dataKey.__typename) ? sanitizeResource(dataKey) : dataKey, 
             };
         }

@djhi does that make the issue clearer?

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

Successfully merging a pull request may close this issue.

3 participants