#211 Add a transformFile option

[wesleytodd]

This is obviously not a PR ready for merging. But I wanted to post it to get the discussion started. Before I write tests and docs, is there anything inherently wrong with the approach I took on this? Is there any problem with not handling the fileStream events? Or would that be up to the implementor of the transform method?

This is an example of how I saw this working from a users perspective:

var upload = multer({
  transformFile: function (req, file, cb) {
    file.stream = pump(file.stream, resizeMyImage(file), saveMyImageSomewhereElse(file))
    // Optionally change other parts of the file object...
    cb(null, file)
  }
})

Thoughts? Feedback before I write tests to make sure it all works correctly?

[LinusU]

Looks good, it should probably look something like this though

var upload = multer({
  transformFile: function (req, file, cb) {
    cb(null, pump(file.stream, resizeMyImage(file)))
  }
})

[LinusU]

Sorry about that, wrong button :)

[wesleytodd]

Oh, so the reason I returned the whole file object is because it offers more flexibility to change other parts. For example, in my app I also modify the filename when saving to make it a normalized path. I can post the code example if you need more context.

[wesleytodd]

I actually edited the example, because it was completely wrong lol. The new code is the correct implementation.

[wesleytodd]

ping :)

[wesleytodd]

Sorry to be a pain. We have published this to our internal npm, but I just like to follow up on my open pull requests.

[aslafy-z]

I agree, this should be merged !

[LinusU]

Could you add a test or two as well?

edit: Also, what do you think about having the callback give just the new stream? I think it makes more sense and falls in line with the other callbacks...

function defaultTransformFile (req, file, cb) {
  cb(null, file.stream)
}

edit2: Also, can you rename the identity function to defaultTransformFile or something else. I think it's expected that a function named identity works like x => x

edit3: Also, sorry for the delay on this, vacation times :)

[wesleytodd]

@LinusU I was planning on adding tests, as I mentioned in my first comment, I wanted feedback before I wrote the tests in case this was not the direction you wanted to go in. Now that I have the feedback I will go ahead and do that.

Edit: I originally had it working that way, but I also realized that it might be nice to be able to transform other options on the file object. For example, if a user uploads an image that is a photoshop document, but you want to convert all images to jpg before saving, you might want to change the file info to reflect the new format. Thus the modifications on the file object are nice to expose.

Edit2: Sure, I guess I was thinking of it as a async version of the traditional identity :)

Edit3: No worries!! Its summer and we should all be enjoying that. I'm just overly meticulous about following up on my PR's. Hope you enjoyed your vacations!

[clakech]

👍 this may help us to upload client side encrypted file to s3 using multer-s3.

[wesleytodd]

Ok, I updated it and added two tests. I didn't change it to only pass the stream, because I think my use case above is good. If you disagree I would love to hear why, or if there is some other way to do that that I cannot think of.

Thanks!

[iyunusov]

So did you pull it? and if so how can I use it ? nothing is mentioned in the documentation about this method you have discussed here

[wesleytodd]

This has not been merged yet. We have been using the fork at my company for a bit now without issues. Feel free to install from that.

[niftylettuce]

What needs to be done to get this merged @LinusU?

[LinusU]

@niftylettuce I have another angle on this in #399 which I think solves this and many other problems, would love some feedback on that one

[niftylettuce]

@LinusU I'm definitely in favor of streams - but how would we implement something similar to this transformFile before sending it over to storage with streams? Maybe I just need to see more of the Readme.md rewritten with the new stream stuff.

[LinusU]

Since you get a stream from multer, you can do anything to it before storing it. E.g.

req.file.stream
  .pipe(resizeMyImage())
  .pipe(saveMyImageToDisk())

[niftylettuce]

Can you give more of a full example? Sorry I don't know how you get to req.file.stream?

[LinusU]

absolutely

const app = require('express')()
const upload = require('multer')()

app.post('/profile', upload.single('avatar'), (req, res) => {
  req.file.stream
    .pipe(resizeMyImage())
    .pipe(saveMyImageToDisk())
})

[niftylettuce]

Any chance you could also rewrite the Readme with this? This is absolutely great. Ship this and major version bump! 👍

[LinusU]

Full example, which encrypt files and saves them to disk:

const fs = require('fs')
const crypto = require('crypto')

const app = require('express')()
const upload = require('multer')()

app.post('/profile', upload.single('avatar'), (req, res) => {
  const cipher = crypto.createCipher('aes192', 'a password')
  const output = fs.createWriteStream('uploads/avatar.jpg')

  req.file.stream.pipe(cipher).pipe(output)
})

[LinusU]

Yes, I would love to improve the readme to show this!

[niftylettuce]

Awesome work. I plan to use the branch with http://sharp.dimens.io/ and https://github.com/lovell/attention

[wesleytodd]

I am going to close this, in favor of the streams implementation. #399