Accepting multiple fields/files of the same name

[cjc]

G'day,

I've added the ability to accept multiple values for the same field/file name. Default behaviour is unchanged, new functionality is selectively enabled with the bindAsList and bindAllAsList properties. Unit tests for the new functionality and updated readme.

Cheers.

[felixge]

Hmm, I'm not sure if this should go in or not. Can you explain your use case and why using the 'file' / 'field' events doesn't work well there?

[cjc]

The initial use case was for a form. I needed to process all the input at once, rather than file at a time, so it made more sense to have everything supplied to parse than to gather the file event output manually.

More generally I was missing the bind as list behaviour I got used to with asp.net MVC and Monorail. I'm not sure how common it is in other frameworks, certainly the same thing could be achieved with the file/field events. It might make sense for me to keep this functionality in a wrapper around formidable rather than as part of it.

[felixge]

I've slept a few nights over this now, and I think I'd be ok with merging this. However, this is what I think the API should be like:

Make it a single option called arrayFields. By default arrayFields is false. Setting arrayFields to true acts like bindAllAsList, and setting an array acts like bindAsList.

What do you think? Would that work for you as well?

[cjc]

That works, a nice improvement to the API too. I'll make the changes tonight and see how it goes. Cheers.

[dvv]

Hi! Is there a chance to see options.arrayFields in master?

Also, shouldn't it be an automated guess: if we see a field and fields[name] is set then make it array unless it already is or push to this array. I admit that it is easy to move this logic to custom 'field' and 'file' handlers, but having such stuff in master is a good default.

Best regards,
--Vladimir

[felixge]

dvv: No, automatically converting a field into an array is a very bad thing. Otherwise you would always have to run Array.isArray() on every parameter you access, or explicitly typecast it to a string. Both are bad in terms of convenience and potential of unexpected errors.

[ghost]

What is the status of putting this in master?

[dvv]

NAKed

[coolaj86]

Seems like the best way to handle this issue would be to do it the same way the HTTP spec does - treat the fields as a whole as an array, not on a field-by-field basis and not as maps:

Rather than fields.fieldname or files.fieldname have the fields and files be arrays rather than maps.

form.onComplete(function (err, fields, files) {
    fields.forEach(function (field) {
        console.log(field.name);
    });
});

This seems to satisfy all needs

• multiple files of the same name are represented
• developers get consistency - no magic arrays
• developers get to decide how to handle multiple files
• no special syntax such as fieldname[] is required

And sense developers can easily provide their own map function, no real convenience is lost.

[felixge]

I think I like coolaj86's proposal.

[cjc]

Sorry, long coding hiatus. I concur that coolaj86's proposal is nice and elegant.

[felixge]

Ok, I will implement the new API as soon as I find time, or somebody sends me a patch + test.