search

tj / palette

Node.js image color palette extraction with node-canvas
292 stars 24 forks source link

node-canvas shouldn't be mandatory #12

Open micky2be opened 7 years ago

micky2be commented 7 years ago

I was using this module on browser side. Having node-canvas as an hard dependency kind of breaks the purpose of using it and makes our modules installation slower for something we don't need.

ghost commented 7 years ago

I can look into switching node-canvas to an optional dependency around 8am PT.

ghost commented 7 years ago

Unfortunately optional dependencies don't look like they would make installing the package without node-canvas any easier. However, the package was originally intended for use on the backend, so I'm not planning on removing node-canvas as a dependency. My recommendation for the time being would be to fork the repo and modify it as you need to for your use case.

You could also look at the palette.js source, since it's actually pretty short and is just making a call to quantize.js with an array of pixels. You might be best off just grabbing the quantize.js source and using it instead.

This npm package for quantize might also suit your needs better.

ghost commented 7 years ago

We can chat more about this if you'd like, but I'm going to close this issue for now, unless there's a good reason to open it back up.

micky2be commented 7 years ago

It's really sad. We had a perfect module for browser and nodeJS without your changes. The only place node-canvas is required is in the test and benchmark.

ghost commented 7 years ago

Installing the module at version 0.0.1 should give you the exact same version you were using previously, with no node-canvas hard dependency.

As I said, I'm more than happy to discuss why node-canvas should or should not be a dependency. My original reason for adding the hard dependency was because I personally thought it would be beneficial to have the dependency installed along with the module itself. However, that was also based on my original impression that the module was primarily intended for server-side nodeJS.

If node-canvas is moved back to being simply a development dependency, I think it would be appropriate to update the README and repository description to reflect that. Perhaps if you'd also like to add a PR with an example based on your browser-side use-case, it might help the use cases clearer.

How does that sound?

micky2be commented 7 years ago

My use case is really simple, a user load an image in the browser and we need to detect the main color of it. Since we load the image in a canvas in the browser it is pretty straight forward to use this module.

canvas is a browser feature, it should be logic to be compatible with it without having to install node-canvas.