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

ReferenceInput with allowEmpty has emptyValue '', not null #3087

Closed
fvieira opened this issue Apr 2, 2019 · 9 comments
Closed

ReferenceInput with allowEmpty has emptyValue '', not null #3087

fvieira opened this issue Apr 2, 2019 · 9 comments
Labels

Comments

@fvieira
Copy link

fvieira commented Apr 2, 2019

What you were expecting:
Using any of these:

<ReferenceInput source="resourceId" reference="resource" resettable>
  <SelectInput optionText="name" />
</ReferenceInput>
<ReferenceInput source="resourceId" reference="resource" resettable allowEmpty>
  <SelectInput optionText="name" />
</ReferenceInput>
<ReferenceInput source="resourceId" reference="resource" resettable allowEmpty emptyValue={null}>
  <SelectInput optionText="name" />
</ReferenceInput>

should result in the value being null when the select is empty, as per the documentation:
https://marmelab.com/react-admin/Inputs.html#referenceinput

What happened instead:
The value is an empty string '', which crashes as the dataProvider is (correctly) expecting either a integer or null.

Environment

  • React-admin version: 2.8.4
  • React version: 16.8.5
  • Browser: Chrome
@fzaninotto
Copy link
Member

Hi @fvieira,

There is no good value for the empty choice that works in all cases. Whether you need a number or a string, you'll want different default values. So ReferenceInput chose one (the empty string) which is the most common, and lets you change it when you need. You can choose the value of the empty choice with the emptyValue prop, as explained in the documentation.

@fvieira
Copy link
Author

fvieira commented Apr 3, 2019

Hi @fzaninotto,

I totally agree with what you said, but there's still two gotchas:

  • If the default empty value of ReferenceInput is the empty string, then the documentation is wrong, as it says it is null and even shows the emptyValue being used with an empty string (which wouldn't make sense were it the default).
  • The emptyValue prop would solve my problem, the problem is it doesn't seem to work. Check my third code snippet above, I explicitly pass null to the emptyValue prop and I still get an empty string back...

@fzaninotto fzaninotto added the bug label Apr 3, 2019
@fzaninotto
Copy link
Member

Then it means that you're right and I'm wrong. I'm reopening this and marking it as a bug. Thanks for the report!

@fvieira
Copy link
Author

fvieira commented Apr 3, 2019

Well, can't we both be right? Your arguments were on spot! ;)
Just don't forget to actually reopen the ticket, cause you added the bug label but the ticket is still closed...

@fzaninotto fzaninotto reopened this Apr 4, 2019
@yitzgold
Copy link

yitzgold commented Aug 7, 2019

I'm having the same issue, any update?

@macrozone
Copy link
Contributor

macrozone commented Sep 2, 2019

even with emptyValue={null} it crashes the dataprovider, in my case https://github.com/Weakky/ra-data-opencrud/

tested with react-admin 3.0.0-alpha.4

edit: sorry, #3087 (comment) already mentioned that this is not fixed.

@macrozone
Copy link
Contributor

I debugged that a bit and it seems that its not only a matter of emptyValue.

The referenceField will use useGetMany with ids=[""] when it has no value, this is called from useReference({id: ""}). That is semantically wrong. When no item is selected, it should not try to fetch a referenced record. A SelectField should know whether an item of its options is selected or not. At the moment it just says: 'yes, i got a value: ""'.

I don't think that this is solved properly with emptyValue, at least not for SelectFields. These should now their option and know whether a value is selected or not. Encoding that with an empty string is probably not a good idea. null would be semantically better, but still not ideal. It should have some property "hasNoValue" or so to indicated that it has nothing selected

@macrozone
Copy link
Contributor

error is still there on react-admin 3.0.1.

I also noticed, that if i don't touch the field, it now sends an empty string "" to the dataprovider instead of null.

When i touch the field and select the empty value there, i get a different error:

TypeError: Cannot convert undefined or null to object

this occures sanitizeEmptyValues, it will recursivly sanitize something like this data:

Bildschirmfoto 2019-11-25 um 17 34 25

notice the parentPage: undefined and parentPage.id: undefined

it will then call the sanitizeFunction with initialValues["parentPage"], which is undefined. So calling Object.keys(undefined) will crash. It should have a check there

@fzaninotto
Copy link
Member

I can't reproduce the error on master anymore, I believe it was accidentally fixed by #4103 or #4082. Feel free to reopen if the error is still there once 3.0.3 is released.

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

No branches or pull requests

4 participants