Skip to content

Conversation

@layerssss
Copy link

Hi, this issue is illustrated in http://jsfiddle.net/x6fz3mgt/ .

The expected output is:

{
  "items": [
    {
      "name": "",
      "types": ["dog"]
    },
    {
      "name": ""
    }
  ]
}

But the current output is:

{
  "items": [
    {
      "name": ""
    },
    {
      "types": [
        "dog"
      ],
      "name": ""
    }
  ]
}

layerssss added a commit to layerssss/jquery-serialize-object that referenced this pull request Nov 27, 2014
layerssss added a commit to layerssss/jquery-serialize-object that referenced this pull request Nov 27, 2014
layerssss added a commit to layerssss/jquery-serialize-object that referenced this pull request Nov 27, 2014
@layerssss
Copy link
Author

Hi, this is a patch to properly handle nested array, with two integration tests added.

@layerssss layerssss force-pushed the master branch 2 times, most recently from 95e6d74 to 8d5742e Compare November 27, 2014 06:34
layerssss added a commit to layerssss/jquery-serialize-object that referenced this pull request Nov 27, 2014
layerssss added a commit to layerssss/jquery-serialize-object that referenced this pull request Nov 27, 2014
@layerssss
Copy link
Author

ok, it's mergable now! This patch properly handled nested array cases.

@macek
Copy link
Owner

macek commented Nov 28, 2014

The array "push" operator is not really meant to be used like that. Without a specific index, the serializer has no context to logically group items as you originally intended.

The following form

<input name="items[0][name]" value="foo">
<input name="items[0][types][]" value="dog">
<input name="items[0][types][]" value="cat">

<input name="items[1][name]" value="bar">
<input name="items[1][types][]" value="bat">
<input name="items[1][types][]" value="rat">

Will generate

{
  "items": [
    {
      "name": "foo",
      "types": [
        "dog",
        "cat"
      ]
    },
    {
      "name": "bar",
      "types": [
        "bat",
        "rat"
      ]
    }
  ]
}

Is this more along the lines of what you were going for?


Because the [] is really only intended to be used at the end of a key, I think the default validation pattern should be changed to not parse a key that has [] the middle of it.

For example, in[][valid] would be invalid, but valid[] would be valid.

@layerssss
Copy link
Author

Thanks for the suggestion @macek ! But I really feel that there can be a simple rule for the push operator to keep track of the context. that is:

  1. use a path to keep track of array size for each depth of push

  2. get path of the rest part to identify which part of the element are they assigning (other push operators should be converted into index to indicate difference)

    e.g

    items[][name]: the next-level key is name
    items[][types][]: the next-level key is types[0]
    items[][types][]: the next-level key is types[1]
    items[][name]: the next-level key is name
    items[][types][]: the next-level key is types[0]
    items[][types][]: the next-level key is types[1]

  3. check if the key of the last element (in the array) has been assigned value.

  4. if not ,just assign it. if yes, push a new element, and assign it.

This rule is actually applied in my patch.

  1. https://github.com/macek/jquery-serialize-object/pull/45/files?diff=unified#diff-fec0876574a1cb80e49bb26ab46fc899R57
  2. get the path of the reset
  3. incrementPush(path, key)
  4. also in incrementPush(path, key)

I'm not sure whether this format/usage is listed in any specs. But do you know is there a spec for this?

@macek
Copy link
Owner

macek commented Dec 1, 2014

@layerssss I sincerely appreciate the work and detailed information you put in here. There is actually a draft spec I'm currently working towards: W3C HTML JSON form submission. You can see more discussion about this in #24.

I have an morning appointment so I'm going to generate more of a response for you a bit later today. Meanwhile, if you have a chance, take a look at that spec.

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

Successfully merging this pull request may close these issues.

2 participants