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

CheckboxGroupInput overrides the type of the selected value #6516

Closed
wmwart opened this issue Aug 19, 2021 · 6 comments · Fixed by #7248
Closed

CheckboxGroupInput overrides the type of the selected value #6516

wmwart opened this issue Aug 19, 2021 · 6 comments · Fixed by #7248
Labels

Comments

@wmwart
Copy link

wmwart commented Aug 19, 2021

What you were expecting:
As a backend I use a GraphQLServer. in it, the values of all fields are strongly typed. The type specified for the resource identifier is String.

For selection inside the CheckboxGroupInput, the backend returned one of options with a record {id: "54367564", ///...}. When I chose this option and sent the mutation to the server, it gave me a type error.

It turned out that instead of a string value, the input converted the identifier to a number.

What happened instead:
Expected CheckboxGroupInput to leave the identifier unchanged

Steps to reproduce:

  1. Set resource id as a string with numbers {id: "54367564", ///...}
  2. Try to save your changes
  3. Observe {ids: [54367564, ...]} entry as the value in the dataprovider

Related code:
here is a piece of code CheckboxGroupInput that does it.
In my use case, I don't understand why it is needed

 const handleCheck = useCallback(
        (event, isChecked) => {
            let newValue;
            try {
                // try to convert string value to number, e.g. '123'
                newValue = JSON.parse(event.target.value);
            } catch (e) {
                // impossible to convert value, e.g. 'abc'
                newValue = event.target.value;
            }
            if (isChecked) {
                finalFormOnChange([...(value || []), ...[newValue]]);
            } else {
                finalFormOnChange(value.filter(v => v != newValue)); // eslint-disable-line eqeqeq
            }
            finalFormOnBlur(); // HACK: See https://github.com/final-form/react-final-form/issues/365#issuecomment-515045503
        },
        [finalFormOnChange, finalFormOnBlur, value]
    );
  • React-admin version: 3.17.0
  • Last version that did not exhibit the issue (if applicable):
  • React version:
  • Browser:
  • Stack trace (in case of a JS error):
@djhi
Copy link
Collaborator

djhi commented Aug 30, 2021

Until I figure out why we have this, you can override the behavior using the parse prop (https://marmelab.com/react-admin/Inputs.html).

@fzaninotto
Copy link
Member

I'm sorry, I fail to understand how to reproduce this. Could you please elaborate and show the application code you're using?

@wmwart
Copy link
Author

wmwart commented Dec 16, 2021

Hello. This problem has surfaced again in the project.
Here is a link to a simple example https://codesandbox.io/s/dreamy-bose-61yee?file=/src/posts/PostEdit.tsx

<CheckboxGroupInput
            source="notifications"
            choices={[
              { id: "12", name: "Ray Hakt" },
              { id: "31", name: "Ann Gullar" },
              { id: "42", name: "Sean Phonee" }
            ]}
          />
{data: Object}
data: Object
id: 13
title: "Fusce massa lorem, pulvinar a posuere ut, accumsan ac nisi"
teaser: "Quam earum itaque corrupti labore quas nihil sed. Dolores sunt culpa voluptates exercitationem eveniet totam rerum. Molestias perspiciatis rem numquam accusamus."
body: "<p>Curabitur eu odio ullamcorper, pretium sem at, blandit libero. Nulla sodales facilisis libero, eu gravida tellus ultrices nec. In ut gravida mi. Vivamus finibus tortor tempus egestas lacinia. Cras eu arcu nisl. Donec pretium dolor ipsum, eget feugiat urna iaculis ut.</p> <p>Nullam lacinia accumsan diam, ac faucibus velit maximus ac. Donec eros ligula, ullamcorper sit amet varius eget, molestie nec sapien. Donec ac est non tellus convallis condimentum. Aliquam non vehicula mauris, ac rhoncus mi. Integer consequat ipsum a posuere ornare. Quisque mollis finibus libero scelerisque dapibus. </p>"
views: 222
average_note: 4
commentable: true
published_at: Sat Dec 01 2012 04:00:00 GMT+0400 (Москва, стандартное время)
tags: Array(2)
category: "lifestyle"
notifications: Array(1)
0: 12
  1. Replace numeric identifiers with strings with numbers
  2. Choose one and options
  3. Save resource
  4. Observe in the console that a number is sent instead of a string

@SergeiDubrov
Copy link

SergeiDubrov commented Jan 14, 2022

Faced the same problem. In my case id field could contain large numbers and in case of 16 digits or higher CheckboxGroupInput changed its value! For id = '81999999999999999' the control picked up value 82000000000000000, and for '8199999999999999899999' - even 8.2e+21. In case of using alphanumeric id - the problem does not appear. It is also not clear how to use 'parse' parameter to work around the problem.

@WiXSL WiXSL added bug and removed needs more info labels Jan 21, 2022
@WiXSL
Copy link
Contributor

WiXSL commented Jan 21, 2022

Reproduced. Thanks

@WiXSL
Copy link
Contributor

WiXSL commented Jan 21, 2022

I hadn't read the entire conversation.
Sure, the problem is in the handleCheck handler.

@fzaninotto,
I don't know why is trying to convert the value to number newValue = JSON.parse(event.target.value);, but if I change it to simply newValue = event.target.value;, everything seems to work as expected.
All the test pass and it seems to fix the problem.

const handleCheck = useCallback(
      (event, isChecked) => {
+         const newValue = event.target.value;
-          let newValue;
-          try {
-              // try to convert string value to number, e.g. '123'
-             newValue = JSON.parse(event.target.value);
-          } catch (e) {
-              // impossible to convert value, e.g. 'abc'
-              newValue = event.target.value;
-          }
          // ...
      },
      [finalFormOnChange, finalFormOnBlur, value]
  );

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

Successfully merging a pull request may close this issue.

5 participants