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

An array index number #7

Merged
merged 9 commits into from Oct 18, 2016
Merged

Conversation

jumper423
Copy link
Contributor

No description provided.

an array index number
an array index number
@elodszopos
Copy link

elodszopos commented Oct 16, 2016

Let's go @iambumblehead :D!

@iambumblehead
Copy link
Owner

The tests fail with this branch. What do you think of putting this behaviour behind a flag? for example,

formurlencoded(obj, { isarrayindex : true });

@elodszopos
Copy link

I'd be more than happy to. Let's give @jumper423 a chance to respond.

@jumper423
Copy link
Contributor Author

I can not think of a situation when you need a flag with a value of "false". So I think it is not necessary.

I can correct tests. To do this?

@iambumblehead
Copy link
Owner

Current users of this script get arrays encoded as '[]'. I have not used index encoded array values myself and am not sure if it is a safe change to make... Do you think the tests should be changed or do you think a flag would be best? Would you explain which change is best for users of this script?

I will test it with an express service to see how express receives data encoded with these changes

@iambumblehead
Copy link
Owner

iambumblehead commented Oct 16, 2016

Express does parse these correctly... please update the tests. I will merge and update the npm package.

and by the way, thank you

@jumper423
Copy link
Contributor Author

It's my pleasure. Later this loan. Justify your position and more specific tests will change.

@iambumblehead
Copy link
Owner

iambumblehead commented Oct 16, 2016

the "i" here would be undefined... that line encodes an empty array

@iambumblehead
Copy link
Owner

@jumper423 what's going on :)

@jumper423
Copy link
Contributor Author

If we have input

{array:[1,2,3]}

We can convert it to

array[]=1&array[]=2&array[]=3

If the input

{
   parent:[
      {array1:[1,2]},
      {array2:[3,4]}
   ]
}

It is necessary to convert to

parent[0][array1][]=1&parent[0][array1][]=2&parent[1][array2][]=3&parent[1][array2][]=4

At the same time get the following picture

parent[][array1][]=1&parent[][array1][]=2&parent[][array2][]=3&parent[][array2][]=4

What comes out of this if decode execute.

{
   parent: [
      {
         array1: [1]
      },
     {
         array1: [2]
      },
      {
         array2: [3]
      },
      {
         array2: [4]
      },
   ]
}

When you do not set the digital code, he can not understand that it is one and the same element of the array and each time it creates a new one.

In addition, the key doesn't see the point. As they use this library. Should have meet the same problem when using multidimensional arrays.

You can also see the difference in the results for example on this resource.
http://www.freeformatter.com/url-parser-query-string-splitter.html

Enter what happens in You and look at the answer

http://site.com?parent%5B%5D%5Barray1%5D%5B%5D=1&parent%5B%5D%5Barray1%5D%5B%5D=2&parent%5B%5D%5Barray2%5D%5B%5D=3&parent%5B%5D%5Barray2%5D%5B%5D=4

And compare with my data

http://site.com?parent%5B0%5D%5Barray1%5D%5B0%5D=1&parent%5B0%5D%5Barray1%5D%5B1%5D=2&parent%5B1%5D%5Barray2%5D%5B0%5D=3&parent%5B1%5D%5Barray2%5D%5B1%5D=4

I hope the example is clear.

Sorry to be so long responding.
If you have any questions, I will try to promptly answer them.

@iambumblehead
Copy link
Owner

I'm using express to look at the results... I understand what you are showing

@iambumblehead iambumblehead merged commit e59e31e into iambumblehead:master Oct 18, 2016
@jumper423
Copy link
Contributor Author

jumper423 commented Oct 18, 2016

You can also make indexes was not raised when all the array elements are scalar data type, then we are not numbered.
But in this case you will have to go through the entire array and check. I'm not sure that it is justified in order to save a few bytes of memory when sending http request.

@jumper423
Copy link
Contributor Author

View to express, please, at least I checked in php.

@iambumblehead
Copy link
Owner

iambumblehead commented Oct 18, 2016

The output you gave above is parsed correctly by express. Do you want these changes published to npm?

@jumper423
Copy link
Contributor Author

Yes

@iambumblehead
Copy link
Owner

thank you, it is published as form-urlencoded@1.5.0

@elodszopos
Copy link

Thanks kind sir.

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.

None yet

3 participants