Serialize Form Data

[rmorshea]

Current Situation

Currently, onSubmit events for form elements do not serialize the data contained within them. To do this we need to serialize the form.elements attribute.

Proposed Changes

diff --git a/src/client/packages/idom-client-react/src/event-to-object.js b/src/client/packages/idom-client-react/src/event-to-object.js
index b11e653..80b098b 100644
--- a/src/client/packages/idom-client-react/src/event-to-object.js
+++ b/src/client/packages/idom-client-react/src/event-to-object.js
@@ -49,6 +49,13 @@ const elementTransformCategories = {
       return {};
     }
   },
+  hasElements: (element) => {
+    const result = {};
+    for (key in element.elements) {
+      result[key] = serializeDomElement(element.elements[key]);
+    }
+    return result;
+  }
 };
 
 function defaultElementTransform(element) {
@@ -69,6 +76,7 @@ const elementTagCategories = {
   ],
   hasCurrentTime: ["AUDIO", "VIDEO"],
   hasFiles: ["INPUT"],
+  hasElements: ["FORM"],
 };
 
 const elementTransforms = {};

Implementation Details

No response

[Archmonger]

There's going to need to be considerations made for file fields. We're also going to need to make considerations related to backpressure, especially since there's the potential to upload files with an uncapped size.

Probably worth generalizing the data serialization (perhaps even as a separate package?). If it's generic enough, there's a possibility that down the line it would allow for using the WS to stream video.

[rmorshea]

I think there's an issue (or discussion) about serializing files.

Could you make file serialization into an issue if it isn't one and create an separate issue for dealing with back-pressure?

[rmorshea]

@acivitillo this seems like a less-trivial first issue.

[Archmonger]

File serialization issue appears to exist: #574

Shouldn't backpressure be handled within the file serialization issue? Feels strange creating a new issue to handle an not yet developed limitation.

[rmorshea]

I think back-pressure is a general problem. Any event, not just those with files, could take a long time for the network to send and the server to process which would thus cause things to back up.

[Archmonger]

Something else I'm generally thinking about on this is there's going to need to be some degree of consideration for Django here.

Django forms have a validator function that is run to verify the form's data. I'm not sure how we'd work that in to an "IDOM form"...

Maybe we need some sort of concept of form middleware? Would also need some class mixins within Django IDOM, most likely.

[acivitillo]

Well, from my end I can take a stab at the initial PR idea, but I am not sure if I can also consider all the other cases. Could we implement form serialization incrementally via multiple PRs?

[Archmonger]

Yeah it can be handled incrementally.

[acivitillo]

ok so I started working on this, there is a problem with keys. This is because a form can have an indefinite number of elements, so target will be a json of multiple targets.

We can go with index keys, but then I need to understand how to build a test case for FORM in event-to-object.test.js

  hasElements: (element) => {
    const result = {};
    Object.keys(element).forEach((key) => {
      result[key] = serializeDomElement(element.elements[key])
    });
    return result;
  }

gives the below json

{
  "target": {
    "0": {
      "boundingClientRect": {
        "x": 8,
        "y": 8,
        "width": 149,
        "height": 22,
        "top": 8,
        "right": 157,
        "bottom": 30,
        "left": 8
      },
      "value": ""
    },
    "1": {
      "boundingClientRect": {
        "x": 8,
        "y": 80,
        "width": 52,
        "height": 22,
        "top": 80,
        "right": 60,
        "bottom": 102,
        "left": 8
      },
      "value": "Submit"
    },
    "2": {
      "boundingClientRect": {
        "x": 60,
        "y": 80,
        "width": 149,
        "height": 22,
        "top": 80,
        "right": 209,
        "bottom": 102,
        "left": 60
      },
      "value": ""
    },
    "boundingClientRect": {
      "x": 8,
      "y": 8,
      "width": 408,
      "height": 128,
      "top": 8,
      "right": 416,
      "bottom": 136,
      "left": 8
    },
    "__reactInternalInstance$6poh51syf6f": null,
    "__reactEventHandlers$6poh51syf6f": null
  },
  "currentTarget": {
    "0": {
      "boundingClientRect": {
        "x": 8,
        "y": 8,
        "width": 149,
        "height": 22,
        "top": 8,
        "right": 157,
        "bottom": 30,
        "left": 8
      },
      "value": ""
    },
    "1": {
      "boundingClientRect": {
        "x": 8,
        "y": 80,
        "width": 52,
        "height": 22,
        "top": 80,
        "right": 60,
        "bottom": 102,
        "left": 8
      },
      "value": "Submit"
    },
    "2": {
      "boundingClientRect": {
        "x": 60,
        "y": 80,
        "width": 149,
        "height": 22,
        "top": 80,
        "right": 209,
        "bottom": 102,
        "left": 60
      },
      "value": ""
    },
    "boundingClientRect": {
      "x": 8,
      "y": 8,
      "width": 408,
      "height": 128,
      "top": 8,
      "right": 416,
      "bottom": 136,
      "left": 8
    },
    "__reactInternalInstance$6poh51syf6f": null,
    "__reactEventHandlers$6poh51syf6f": null
  },
  "relatedTarget": null
}

[acivitillo]

I tried playing with a FORM test case but I get Cannot read property 'tagName' of undefined

  ...["AUDIO", "VIDEO"].map((tagName) => ({
    case: `adds 'currentTime' attribute for ${tagName} element`,
    tagName,
    output: { target: { currentTime: allTargetData.currentTime } },
  })),
  ...["FORM"].map((tagName) => ({
    case: `adds 'value' attribute for ${tagName} element`,
    tagName,
    output: { target: { value: allTargetData.value } },
  })),

[rmorshea]

Can you put up PR so I can look at the diff?

[acivitillo]

here