leobudima
commented
7 years ago Fully agree with your sentiments about it being an "issue" that uws is packaging builds into their npm package (we shouldn't adopt the attitude that 9 MB is irrelevant) and compliments on thoroughly researching and covering every aspect of it.
While I think that reporting an issue here can hopefully serve to bring additional attention to the problem and potentially signal the interest of others to uws maintainer, I also do think that socket.io developers / maintainers are better utilized to fix issues directly related to the library...
brenc
leoj3n
ghost
STRML
telemakhos
darrachequesne
Hi, I was doing a check of my node_modules folder last week and came across an issue around how socket.io uses the uws package as a dependency. I noticed that uws is an optional dependency with engine.io as of socket.io 2.0+ release notes and wanted to bring up a few things I noticed about it. I’ll write down how I discovered this, but because its a large post, here’s a TLDR to start with:
node-gyp rebuild > build_log.txt 2>&1 || exit 0Since this redirects stderr to stdout and also forces an exit 0, npm will always think that the installation was successful. This means that even if I do something like break node-gyp by setting the python config to a non-existent python:npm config set python /usr/bin/p && npm install uws, uws will still be installed “successfully” with all the prebuilt binaries.The long post:
To preface, I did bring this up with the uws repo first, as shown here: https://github.com/uNetworking/uWebSockets/issues/626 and here: https://github.com/uNetworking/bindings/pull/30, however the maintainer of the repo, seems to have no intention of fixing or even having a discussion around the issue.
Socket.io 2.x uses uws as an “optional” dependency. When doing some research into why my node_modules folder was ballooning in size, I noticed that uws was the 3rd largest package installed.
Those sizes are in MB. (Before anyone comments on monaco-editor or nyc, I did make github issues/comments for those projects as well).
Checking the dependency tree using npm ls, I found that Socket.io was using uws as a dependency (technically an “optional” dependency, but will cover why its not necessarily optional below).
Wanting to see why uws was producing such a large npm package, I checked its directory:
Above, uws is packaging linux, windows, and other Mac OS X prebuilt binaries into its npm package. uws_darwin_57.node is the package that was built on my own system (using Node.js 8.x).
A couple of issues here: First, uws is packaging unnecessary binaries into its npm package - there’s no reason to have multiple platforms (“process.platform” in node) and multiple module versions (“process.versions.modules” in node) of binaries installed for one machine. This is causing the uws folder to be around 9MB larger than it needs to be. This really shouldn’t be an issue since there are well known solutions to the problem of having prebuilt binaries with npm - specifically use Github releases, AWS S3, or any other storage solution to upload binaries to, then download the appropriate binary at install time. I mentioned this to uws’s maintainer here: https://github.com/uNetworking/uWebSockets/issues/626
Since uws’s maintainer had no intention of fixing the unnecessary binary issue, I decided to make some builds using Travis CI and Appveyor using a fork of the uWebSockets repo. My fork adds an .appveyor.yml and travis.yml along with changing the install script to download from GitHub releases (and auto publishes to NPM on successful build): https://github.com/Flux159/uWebSockets https://www.npmjs.com/package/uws-light https://travis-ci.org/Flux159/uWebSockets/builds https://ci.appveyor.com/project/Flux159/uwebsockets/history Note that I do intend for anyone else to use those builds at the moment - the install script hasn’t been fully tested on all platforms (only OS X) and it was made in a very quick way.
This is where I found two other issues with uws’s npm package - the binaries that Travis CI was building for linux were significantly smaller than those in uws version 0.14.5 (1MB vs 1.5MB). I asked about this in issue 626 and received no response.
The last issue I found was with uws’s package.json install script. It was using this command:
That command will never fail since it is redirecting stderr to stdout and always exiting 0. This means that uws will still be installed “successfully” with all the prebuilt binaries even if node-gyp fails. There’s a partially reasonable reason for this if you’re trying to install uws directly from npm, but when installing engine.io this means that uws will always be installed regardless of whether its being used or not.
Again, I tried to submit a pull request to uws’s maintainer about a fix for the original issue here: https://github.com/uNetworking/bindings/pull/30 yet the pull request was rejected without (in my opinion) sufficient reasoning.
I wanted to open this issue to discuss moving engine.io’s dependency on uws to a different npm package / build due to the issues above (note I don't mean moving off of the C++ based uWebSockets, I mean moving to a different npm build of uws). Whether this is a package maintained by socket.io or someone else, that could be discussed as well (I’m purposely not recommending using my npm package / build in fairness and since I haven’t done thorough testing on it).
Ping @darrachequesne