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

Empty text fields and "" vs null #8537

Closed
hiaselhans opened this issue Dec 30, 2022 · 1 comment
Closed

Empty text fields and "" vs null #8537

hiaselhans opened this issue Dec 30, 2022 · 1 comment

Comments

@hiaselhans
Copy link
Contributor

What you were expecting:

I have a couple of forms with textfields.

An example openapi spec would be:

Person:
  required:
    - name
  type: object
  properties:
    name:
      type: string
   nickname:
      type: string

Here is what i would expect:

create:

  • name: "michael", nickname: "" => {name: "michael"}
  • name: "michael", nickname: "mick" => {name: "michael", nickname: "mick"}

update

  • name: "michael", nickname: "" => {name: "michael", nickname: ""}
  • name: "michael", nickname: "mick" => {name: "michael", nickname: "mick"}

What happened instead:

When "nickname" was empty, in the request object "nickname" was set to null.
this is documented behavior:
https://github.com/marmelab/react-admin/blob/master/docs/SimpleForm.md#sanitizeemptyvalues
and it is stated that this is expected by most backends.

// parse empty string into null as it's more suitable for a majority of backends
const defaultParse = (value: string) => (value === '' ? null : value);

From my understanding, this is how an optional text-field would be handled in a backend

  • not present in the object => optional => don't touch it
  • empty string "" => set the value as an empty string
  • any other string => set the value.

So it might make sense to remove the value from the object when the state has not changed, but to make it null when it is empty feels a bit counter-intuitive to me. In pydantic for example this would be an error:

image

Also this reads to me as it would be preferable to use an empty string:
https://stackoverflow.com/questions/45575493/what-does-required-in-openapi-really-mean

This seems to be a choice made by react-admin dev's and that*s okay, but I would like to get your motivation on this and maybe you can point me to the backends that implemented this in that particular way?

I will also send a pull-request to update the docs with an example if that's okay.

related issues/prs

#8188
#8262

#8472

Environment

  • React-admin version: 4.6.0
  • Last version that did not exhibit the issue (if applicable): 4.2.2
  • React version: 17
  • Browser: recent firefox & chrome
  • Stack trace (in case of a JS error):
@djhi
Copy link
Contributor

djhi commented Jan 3, 2023

@djhi djhi closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants