multer
multer copied to clipboard
Switch to stream based API
As have been discussed in a few other issues, I thought it would be interested to see what the api would look like if Multer just gave you .stream
on the uploaded files. This removes the concept of storage engines completely and you can use any other module that works with streams, instead of multer-specific modules.
I would love some thoughts and feedback on this β€οΈ
ping @expressjs/multer-collaborators, @wesleytodd
Love this idea. It is much simpler, more modular, and would solve way more use cases than the current situation (#356 for example). I don't have time now to read all the code, but in theory this has my thumbs up. I will try to look through it more throughly this weekend.
My only issue @LinusU is to fix it so that you expose _makeMiddleware
again because it's needed in koa-multer
here https://github.com/koa-modules/multer/blob/master/index.js#L19. Here was the commit it was removed in https://github.com/expressjs/multer/commit/42a9e3b6376ddd9878f2eb26060713e7479ccb3f.
Just released 2.0.0-alpha.1
for you guys to try it out. It's published under the next
-tag and thus won't be automatically installed for anyone.
π
@niftylettuce I'll look into the koa-issue shortly π
@niftylettuce see koa-modules/multer#11
You are a rockstar. How do I donate to you?
On Oct 1, 2016 10:02 AM, "Linus UnnebΓ€ck" [email protected] wrote:
@niftylettuce https://github.com/niftylettuce see koa-modules/multer#11 https://github.com/koa-modules/multer/pull/11
β You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/expressjs/multer/pull/399#issuecomment-250914034, or mute the thread https://github.com/notifications/unsubscribe-auth/AAf7hWB-603_neTY5DRoVIaJ8iV_WiTdks5qvmfagaJpZM4KJnV5 .
Also @LinusU - is limits option still enforced?
How do I donate to you?
There are others who need it more than I, e.g. https://my.charitywater.org/donate/home
is limits option still enforced?
Yes they are :ok_hand:
2.0.0-alpha.2
is out with improvements to the error codes. Happy coding :tada:
To try it out, use the following command: npm install --save multer@next
πππ On Sun, 2 Oct 2016 at 10:44 PM, Linus UnnebΓ€ck [email protected] wrote:
2.0.0-alpha.2 is out with improvements to the error codes. Happy coding π
To try it out, use the following command: npm install --save multer@next
β You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/expressjs/multer/pull/399#issuecomment-250982577, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5_YBKGjMMJ9UAM2cWfbp0LBglL5mHSks5qv-Z_gaJpZM4KJnV5 .
It's working great for me! I have a fork of koa-multer
with this version added and @LinusU fork fix https://github.com/niftylettuce/koa-multer/tree/lu-fix
Solves many problems, thanks!
But I don't understand why no encoding and mimetype properties in a file? I think it's critical.
Oh nevermind, mime type is usually determined by file's extension, and file encoding would not help at anything. Correct if wrong.
That is correct, as I have understood it the encoding is only how it's sent over the wire and doesn't matter at all for the end user
@dougwilson Would you mind giving this a quick look? π
I'm close to feeling that this is ready for releasing :)
@LinusU CHANGELOG
needs an update.
- [x] changelog updated
Since it will help people keeping their apps secure, and since a lot of the opened issues are about it, I'm leaning towards including mime-detection with 2.x. This is my current thoughts:
The File
object would gain the following fields:
// The detected mime-type, or null if we failed to detect
detectedMimeType: String | null
// The typical file extension for files of the detected type, or null if we failed to
// detect (with leading `.` to match `path.extname`)
detectedFileExtension: String | null
// The mime type reported by the client using the `Content-Type` header, or null if the
// header was absent
clientReportedMimeType: String | null
// The extension of the file uploaded (as reported by `path.extname`)
clientReportedFileExtension: String
Any thoughts on this would be appreciated; anyone is welcome to give feedback! :)
@LinusU I think this is a great idea, as I imaging almost everyone needs most of this info anyway. I know I do in my apps.
Note to self: When releasing this, remember to update this page: http://expressjs.com/en/resources/middleware/multer.html
Released [email protected]
with file type detection support π
Released [email protected]
with a fix for #455 π
Released [email protected]
with a new error code that will be given when a client aborts a request. Related to #438
Fairly stupid question because I'm not terribly familiar with multer other than whats on the docs, but what is the API to consume the stream? I essentially want to proxy the multipart file upload request to another backend and would like to take the stream and pass it along
@oshalygin You can see the docs here: https://nodejs.org/api/stream.html
But most likely you just want to pipe
the entire request if you want to maintain the multipart form encoding. In which case you wouldn't need this module. If on the other hand, you want to pipe just one file to the upstream, then you could decode the multipart form with this module, and just write the one file to the upstream.
@wesleytodd awesome thanks I'm going to give that a whirl.
@LinusU I like this idea of switching multer API to stream. :)
Few thoughts regarding multer new api implementation: Are you creating a temporary file for each file field and then creating read stream from that file? (https://github.com/expressjs/multer/blob/explore-new-api/lib/read-body.js#L71) If so, couldn't we get the same outcome with current multer using DiskStorage - createReadStream and then remove/unlink created file? (https://github.com/expressjs/multer/blob/explore-new-api/lib/middleware.js#L25) Furthermore, what if someone would prefer to stream/pipe uploaded file through memory instead of a temporary file?
Last one: what are your thoughts about an option to pipe the request's stream file directly? (Without writing anything to a temporary file or memory, although I guess that's not the purpose of this library).
Hi @orenklein, sorry for the late reply :)
Are you creating a temporary file for each file field and then creating read stream from that file? If so, couldn't we get the same outcome with current multer using DiskStorage - createReadStream and then remove/unlink created file?
That is probably doable, although here it would be done automatically for the user...
Furthermore, what if someone would prefer to stream/pipe uploaded file through memory instead of a temporary file?
My plan is to let this decide itself, so that small files only ends up in memory, and large files on disk. Since the new api completely hides this implementation detail, it could be added at a later date without anything needing to be changed for the user!
Last one: what are your thoughts about an option to pipe the request's stream file directly? (Without writing anything to a temporary file or memory, although I guess that's not the purpose of this library).
I think that this use case is better served by busboy, or by having a lower level api that we in itself build Multer on top of
@LinusU Can you publish a new package with this?
Actually I mean - why don't we do this directly in a new major version of multer?