Feature/sharp resizer #24
Conversation
| @@ -231,35 +229,27 @@ plugin.uploadImage = function (data, callback) { | |||
| } | |||
|
|
|||
| fs.readFile(image.path, function (err, buffer) { | |||
| uploadToS3(image.name, err, buffer, callback); | |||
| var maxWidth = parseInt(meta.config.maximumImageWidth); | |||
There was a problem hiding this comment.
I don't think this is correct. I looks like it will try to resize files? that are not images.
There was a problem hiding this comment.
I think "normal" files and images are processed in different hooks and therefore in different handler methods.
Normal files are processed by plugin.uploadFile
There was a problem hiding this comment.
It kind of is there are cases when both hooks are called or when a file can be passed to the image hook so its best to just let that through. I've not used sharp and don't know what errors it gives but a try catch around the resize and just uploading if it fails would be best I think.
| @@ -6,14 +6,13 @@ var AWS = require("aws-sdk"), | |||
| fs = require("fs"), | |||
| request = require("request"), | |||
| path = require("path"), | |||
| sharp = module.parent.require("sharp"), | |||
There was a problem hiding this comment.
I may be wrong but this looks like it would try and require sharp from nodebb not the s3-uploads package?
There was a problem hiding this comment.
You're right with your thought but let me explain the reason of using the import in that way:
If I use sharp = require("sharp") it works fine in my local dev environment as it should, but when I use the same import statement in Heroku containers, the index.js of the plugin throws an error saying that sharp package could not be found.
I also saw that "sharp" package was being installed inside the "nodebb -> node_modules" folder and not in "nodebb -> node_modules -> s3-uploader -> node_modules" after a clean nodebb install.
With sharp = module.parent.require("sharp") approach, it works in both local devel and Heroku containers.
Why?
Well... it's a weird behaviour that I can't explain better, but it works
There was a problem hiding this comment.
NPM will sometimes install it in the nodebb -> node_modules folder but it should always put a reference to it in the s3-uploader -> node_modules -> bin folder as far as I'm aware. This sounds more like a bug with heroku and npm than the code. I'm also reticent to try and add fixes for specific platforms as the plugin needs to support all platforms.
I've been updating the old ImageMagick with the newer and faster sharp image processor.
In addition, this new branch checks and resizes (if needed) the size of the uploaded image according to forum's general settings.
Original benchmark results:
