-
Notifications
You must be signed in to change notification settings - Fork 6
add demo #2
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
base: master
Are you sure you want to change the base?
add demo #2
Conversation
|
Sorry about the delay - I've been quite busy over the past few days. I noticed that you modified lib/cjpeg.js. That file is a CommonJS module, meant to be used by Node.js. To use it in a browser, you'll need to bundle it correctly, which is done in the NPM package: https://unpkg.com/mozjpeg-js@3.1.0-beta.1/dist/mozjpeg.js Thanks for your contribution! I'll probably reuse your code once I get a chance to create a demo page (and properly document this library). |
|
Thanks, yes I know cjpeg.js was meant to be required and felt bad to modify it, but I use mozjpeg-js or pngquantjs from a Worker in browser, doesn't really need to be integrated in some other code, a global function is enough I think. Do you accept if I put a if (typeof module!=='undefined') .. in cjpeg.js instead to keep compatibilty?
|
|
That's already done in the bundled version. The Rollup build process automatically creates a module that, if running in a browser, creates a global function |
|
and the bundle comes with npm run build right, I had and still have this error: |
|
the problem is probably that index.js is in lib/ it should be at the root Ok fixed now, dist/mozjpeg.js is uningnored, because I need it to use it from the browser |
And thanks a lot for this project