Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

numeric keys as arrays #14

Open
tj opened this Issue Jul 5, 2011 · 12 comments

Comments

Projects
None yet
6 participants
Owner

tj commented Jul 5, 2011

console.log(qs.parse('_method=put&data%5Bprojects%5D%5B0%5D%5Bname%5D=Stratus+Editor23&data%5Bprojects%5D%5B0%5D%5Bpath%5D=%2Fhome%2Fx%2Fnode%2Fstratus%2Fstratus&data%5Bprojects%5D%5B1%5D%5Bname%5D=Menu66&data%5Bprojects%5D%5B1%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Fmenu66&data%5Bprojects%5D%5B2%5D%5Bname%5D=499+(New)&data%5Bprojects%5D%5B2%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Fe499&data%5Bprojects%5D%5B3%5D%5Bname%5D=Utopia+Engine&data%5Bprojects%5D%5B3%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Futopia_engine&data%5Bprojects%5D%5B4%5D%5Bname%5D=499+(Old)&data%5Bprojects%5D%5B4%5D%5Bpath%5D=%2Fhome%2Fx%2Frails%2Fe499&data%5Bprojects%5D%5B5%5D%5Bname%5D=Stratus+2.0&data%5Bprojects%5D%5B5%5D%5Bpath%5D=%2Fhome%2Fx%2Fnode%2Fstratus%2Fstratus-2').data);

currently gives you:

  { projects: 
     { '0': 
        { name: 'Stratus Editor23',
          path: '/home/x/node/stratus/stratus' },
       '1': { name: 'Menu66', path: '/home/x/apps/menu66' },
       '2': { name: '499 (New)', path: '/home/x/apps/e499' },
       '3': 
        { name: 'Utopia Engine',
          path: '/home/x/apps/utopia_engine' },
       '4': { name: '499 (Old)', path: '/home/x/rails/e499' },
       '5': 
        { name: 'Stratus 2.0',
          path: '/home/x/node/stratus/stratus-2' } } }

Marak commented Jul 17, 2011

Ohh heh, here it is!

Contributor

aredridel commented Jul 17, 2011

Ooh!

qs.parse('user[0]=tj&user[1]=TJ');
// { user: [ 'tj', 'TJ' ] }

qs.parse('user[1]=tj&user[2]=TJ');
// { user: [ 'tj', 'TJ' ] } but should be { user: {'1': 'tj', '2': 'TJ'} }

Owner

tj commented Aug 6, 2011

@Delapouite hmm tough call, that could be a sparse array as well

Contributor

aredridel commented Aug 6, 2011

It should come out as a sparse array, I'd think.

Owner

tj commented Aug 7, 2011

yeah that's what I would expect

troyk commented Nov 17, 2011

This is related, not sure if you want a separate issue, but it's nice to drop the key all together when the ui allows the user to 'add another' functionality of nested form data.

console.log(qs.parse('workout%5Bname%5D=Stud&exercises%5B%5D%5Bsets%5D=1&exercises%5B%5D%5Bname%5D=Push+Up&exercises%5B%5D%5Bsets%5D=3&exercises%5B%5D%5Bname%5D=Pull+Up&exercises%5B%5D%5Bsets%5D=3&exercises%5B%5D%5Bname%5D=Situp+Up');

gives you:

{ workout: { name: 'Stud' },
  exercises: [ '1', 'Push Up', '3', 'Pull Up', '3', 'Situp Up' ] }

where (especially peeps used to rails) expect:

{ workout: { name: 'Stud' },
  exercises: 
   [ { sets: '1', name: 'Push Up' },
     { sets: '3', name: 'Pull Up' },
     { sets: '3', name: 'Situp Up' } ] }
Owner

tj commented Nov 17, 2011

@troyk agreed, that looks like a bug to me

troyk commented Nov 17, 2011

@visionmedia I was playing with it last night and gave up, but I'll go check it out again and see if I can muster up a pull request.

troyk commented Nov 17, 2011

Ok, so after messing with the current implementation, I decided to take a crack at porting Rack's to javascript, as I'm sure the Rack implementation is the right feature set. But it's getting clobbered on these tests:

    qs.parse('a[>=]=23')
      .should.eql({ a: { '>=': '23' }});

    qs.parse('a[<=>]==23')
      .should.eql({ a: { '<=>': '=23' }});

    qs.parse('a[==]=23')
      .should.eql({ a: { '==': '23' }});

and a few more. Shouldn't 'a[>=]=23' really be 'a%5B%3E%3D%5D=23' ? If so, I'll go add the encoding to the tests to and keep moving forward. If I'm missing something, just let me know. Below is where I'm at, let me know what you think about proceeding down this path.

function normalizeParams(params, name, v) {
  var matches = name.match(/^[\[\]]*([^\[\]]+)\]*/);
  if (!matches) return;

  if ('undefined' == typeof v) v = '';

  var k     = matches[1]
    , after = name.substr(k.length);

  if (after == '') {
    params[k] = v;
  } else if (after == '[]') {
    if (!params[k]) params[k] = [];
    params[k].push(v);
  } else if (matches = after.match(/^\[\]\[([^\[\]]+)\]$/) || after.match(/^\[(\d{1,9})?\](.+)$/)) {
    if (!params[k]) params[k] = [];
    var child_key = matches[1]
      , param_len = params[k].length 
      , last_val  = params[k][param_len - 1];

    if ('object' == typeof last_val && !Array.isArray(last_val) && !last_val[child_key]) {
      normalizeParams(last_val, child_key, v);
    } else if (child_key == String(param_len)) {
      params[k].push(v);
    } else {
      params[k].push(normalizeParams({},child_key,v));
    }
  } else {
    params[k] = normalizeParams(params[k] || {}, after, v);
  }
  return(params);
}

exports.parse = function(str) {
  var params = {}
  if (null == str || '' == str) return params;

  String(str).split('&').forEach(function(pair){
    pair = decodeURIComponent(pair.replace(/\+/g, ' ')).split('=',2);
    console.log(pair);
    normalizeParams(params,pair[0],pair[1]);
  });

  return params; 
}

IMO these should stay as objects with string keys, to match Rack/Rails behavior. We're running into an issue using Express to do some proxying of requests from client to Rails app. connect.bodyParser() uses qs and causes params like nodes[500]=159&nodes[501]=39 to turn into:

{"nodes": ["159", "39"]}

Whereas Rack/Rails sees the same params as:

{"nodes": {"500": "159", "501": "39"}}

Not that "because the Ruby community does it" is a great reason, but why break from convention?

I guess a (gross) workaround is to do something like this:

<input type="hidden" name="nodes[_]" value="">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment