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.

Sign up for GitHub

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

Jump to bottom

null / undefined handling #105

Closed
TheYoxy opened this issue Jul 17, 2024 · 5 comments · May be fixed by theskillwithin/remix-hook-form#1
Closed

null / undefined handling #105

TheYoxy opened this issue Jul 17, 2024 · 5 comments · May be fixed by theskillwithin/remix-hook-form#1

Comments

@TheYoxy
Copy link
Contributor

TheYoxy commented Jul 17, 2024

As is, when you create a form data with a value that has been defined with a value, even values that represents nothing (null and undefined), it gets serialized in string.

Wouldn't it be better to ignore theses values or return the raw underlined type ?

One idea of test implementation in the case that the choice is to handle base type:

  describe('should handle undefined', () => {
    it('with stringify all', () => {
      const data = {
        undefined: undefined,
      };
      const formData = createFormData(data, true);
      expect(formData.get('undefined')).toEqual(undefined);
    });
    it('with stringify all', () => {
      const data = {
        undefined: undefined,
      };
      const formData = createFormData(data, false);
      expect(formData.get('undefined')).toEqual(undefined);
    });
  });
  describe('should handle null', () => {
    it('with stringify all', () => {
      const data = {
        null: null,
      };
      const formData = createFormData(data, true);
      expect(formData.get('null')).toEqual(null);
    });
    it('with stringify all', () => {
      const data = {
        null: null,
      };
      const formData = createFormData(data, false);
      expect(formData.get('null')).toEqual(null);
    });
  });
@theskillwithin
Copy link

theskillwithin commented Sep 12, 2024 •
edited
Loading

+1

@theskillwithin theskillwithin mentioned this issue Sep 14, 2024
Draft
13 tasks
@theskillwithin
Copy link

@TheYoxy actually I don't think this can be done in createFormData because this is using FormData

https://developer.mozilla.org/en-US/docs/Web/API/FormData/append#value

What is really desired here is something that conform does with parseWithZod

https://conform.guide/integration/remix

@hannesj
Copy link

hannesj commented Sep 16, 2024 •
edited
Loading

Null works already when used together with generateFormData, as JSON.parse("null") returns null. However, undefined is not valid JSON, so the function throws, and returns "undefined" instead. I think createFormData should just filter out undefined values, as formData.entries in generateFormData would then not contain that entry.

@oezbeyyf
Copy link

That's a good point regarding "null". So basically, theskillwithin#1 should do what we want (filter "undefined" in createFormData before putting the data into FormData and sending it).
But perhaps the same check for "null" should be disregarded, as "null" should properly work with JSON.parse("null")?

@AlemTuzlak
Copy link
Contributor

AlemTuzlak commented Sep 17, 2024 •
edited
Loading

@here fixed with 5.1.0, from this version onwards undefined values are ignored and not sent over the wire as they are not really useful in any way, null values are still serialized and the reason behind this is that sending null to your database might mean a whole different thing than just not sending anything so you might want the null to be there to remove some set values. I would recommend handling the null values in your own code if you want to ignore them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle null undefined theskillwithin/remix-hook-form
5 participants
@hannesj @theskillwithin @AlemTuzlak @TheYoxy @oezbeyyf