Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

img.getMIME is not a function #5

Open
turakvlad opened this issue Mar 5, 2018 · 12 comments
Open

img.getMIME is not a function #5

turakvlad opened this issue Mar 5, 2018 · 12 comments

Comments

@turakvlad
Copy link

I have tried to use your sample but it does not work for me with Node 8.9.4 (of course, I have provided the paths to the real images on my local machine). I am getting img.getMIME is not a function.

Thanks for the help.

@turakvlad
Copy link
Author

turakvlad commented Mar 5, 2018

One more thing. As I can see you're using:
img.getBuffer(img.getMIME(), (buf) => console.log(buf));

I think it's not right because jimp uses error-first callback here and the first argument will contain an error and the second will contain the result (buffer in our case). Please, take a look at this - https://github.com/oliver-moran/jimp#writing-to-buffers. In comments there are

Node-style callback will be fired with result

It should be
img.getBuffer(img.getMIME(), (error, buffer) => console.log(buffer));

I am still investigating the issue with getMIME(). It works fine with this code:

jimp.read("sample.jpg").then(image => {
    console.log(image.getMIME());
}).catch(console.log);

But this function does not work with the result of your mergeImg function. I mean that this object (https://github.com/preco21/merge-img/blob/master/src/index.js#L84) does not have getMIME() method on itself.

@preco21
Copy link
Owner

preco21 commented Mar 6, 2018

Thanks for reporting the issue!

One more thing. As I can see you're using:
img.getBuffer(img.getMIME(), (buf) => console.log(buf));

Yes, indeed this is wrong usage. I made this for a long time ago and I could not find until you report it 😂. I'm going to fix the document now. Again, thanks for reporting this.

Let's get back to the original issue here, I assume this is probably upstream issue: img.getMIME is not a function.

I could find such method on the upstream source code: https://github.com/oliver-moran/jimp/blob/f7b5e5b543b012069c513ae8a2368c388e54e6ad/index.js#L834-L837

However, I'm not quite sure when this method is applied in which version. But I could find that the getMIME() method is added over about a year ago: jimp-dev/jimp@3e70734#diff-168726dbe96b3ce427e7fedce31bb0bcR697

@turakvlad Could you provide which version of jimp you used for your experiments?

@turakvlad
Copy link
Author

@preco21, no problem. I am happy to help.

I am using 0.2.28 for my experiments. As I can see, you're using 0.2.27. I've just played with these versions a little bit and ... the problem is actually with 0.2.27. I have updated jimp from 0.2.27 to 0.2.28 in merge-img package in my local node_modules folder and now it works like a charm.

Please, try it on your side. If everything is okay, it looks like we resolve this issue 😃

@turakvlad
Copy link
Author

You were right. getMIME() was added just 2 commits after releasing 0.2.27. The 0.2.27 was released on August, 12 and the support for getMIME() was added on August, 13.

Releasing 0.2.27 version - jimp-dev/jimp@2dbb58f.
Adding support for getMIME() - jimp-dev/jimp@3e70734
selection_018

@preco21
Copy link
Owner

preco21 commented Mar 8, 2018

@turakvlad Hmm, I think this is going to be much more tricky: #3

I have downgraded it because of the issue causing some sort of errors on browser. Actually this should not be an issue since this module is focused on node environment. But, as you know, better than having nothing.

jimp has infrequent releasing cycle. They didn't update the module after releasing v0.2.28 and there are a number of commits made to their module ever since that release.

The thing is that they have different build to support browser:

I believe this issue was on the use of browser build of the module in the node-like environment like Electron via webpack.

Since webpack automatically manages the environment as like node, so the developers don't have a reason to use browser build and webpack will do rest of jobs.

So hacky and simple the workaround is to change mainFields option in webpack from user side.

According to jimp collaborator, he mentioned that the module currently published to npm is not identical to one in the repo: jimp-dev/jimp#280 (comment)

So I installed the module via npm directly and I found this:

carbon

I can confirm that is not the same one. Since this is an upstream issue, I don't think we could solve it from our side.

Thoughts?

@turakvlad
Copy link
Author

Hi, @preco21!

I am sorry for my delayed response.

I have downgraded my jimp from 0.2.28 to 0.2.27 because of this issue - jimp-dev/jimp#262. This problem was crucial for me (and I think not for me only).

To my mind, 0.2.28 is not stable and "buggy" a little bit.

Maybe, it would be better not to upgrade jimp to 0.2.28 and leave it as now - 0.2.27. I just recommend you to update this sample (remove img.getMIME() function) in your README.md.

@preco21
Copy link
Owner

preco21 commented Mar 28, 2018

@turakvlad Alright, thanks for pointing out that. I’ll check it.

@elipeters
Copy link

Probably need to update the readme on NPM also..

@elipeters
Copy link

elipeters commented Apr 17, 2018

I'm also noticing a third reduction in file size between img.getMIME() over implicitly setting the mime type to Jimp.MIME_JPEG

All source images I used are jpg and the result is 367.8 KB to 212.2 KB.

@preco21
Copy link
Owner

preco21 commented Apr 18, 2018

@elipeters If I understand what you meant correctly: Using img.getMIME() instead of Jimp.MIME_JPEG produces different file sizes even if you match every input image's mime type to jpeg?

If so, I guess this is yet another upstream issue. As for now, merge-img does not take handle image processing itself. Instead, it's essentially relying on another library. As you know it's jimp.

I have plan to make it more like a general purpose in v3, still working on it.

@Dogatorix
Copy link

So now that getMIME is removed from merge-img, now what?

@preco21
Copy link
Owner

preco21 commented May 19, 2018

@PokeTehDoge Now things left is to make some stuff better.

But I have been so busy for a couple of months so I couldn’t get to make some stuff for this module.

I’ll start work on it as soon as I finish my job.

But, always PRs are welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants