v0.2.0 dependency on node-canvas problematic

[qrohlf]

Because v0.2.0 depends on node-canvas, I'm probably going to get some confused folks having trouble installing it because of the cairo dependency.

Possible solutions: make node-canvas an optional dependency, eliminate the node-canvas dependency (how?), include documentation on how to install cairo for node-canvas with the whole

PKG_CONFIG_PATH=/opt/X11/lib/pkgconfig npm install

thing.

Need to think about how to handle this one.

[labithiotis]

Yep, the exact issue I was just having until I changed version back to v0.1.5 for your react example.

Maybe forked repo to another module for server side rendering? Sure wish NPM had some way to dictate if a module is used/or should be used on client, server or both?

[qrohlf]

Right now jsdom and node-canvas are set to false in the browser field in package.json, so browserify etc should ignore them. It's just problematic because they're only required dependencies if you're planning on using Trianglify server-side, but they get installed for everyone because node doesn't do optional dependencies. There is of course a package for optional dependencies, but that involves bringing in yet another dependency (not what I want).

I'd really like to avoid forking into a server-side and a client-side version, but it might be unavoidable. For now I'm a bit too busy with other stuff to make any major changes to this codebase, so people will just have to live with installing Cairo in order to use Trianglify via npm. On Mon, Apr 20, 2015 at 1:44 PM labithiotis notifications@github.com wrote:

Yep, the exact issue I was just having until I changed version back to v0.1.5 for your react example.

Maybe forked repo to another module for server side rendering? Sure wish NPM had some way to dictate if a module is used/or should be used on client, server or both?

— Reply to this email directly or view it on GitHub https://github.com/qrohlf/trianglify/issues/34#issuecomment-94563737.

[labithiotis]

If I use v0.1.5 I get a jsdom console error on client: TypeError: undefined is not a function (evaluating 'jsdom.level(1, "core")')

And if I add the browser flag for jsdom to false in package.json I get Uncaught TypeError: jsdom.level is not a function

[qrohlf]

Version v0.1.5 has a completely different API than v0.2.x, and won't work with a module bundler.

To use Trianglify with a module bundler right now, you should install cairo and use Trianglify v0.2.1.

If you're on a mac, it will look something like this:

brew install cairo
PKG_CONFIG_PATH=/opt/X11/lib/pkgconfig npm install -S trianglify

[labithiotis]

soooo many dependencies for cairo -> xquartz :disappointed:

[qrohlf]

Yeah, it's definitely not ideal to force people to install the dependencies for node-canvas in order to use Trianglify with a module bundler, but right now I don't see an easy way around it that doesn't break canvas on the server.

Suggestions and/or PR's welcome.

[alexlande]

@qrohlf: I might be missing context here, but why not make canvas an optional dependency? I did that just now to get it working for my case without installing cairo: https://github.com/alexlande/trianglify/commit/fe4ffb1c9c7468ff6c49d34e93b4926754f91831

I can submit a PR with that change if you want it, let me know.

[qrohlf]

Alex, if you want to submit at PR that would be fine. I didn't know that optionalDependencies was an npm thing, actually. Looks like it'll require some additional error-handling in pattern.js to catch errors when jsdom doesn't find the node-canvas module.

Do you know if there's a way to have a script run if an optionalDependency fails? I think a message like "node-canvas was not installed because of a missing dependency. If you need server-side canvas & png support, please see https://github.com/Automattic/node-canvas for info on installing node-canvas." would be helpful in preventing frustration from folks trying to use Trianglify in node.

[alexlande]

@qrohlf: I don't know of any way to run a script/log something specific if an optionalDependency fails. I haven't dug into the source (and haven't used this library on the server), but one option could be to throw an error with a message like that if one of the server-side methods is called and the canvas dependency isn't available.

[qrohlf]

That's definitely going to be needed, I'd just also like to follow the principle of least surprise and warn users upfront if their install is missing features. For now I think postinstall is probably the hook I'll use.

[alexlande]

Ah, makes sense.

[qrohlf]

Just wanted to say for any new people running into this issue that a fix is coming; node-canvas will be an optional dependency in the next released version. No ETA, but now that I'm done traveling it should be fairly soon.

[jbraithwaite]

Can't wait, thank you @qrohlf

[qrohlf]

Finally!