-
Notifications
You must be signed in to change notification settings - Fork 661
audit npm dependencies #337
Comments
👍 I think we should take the time to analyze our dep tree in more detail after 1.1 is out. There are a few things we could try here like |
|
Ah, that seems right. Thanks for reporting @JosefJezek. Removed those in #358. |
LGTM |
The dep tree has a lot of duplicates, so it will be significantly smaller when deduped or installed with npm@3. |
Thanks for the feedback @sindresorhus. Started looking into deduping and shrinkwrapping and came to the same conclusions about @azakus any opposition to us switching to this approach upstream? |
Probably for the 0.1% use case of people not having bower installed. BTW, there's both a gulp-vulcanize, and vulcanize in that list. Probably only need one. |
That really depends on your system. If there are precompiled binaries available it will be faster, but compiling those libs from source is extremely slow. But yeah, it adds a lot of overhead regardless.
Please do so with npm@3, so to measure with the right conditions. Also keep in mind that the dependency tree printed by npm@3 is the virtual tree and not what's actually on disk, as it's deduplicated on disk, but not in the printed tree. |
@paulirish had a comical experience installing this with npm@3 last friday His 💻 === 💀 🔥 |
It's taking ~10-14 minutes with |
Ugh. That's unfortunate. Thanks for commenting on the upstream regression and also putting together the shrinkwrap for them to repro. Honest question: given the clear slowdown in dependency installation with |
I don't think it will be a big problem yet. Most users, especially the front-end people using this, are from experience very slow with updating both npm and node. I guess it couldn't hurt to warn people temporarily though. |
@sindresorhus worth opening a ticket on those offenders |
@samccone Do you think we'll be able to take an initial look at this this week? If not, we might need to bump it to the next milestone. |
I just did a clean install on the latest npm and it took well over 7 minutes on a newish macbook air I think this is really undesirable and we should try and figure out what we can do to make this fast. |
@samccone Bother npm about it? A large part of it is clearly a regression in npm@3 as npm@2 i so much faster relatively (still slow though). It's also clear from #337 (comment) where the biggest size offenders are. |
Looks like |
@chuckh Good catch. It may be worth us trying to switch before the next release is out. |
I found this using Bithound. You can see it for PSK at https://www.bithound.io/github/PolymerElements/polymer-starter-kit/master. @addyosmani I will create a pull request for gulp-minify-css today, |
Thanks Chuck! |
WCT 4.0 no longer depends on bower |
My primary concern with removing imagemin is that images account for so much of page weight and I fear folks won't add it back in.. Web Component Tester, on the other hand, could maybe be omitted. It's a big dependency and I'm already making it optional in the yeoman generator. |
It seems that gulp-minify-html https://www.npmjs.com/package/gulp-minify-html has been deprecated in favor of https://www.npmjs.com/package/gulp-htmlmin. I'll do a pull request after christmas if it hasn't been done. |
👌 sgtm |
There's an open PR for this now: A long time ago we tried using htmlmin in generator-polymer and it would do nasty things to the page. Probably need to run through both htmlmin and cssnano again to see if things have improved |
Our project has 847 dependencies loaded: https://gist.github.com/TimvdLippe/a62ee54f8d1541557f550aa9ffb738f7 The gist also includes our package.json and gulpfile.js. Since we are working towards PSK2 I think it is a good time to evaluate all dependencies. In my opinion, switching to The biggest dependency contributors in our project are:
An item still on my TODO list is to look for other image plugins, preferably compressing them too to for example In total, installation on my computer was 2m4.215s. (I do not know if this includes downloading, I just removed TLDR: For PSK2, consider adding back |
I've just downloaded a fresh PSK and it took 30 min on a low bandwidth connection just to install 151 MB (!) of the dependencies. This is way too much for a starter kit. Please slim it down. |
@niutech probably the biggest thing would be removing web-component-tester from both package.json and bower.json. It downloads Selenium which is the majority of that 151 megs i believe |
Right. Among 738 modules in |
Maybe considering an alternative to gulp? The webpack image loader for example seems to be a lot smaller. |
I feel like we conveniently sidestepped this issue by switching to Polymer CLI. Now we only have 1 dependency :P I'm going to float the idea of closing this unless folks object? |
@robdodson SGTM to close this issue. |
@robdodson SGTM! |
npm i
currently installs 1175 dependencies for this project, some of which require native compilation. The install time on my system takes around 8 minutes. I wonder if there is a way we can trim this install down quite a bit.Root tree
from 30 comes many
Full tree
The text was updated successfully, but these errors were encountered: