janczizikow / sleek

:chart_with_upwards_trend: Sleek is a modern Jekyll theme focused on speed performance & SEO best practices
https://janczizikow.github.io/sleek/
MIT License
423 stars 638 forks source link

dependency requires node >= 8 #9

Closed benpatterson closed 6 years ago

benpatterson commented 6 years ago

There is a dependency (puppeteer) that requires node 8. Users on older versions of node encounter an install failure (as per below):

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! puppeteer@0.12.0 install: `node install.js`
npm ERR! Exit status 1

Happy to update package.json but thought I'd log an issue for now. Workaround for anyone that wants to work with sleek is to upgrade to node 8 or better.

janczizikow commented 6 years ago

Hey @benpatterson,

Is that a big issue? I think >= node 8 is the most recent stable version. Not sure from where puppeteer dependency comes from, perhaps it's a "dependency of a dependency"?

benpatterson commented 6 years ago

@janczizikow The EOL for node 6 is April 2019. (Node 4 EOL's this month.) It's your call if you want to take action...I just wanted to call this out since I ended up spending time troubleshooting an install failure; took some digging to determine why the failure was occurring when following the install steps. Could also be a doc update if you'd rather keep package.json where it is.

puppeteer is a dependency of penthouse. I'm happy to put in a PR if that would help, but it's all good if you want to handle this a different way. Thanks for taking the time to read through my issue.

janczizikow commented 6 years ago

Hey @benpatterson,

I just took a look: penthouse is a dependency of critical. It's a package to inline the critical CSS, which helps to improve page render time. Did you fix this issue on your machine by upgrading node or removing penthouse? I don't really want to give up on critical. Since this theme focuses on performance, having an autorunner task to inline the critical chunks of CSS it's rather a priority. I think the easiest solution would be to update the docs like you said.

Thanks to you for your time and creating this issue 👍