arrayFormat for parse #329
Conversation
|
Are these tests expected to pass as-is, or fail? |
|
they’re expected to fail. i just wanted to be sure that this is the behavior we want. if you’re ok with it - i’ll proceed to the implementation |
|
@dreyks i think it would be helpful to have two commits - one with passing tests, and another with the tests that fail. It makes it much easier for me to see what's breaking or not, and what requires a new option or not. |
|
oh. gotcha. ok will do |
dreyks
force-pushed
the
parse-array-format
branch
from
a2bd7e1 to
136b204
Compare
|
done |
ljharb
force-pushed
the
parse-array-format
branch
from
136b204 to
146dff0
Compare
ljharb
force-pushed
the
parse-array-format
branch
from
146dff0 to
7ed81cd
Compare
| @@ -34,17 +34,31 @@ test('parse()', function (t) { | |||
|
|
|||
| t.test('arrayFormat: brackets allows only explicit arrays', function (st) { | |||
| st.deepEqual(qs.parse('a[]=b&a[]=c', { arrayFormat: 'brackets' }), { a: ['b', 'c'] }); | |||
| st.deepEqual(qs.parse('a[0]=b&a[1]=c', { arrayFormat: 'brackets' }), { 'a[0]': 'b', 'a[1]': 'c' }); | |||
There was a problem hiding this comment.
i see how this seems correct, because it's currently set to brackets but is parsing indices.
However, the current behavior matches what PHP, Rails, and express all do, so I'm not sure it should change.
There was a problem hiding this comment.
yeah, i'm not very sure on this one too, though you have to explicitly state you want this behavior so maybe it's ok
alternatively allowing arrayFormat to be an array itself could solve it
t.deepEqual(qs.parse('a[0]=b&a[1]=c', { arrayFormat: ['brackets', 'indices'] }), { 'a': ['b', 'c'] });
t.deepEqual(qs.parse('a[]=b&a[]=c', { arrayFormat: ['brackets', 'indices'] }), { 'a': ['b', 'c'] });| st.deepEqual(qs.parse('a=b,c', { arrayFormat: 'brackets' }), { a: 'b,c' }); | ||
| st.deepEqual(qs.parse('a=b&a=c', { arrayFormat: 'brackets' }), { a: ['b', 'c'] }); | ||
| st.deepEqual(qs.parse('a=b&a=c', { arrayFormat: 'brackets' }), { a: 'c' }); |
There was a problem hiding this comment.
i see how this seems correct, because it's currently set to brackets but is parsing as repeat.
However, the current behavior matches what PHP, Rails, and express all do, so I'm not sure it should change.
There was a problem hiding this comment.
Rails and PHP actually parse it as overwriting
this is the reason I've started all this: in my rails app I've switched from "chaos and anarchy " to qs and got one spec failure where the suite was appending params and with qs that lead to an array
There was a problem hiding this comment.
hmm, thanks for correcting me then. i'll think on this one.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as resolved.
This comment was marked as resolved.
|
@dreyks ok, so, here's how i think this needs to work:
One important goal is that the default behavior of Rails, PHP, and express should all be expressible with an option - ideally the same one, and I suspect that's "brackets". It'd be great to get this revived :-) |
|
oof, lemme recollect what's going on here, it's been 4 years and not the easiest ones :D |
added basic tests. without arrayFormat parse's behavior does not change
don't think we want to allow specifying several formats simultaneously?