Adding support for multiple files coming from a single input

[aaronSig]

e.g.

<input type='file' id='files' name='files' multiple />

[tojocky]

Ar workaround you can set form.on('file', function(){})

[aaronSig]

Thanks thats handy to know

[tojocky]

Nothing wrong. I just wrote an workaround till the author will correct the error.

[tj]

Connect's bodyParser does this, it doesn't necessarily have to be in the lib but it would be nice to have the option (like parted does now)

[tojocky]

Yes, already checked for this issue!

[andrewrk]

Needs a test case.

[Jellyfrog]

@aaronSig Any news?

[svnlto]

still needs a test case.

[adius]

This should really get fixed!

[felixge]

Duplicate of #33, #104, #97, #49 .

This patch breaks BC and can't be merged as is.

A proper solution will either hide this patch behind a feature switch or bump the version number to 2.0 (we should probably clean / do a few more things if we were to take this route).

[jasonhargrove]

+1. confusing result

[Jammerwoch]

What's the status of this fix? I'm writing an O'Reilly book on Express, and I'm recommending Formidable over Busboy because of the convenience of getting the fileds/files objects in the callback...it would be nice to be able to tell my readers that multiple file uploads for the same field are supported, but it looks like this hasn't been touched for 5-9 months...?

[felixge]

What's the status of this fix?

The usual. Lots of people saying it's important, nobody bothering to do the work.

it would be nice to be able to tell my readers that multiple file uploads for the same field are supported

They are supported. You just have to use the event based API.

--fg

[Jammerwoch]

Heh. Yes, that is the usual problem with open source. Well, I would love to to fix it myself, but I don't fully understand what the problem is: the fix that aaron Sig submitted seems solid...the problem is that that the tests aren't passing? If that's the issue, I'll pull aaron's commit and find out why it's borking the tests and try to get this working, unless you know some reason I shouldn't.

Yes, I'm aware that multi-file uploads are supported through the events-based API, but I like the simplicity of the "files" object on the callback. And, when it comes to writing a book, well, page real estate is limited, and I have a lot of ground to cover.... Also, it's one of the main reasons I picked Formidable over Busboy.

[Jammerwoch]

I forked master, and applied a slightly neater version of aaron Sig's patch. I see that it does indeed break the legacy tests, line 226 in test/legacy/simple/test-incoming-form.js: assert.deepEqual(files, {file1: '2', file2: '3'});.

It fails because file1 comes back as ['1', '2'] instead of '2'. I see two ways to solve this problem: correct the test, or add a configuration switch to enable multi-file handling.

If it were my project, I would probably correct the test...but this may be one reason I'm not running any major open source projects! I want to do the right thing and not break functionality for people depending on this package. I would like a little guidance here: I'm happy to do the work, but I want to do it the right way.

[kethinov]

@Jammerwoch I think @felixge's point is that if we merge this PR as-is, it changes the default behavior which could break some people's existing apps if they depend on this default behavior as-is. That's what the failed test case reflects.

I agree with you that we should probably just change the default behavior eventually, but it's generally considered polite to bump the version to a new major number (2.x in this case) to indicate that we have altered default behavior.

Given that, it seems that his suggestion to hide it behind a feature switch is the best way to go for now, although I would also be in support of a 2.0 release which makes this the default behavior in the future.

To that end, I have submitted a fresh PR which refactors this one somewhat and implements it using a feature switch instead that should hopefully satisfy all of @felixge's (reasonable) objections to this PR as it is currently written.

Have a look here: #272

Additional feedback and suggestions are of course welcome.

Would love to see this issue resolved.