brainfoolong / form-data-json

A zero dependency, cross browser library to easily get or set/manipulate form input values as/from a json object.
https://brainfoolong.github.io/form-data-json
MIT License
60 stars 10 forks source link

v2-dev: make `uncheckedValue` false by default #15

Closed KES777 closed 3 years ago

KES777 commented 3 years ago

Is your feature request related to a problem? Please describe.

    <input type="checkbox" name="scales[]">
    <input type="checkbox" name="scales[]" value="77">

image

Describe the solution you'd like If uncheckedValue is undefined by default then result of .toJson can not be passed .fromJson eg. operation is not reversable

At this example we have two check boxes. Second have value 77. And when we setup { scales: ['77'] } nothing is selected, because first checkbox does not have 77 value.

So in this case we need to loop over all checkboxes with scales names to work properly. Even doing so, we have corner case:

    <input type="checkbox" name="scales[]">
    <input type="checkbox" name="scales[]">
    <input type="checkbox" name="scales[]" value="77">

When second scales is checked result will be { scales: [ 'on' ] }. And we will not know which was checked: first or second??

So for consistent results and for reverse (result of .toJson can be passed back .fromJson (so backend can send data as is)): please make uncheckedValue false by default

Thank you

brainfoolong commented 3 years ago

Good point. This is a tricky situation. I consider this a problem that native solutions never have fixed. Generally, this library should do pretty much the same thing as native FormData and PHP by default and both do the same, they only make an array of the checked elements. If first is not checked, you loose the information to restore the form from given data.

But, i agree. This should be false by default, to workaround this issues of native behaviours.

Will be changed.

brainfoolong commented 3 years ago

Thx for report. Changed as suggested in 2.0.3beta

KES777 commented 3 years ago

does not work for commit:0361d0a

image

brainfoolong commented 3 years ago

Hm, are your sure? It's set to false by default, i've double checked the tests, minified and not minified version of the current v2 branch.

KES777 commented 3 years ago

I am sorry. I pull, but forget to copy dist to my directory =) It works thank you.

brainfoolong commented 3 years ago

Thanks for your tests.

brainfoolong commented 3 years ago

Ok, i have to really undo this. This behaves so differently to all standards, this is not good. In all standards (Browser, FormData, PHP, etc...) unchecked checkboxes are simply not submitted.

This library should do the same by default, i undo this in the beta to "undefined", it must be like that in my opinion.

Even if that mean, that you cannot use the result in fromJson() when not setting uncheckedValue by hand to null or false.

KES777 commented 3 years ago

IMHO: We see that standard behavior is ugly. There are tons of workarounds for this "standard" behavior... Never understand why this "standard" must be by default if first thing will be escaping from it, workaround, etc.? If this standard send broken data which gives nothing, why we should send it at all?

If you count that this library should be same by default, then I will not defend my opinion.
Also I does not support this "standard" default. ;-)

brainfoolong commented 3 years ago

The good/bad situation of this standard really depends on what forms you generate. The case also only applies for non indexed/autoincrement keys in checkboxes, which is an edge case. To break compatibility with other software/v1 just for this very special case is not a good decision. Correctly indexed keys eg.: chk[foo] still can be used in fromJson()

I can think of both good reasons for this behaviour and also for bad reasons.

If you really don't want this behaviour, it is an easy one-liner to make it globally.

FormDataJson.defaultOptionsToJson.uncheckedValue = false

brainfoolong commented 3 years ago

Thanks for your feedback, i close this.