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

Fields with brackets ([]) #386

Closed
ghost opened this issue Jul 6, 2016 · 6 comments · Fixed by #545
Closed

Fields with brackets ([]) #386

ghost opened this issue Jul 6, 2016 · 6 comments · Fixed by #545

Comments

@ghost
Copy link

ghost commented Jul 6, 2016

I'm not sure if this is the appropriate place for this, but I had a need to use form fields with names using brackets: field_name[bracket1][bracket2]. (I guess I really don't NEED this, but it sure makes handling on the server side much better organized.)

I wrote a few lines of code that take the formidable fields and breaks them into JSON objects and values. For instance formidable would pass in the above example as a field named fields.'field_name[bracket1][bracket2]', and I need to have that dealt with in the form fields.field_name.bracket1.bracket2

I don't know whether one would expect to find this code in formidable or some other location of the stack, but I have included below the code snippet I created to make this work:

let form = new formidable.IncomingForm();
form.parse(req, (err, fields, files) => {
  // Handle array inputs
  let newFields = {};
  for(let _i1 in fields) {
    let _curLoc = newFields,
         _split = _i1.split('['),
         _splitLength = _split.length - 1;
    for(let _i2 in _split) {
      let _pointer = _split[_i2].replace(']', '');
      if(_i2 == _splitLength) {
        _curLoc[_pointer] = fields[_i1]; 
      } else {
        _curLoc[_pointer] = _curLoc[_pointer] ? _curLoc[_pointer] : {};
      }
      _curLoc = _curLoc[_pointer];
    }
  }
  fields = newFields;
  // do other stuff
}
@tunnckoCore
Copy link
Member

tunnckoCore commented Jan 16, 2017

What I did in koa-better-body is to collect them as "url" string and pass it to some querystring module.

for example

let qs = require('qs')
let buff = ''

form.on('field', (name, value) => {
  buff += `${name}=${value}&`
})
form.on('end', () => {
  const fields = qs.parse(buff.slice(0, -1))
  console.log(fields)
})

something like that?

I believe it will work because my package have tests for such nested forms - test/urlencoded.js#L60

@badeball
Copy link
Contributor

The problem I see with this, is that there is no standard way of interpreting a non-trivial query string. querystring will emit an array if there are multiple keys with the same value. From the Ruby on Rails-world, I am used to keys postfixed with [] being interpreted as arrays. It really seems highly opinionated to me. Since this is a low-level library, I think it should not be opinionated in terms of this. Rather, I think it should parse and emit the key-value pairs that are found and leave it up to the user of interpreting them.

@tunnckoCore
Copy link
Member

@badeball yea, absolutely.

It has workarounds and we shouldn't do something and it's not a job of that package.

@DylanPiercey
Copy link

I personally use https://github.com/DylanPiercey/q-set when parsing forms to achieve this effect.
Hope it helps some of you :).

adrienjoly added a commit to openwhyd/openwhyd that referenced this issue Nov 26, 2017
solved by using q-set to parse form data fields with brackets, after using formidable to parse the requests’ body (node-formidable/formidable#386 (comment))

closes #126.
@tunnckoCore
Copy link
Member

tunnckoCore commented Nov 28, 2019

Hm. Okay, seems like #380 is pretty basic implementation then. I just realized that.

Overall, most of the issues and PRs are about this support - both in fields and files. So we need to think seriously about some good implementation and #380 won't be enough.

tunnckoCore added a commit that referenced this issue Nov 28, 2019
add bullet for #386
tunnckoCore added a commit that referenced this issue Nov 28, 2019
@tunnckoCore
Copy link
Member

Going to close, in favor of #545 .

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 a pull request may close this issue.

4 participants