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

Add support for sparseIndex #48

Open
KES777 opened this issue Jan 28, 2025 · 3 comments
Open

Add support for sparseIndex #48

KES777 opened this issue Jan 28, 2025 · 3 comments

Comments

@KES777
Copy link

KES777 commented Jan 28, 2025

Is your feature request related to a problem? Please describe.
Sometimes forms are dynamic. Elements could be added or deleted. To same computing power and simplify solution
we can keep indexes as is.

Image

Here is how data looks before the fix:
Image

You can notice objects and that 0 item was deleted by trash button.

Describe the solution you'd like

--- a/src/form-data-json.js
+++ b/src/form-data-json.js
@@ -76,7 +76,14 @@ class FormDataJson {
      * If false than all values that are multiple (multiple select, same input names che>
      * @type {boolean}
      */
-    'arrayify': true
+    'arrayify': true,
+
+    /**
+     * If true then we do not require fields to named sequentially during arrayify
+     * false does not have any effect
+     * @type {boolean}
+     */
+    'sparseIndex': false
   }
 
   /**
@@ -241,15 +248,16 @@ class FormDataJson {
      * @return {*}
      * @private
      */
-    function arrayify (object) {
+    function arrayify (object, options) {
       if (FormDataJson.isObject(object)) {
         let count = 0
         let valid = true
         for (let key in object) {
           if (FormDataJson.isObject(object[key]) && !(object[key] instanceof Element)) {
-            object[key] = arrayify(object[key])
+            object[key] = arrayify(object[key], options)
           }
-          if (parseInt(key) !== count) {
+          let number =  parseInt(key);
+          if( Number.isNaN(number)  ||  number !== count && !options.sparseIndex ) {
             valid = false
           }
           count++
@@ -272,7 +280,7 @@ class FormDataJson {
     function output () {
       if (options.skipEmpty) returnObject = removeEmpty(returnObject) || (options.flatLi>
       if (options.arrayify) {
-        returnObject = arrayify(returnObject)
+        returnObject = arrayify(returnObject, options)
       }
       return returnObject
     }

Here is expected structure we got after the fix:
Image

@brainfoolong
Copy link
Owner

I am actually not sure if i like this. It manipulates the result in a way that it cannot be restored with fromJson when you have activated this option. It's basically "Manipulating the form input names during toJson to fix the applications code when the form is changed".

In your case, your app is responsible to correctly rename form input names after manipulating the form.

@KES777
Copy link
Author

KES777 commented Jan 29, 2025

Yes, agree. When DB row, for example, has 3 items: i1, i2, i3. It is very important to keep the order. If user put the value into i2 it is important to store that value into the second column.

Another example with static IDs is when we put real IDs into the name, eg. Phone[33][phone]. But when new item is added to this list, which index it should be assigned? If we assign [34] it will conflict with another existing phone in DB, [0] or [1] will lead to the same problem.

Company:
3 company1
7 company2

Phone:
ID | company_id | phone
1 77 phonex
33 1 phone1
34 2 phone2
35 2 phone4

From the example above you can see that for company1 we can not add phone with index 34. It is already taken by the phone for company 2.
Thus it does not make sense to put real ID as index into names like Phone[33][phone].

But lets take into account other cases when the order it not important. Lets take DB as at the previous example. We did GraphQL query to DB and it returns the next data structure:

{
  Company: {
    id: 3
    name: company1
    Phone: [
      {
        id: 34
        phone: phone2
      },
      {
        id: 35
        phone: phone4
      }
    ]
  }
}

This is flattened into the HTML form with input's names like this:

  Company[id]
  Company[Phone][0][id]
  Company[Phone][0][phone]
  Company[Phone][1][id]
  Company[Phone][1][phone]

And onto the form we can delete phone with ID 34 (index 0 at the data structure) and add as many phone as we want, eg. 50 phones.

And this should be transformed into the same data structure recognizable by GraphQL

{
  Company: {
    id: 3
    name: company1
    Phone: [
      {
        id: 34
        phone: phone2
        _action: delete
      },
      {
        id: 35
        phone: phone5
      },
      {
        phone: phone9
      },
      {
        phone: phone8
      }
    ]
  }
}

No ID means that phone 8 and phone9 will be created.
phone with id:34 will be deleted
phone with id:35 will be updated (because of passed ID)

If there is no such thing as sparseIndex then array of Phone will be passed as {} hash. Which is not expected and not desirable data structure.

@brainfoolong
Copy link
Owner

brainfoolong commented Jan 30, 2025

I have exactly the same things in many of my applications and in my opinion it is exactly what the app need to deal with... Not an external Form Reader library.

For example 1: One time we need it the same as you, an array. So we just do write input names starting from 0 upwards.
If we delete one, no matter what, we just iterate all inputs and rename them again, starting from 0 upwards. And with "arrayify" enabled, you always get an array without manipulating the output.

For example 2: Another time we have almost the same as example 1, but we do not rename existing inputs. We just remember the highest used index and if we add new items, we add them with highest index + 1. So you never come in the situation that the same index is used twice. We do not get an array then but that is expected.

For example3: Another time we do not want an array, we have real database ids in the indexes, like Company[Phone][239][foo].
So when we add new items, we add them with a prefix in the id to know in backend which thing is new. Company[Phone][new_0][id]

You see, all cases an app need should and can be done by the app and it's not a thing for this library to fix apps behaviour.

I don't really see sparseIndex to be a thing for all those cases.

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

No branches or pull requests

2 participants