mithril.js

mithril.js copied to clipboard

Hi, This is a breaking change proposal about a querystring parser and builder.

Current Behavior

The object is lost a type when use number with brackets.

query = m.buildQueryString({
  hash:  {0: "a", 1: "b"},
  array: ["a", "b"]
});
// > "hash%5B0%5D=a&hash%5B1%5D=b&array%5B0%5D=a&array%5B1%5D=b"
// > decode: "hash[0]=a&hash[1]=b&array[0]=a&array[1]=b"


m.parseQueryString(query)
// Both will convert to an array.
// > {hash:["a","b"],array:["a","b"]}

And, some servers do not handle query correctly.

for example, The rack of ruby (ruby on rails) interpret a[0]=1&a[1]=2 as Hash.

Possible Solution

The querystring array does not use number with brackets.

query = m.buildQueryString({
  hash:  {0: "a", 1: "b"},
  array: ["a", "b"]
});
// > "hash%5B0%5D=a&hash%5B1%5D=b&array%5B%5D=a&array%5B%5D=b"
// > decode: "hash[0]=a&hash[1]=b&array[]=a&array[]=b"

m.parseQueryString(query)

// > {hash:{"1": "a", "2": "b"},array:["a","b"]}

Context

• Prevent loss type.
• Prevent problems between servers.

Your Environment

• Version used: 1.1.5

---

Can i implement it and send PR? Best regards,

I'd back it up, considering I rarely see the foo[0]=whatever syntax outside of Mithril (I usually see foo[]=whatever).

Thanks for a reply.

I tried implement it. But i couldn't think of a good solution about parsing a nested array. ( https://github.com/MithrilJS/mithril.js/blob/next/querystring/tests/test-parseQueryString.js#L56 )

ex) {a: [[1,2],[3,4]]} =[decode]=> "a[][]=1&a[][]=2&a[][]=3&a[][]=4" =[parse]=> ????

I think a nested array is unusual. But should I keep it?

Best regards,

@alfa-jpn You should, since such functionality already exists. It should parse back to what you put into it: {a: [[1, 2], [3, 4]]}. FWIW, that's what Mithril does today with the indices.

@isaaclyman Thanks for reply. I understood the policy.

How about making it configurable?

ex)

m.queryString.arrayWithNumber= // config
m.queryString.parse() // moved from 'parseQueryString'
m.queryString.build() // moved from 'buildQueryString'


m.queryString.arrayWithNumber=true // existing behaviour (default)
m.queryString.build({a: [1,2,3]}) // return "a[0]=1&a[1]=2&a[2]=3"

m.queryString.arrayWithNumber=false // unsupported a nested array
m.queryString.build({a: [1,2,3]}) // return "a[]=1&a[]=2&a[]=3"

Best regards,

@alfa-jpn I believe you tagged me by mistake.

Sometimes, the github auto-completion has weird heuristics...

@alfa-jpn

How about making it configurable?

Eh, I'd rather not. Not for such a simple utility. Worst case scenario, the old version can find its way here, and migration should be relatively painless after making that substitution. It's pretty simple if you just read the source code.

@pygy

Sometimes, the github auto-completion has weird heuristics...

I'd have to vouch for that. At least it's not as bad as Gitter's, though (which I find myself frequently backspacing the name completed). 😄

@isaaclyman Sorry for mistake. :bowing_man:

---

@isiahmeadows Thanks for the advice. I hope I can support old format with mithril-helper, Because I could not resolve the deep array issue.

I implemented it for reference.

• mithril.js
  • https://github.com/MithrilJS/mithril.js/pull/2053
• migration
  • https://github.com/isiahmeadows/mithril-helpers/pull/2

Best regards,

@MithrilJS/collaborators This okay to shortlist for v2? We're kind of the exception here in generating and requiring integers. It's also a pretty simple one-line fix for buildQueryString and (I think) parseQueryString.

• For buildQueryString, you replace key + "[" + i + "]" with key + "[]" here.
• For parseQueryString, I think you remove the second condition, the !isNaN(parseInt(nextLevel, 10)) check, on this line.