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
Browser version requires missing files. #694
Comments
I've noticed that simply removing the |
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 |
Oops! That was just the top level |
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 Anyway, I have changed this one single line. For more, please see pull request #918 . |
Expected Behavior
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.
Current Behavior
Browserify (most likely the browser too, if you use the browser version directly in the browser) will error due to erroneous calls to
require()
.Failure Information (for bugs)
The error complains about
./src/converter
, which simply doesn't exist injimp/browser
(wherejimp
is thenode_modules
subfolder wherejimp
is installed, when installed fromnpm
).Steps to Reproduce
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:browserify index.js
.Context
Failure Logs
The text was updated successfully, but these errors were encountered: