Using the browser version of JIMP, e.g. when running Browserify on a Node.JS package that requires JIMP, should be clean, as it should contain all of the JIMP code in a single file.
Browserify (most likely the browser too, if you use the browser version directly in the browser) will error due to erroneous calls to require().
The error complains about ./src/converter, which simply doesn't exist in jimp/browser (where jimp is the node_modules subfolder where jimp is installed, when installed from npm).
I haven't tried to use JIMP directly in the browser, but here's how to reproduce this error the way I noticed it.
npm i --save jimp).index.js file with the following contents:const JIMP = require('jimp');
browserify index.js.> [email protected] build /home/gustavo6046/Source code/nodemips
> browserify -r ./index.js:mips | babel --minified -f minif.tmp | uglifyjs >browser/mips.js
Error: Cannot find module './src/converter' from '/home/gustavo6046/Source code/nodemips/node_modules/jimp/browser/lib'
at /home/gustavo6046/Source code/nodemips/node_modules/browser-resolve/node_modules/resolve/lib/async.js:55:21
at load (/home/gustavo6046/Source code/nodemips/node_modules/browser-resolve/node_modules/resolve/lib/async.js:69:43)
at onex (/home/gustavo6046/Source code/nodemips/node_modules/browser-resolve/node_modules/resolve/lib/async.js:92:31)
at /home/gustavo6046/Source code/nodemips/node_modules/browser-resolve/node_modules/resolve/lib/async.js:22:47
at FSReqWrap.oncomplete (fs.js:152:21)
I've noticed that simply removing the browser field from JIMP's package.json seems to fix the issue (although I haven't tested the output in a browser environment yet).
Do you think should be browserifying itself? Or should it be left to users
Pardon me, I'm not sure if I get your question; what should be Browserifying itself?
Jimp already tries to make a browserified version of itself (thats what isn't working for you). My question is who should the browserify step belong to, me or you?
I didn't add that functionality so I'm not sure how useful or necessary it is
I'm pretty sure it would be better to leave it to the user, unless JIMP relies on features that don't work in the browser.
I agree with @Gustavo6046. I think it would be better left to the user as well. Any updates regarding this issue?
Yeah, I'm interested too! JIMP is really cool, but kind of obscure, and this could actually be pretty helpful in that regard.
This just requires removing the browserify bundle from npm right? We would still need to use it to test in a browser (I think).
More than happy to accept a PR for this
I can do it if you want me to. It'll be quick and simple enough.
Either way, maybe it would be a good idea to assign someone to do the job, to avoid concurrent pull requests trying to do the same thing.
Go for it!
@Gustavo6046 any updates on this?
Ah yes, sorry. Been busy lately, but I can now. Give me a moment.
I cloned it, and it seems that the browser field does not exist in package.json anymore, which leads me to believe that this issue might already have been solved in a commit since it was first put here. I will quickly test it, though.
Oops! That was just the top level package.json in the repo. Totally overlooked packages! My bad!
Yep, that seems to work so far. I think Browserify is supposed to handle the "browserification" itself (hence the name), and a library explicitly concerning with Browserify compatibility will break stuff. Instead, we should _only_ concern with building a single JS output for the web, and allow that Browserify do that itself if Jimp is linked as a npm dependency rather than a script tag in a webpage somewhere.
To this end, all that is needed is to remove the browser field from package.json, as package.json is only ever used by npm (and thus Browserify), and thus there is no point in including it. To build for including with a script tag, use a development environment, that is, use Browserify as a npm script, and distribute the output .js file for browser usage (e.g. using a CDN or a home website). Browsers don't use npm!
Anyway, I have changed this one single line. For more, please see pull request #918 .