Replace sharp with jimp module #191
Conversation
- removes libvips dependency - simplifies installation process - should work on Node 9
| @@ -66,8 +66,8 @@ module.exports = function(gulp, config) { | |||
| fileClone.path = path.join(name.dir, name.name + '-' + size + name.ext) | |||
| fileClone.imageTransform = { | |||
| width: size, | |||
| height: file.imageData.aspectRatio ? Math.ceil(size/file.imageData.aspectRatio) : null, | |||
| options: file.imageData.options || {}, | |||
There was a problem hiding this comment.
Maybe print some warning if user uses options that it's not working anymore?
lib/tasks/images.js
Outdated
| .resize(file.imageTransform.width, file.imageTransform.height) | ||
| .quality(image.quality) | ||
| .getBuffer(jimp.AUTO, function(err, buffer) { | ||
| if(err) console.error(err) |
There was a problem hiding this comment.
Was so in sharp version as well, but: Shouldn't we pass the error up in the callback? Not just silently dismiss it?
There was a problem hiding this comment.
That's the question - is a broken image a fatal fail or just a inconvience?
There was a problem hiding this comment.
If the image is then missing it's a fail.
| .read(file.contents) | ||
| .then(function(image) { | ||
| image | ||
| .resize(file.imageTransform.width, file.imageTransform.height) |
There was a problem hiding this comment.
Sharp's resize does crop. But JIMP just resizes it no matter what.
There was a problem hiding this comment.
Do you mean that it does scale up but sharp doesn't? Or how does the cropping work in sharp?
There was a problem hiding this comment.
No. If both height and width is specified it resized it (keeping aspect ratio) and then cropped it (to specified dimension). JIMP on the other hand resizes it but does not keep aspect ratio, which leads to distortion.
There was a problem hiding this comment.
Now I understand. Okay, I'll add the behavior.
lib/tasks/images.js
Outdated
| .then(function(image) { | ||
| image | ||
| .resize(file.imageTransform.width, file.imageTransform.height) | ||
| .quality(image.quality) |
There was a problem hiding this comment.
From what I understand it will fail if undefined (source).
There was a problem hiding this comment.
default is set at 90 few lines above
There was a problem hiding this comment.
(but was undefined anyway, thx for the sharp eye!)
|
The speed difference is significant (in favor for sharp), but that is expected:
I would still add some caching strategy to the whole image optimization process (like store original hash, settings for that file etc.) and optional storage outside of dist folder - perhaps AWS S3 bucket? |
|
Closing this for now. More in the #187 |
Todos
Opinions? Can you please test it if it works for you?
Thanks ❤️
Fixes #187, closes #190, see #180