qrohlf / trianglify

Algorithmically generated triangle art
http://qrohlf.com/trianglify/
GNU General Public License v3.0
10.08k stars 669 forks source link

Install fails on Node 4.* #45

Closed mattmischuk closed 8 years ago

mattmischuk commented 9 years ago

Some of the dependencies fail on install with the latest versions of node. Seems as this is coming from JSDom which has already resolved this issue on the latest version.

full error https://gist.github.com/mattmischuk/553e359d3157dca2a0ed

kilianc commented 9 years ago

+1

kilianc commented 9 years ago

and to be fair, on npm there should be a dist folder with the file we need already built

kilianc commented 9 years ago

and, it turns out there is, (also in the repo?) but the issue is:

  "postinstall": "node scripts/postinstall.js"

which triggers after npm i

kilianc commented 9 years ago

and... all dependencies should probably be devDependencies. This way whoever is just "using" it can access the dist/ folder. If you want to hack it, you can git clone it and then npm i it.

kilianc commented 9 years ago

for whoever needs this now you can do:

npm i --save-dev kilianc/trianglify#no-post-install

I hope this help while this gets fixed! :beers:

qrohlf commented 9 years ago

@kilianc not everyone is using this from the browser, so pushing all the deps into devDependencies is not a good solution.

Can you post the stack trace of postinstall failing?

kilianc commented 9 years ago

@qrohlf I see your point but still not clear how this can work in the browser as pre-built package and not in a node app.

Here is the gist: https://gist.github.com/kilianc/80a04637368592822671

jme783 commented 9 years ago

+1

qrohlf commented 8 years ago

Alright, I finally had a look at what's causing this - it's related to a bug in contextify: https://github.com/brianmcd/contextify/issues/188, which is a dependency for jsdom.

Since one of the primary use cases for Trianglify is definitely in the frontend, I will likely move jsdom to the optionalDependencies field where it belongs in the v0.4.0 release (which will be supporting node 4.2 LTS).

HOWEVER, for now v0.4.0 is blocked by that contextify bug, since I can't develop Trianglify against node 4.2 if my deps won't install and my tests won't run.