search

develar / 7zip-bin

7-Zip precompiled binaries
MIT License
117 stars 34 forks source link

Be nice to npm #5

Closed tehnorm closed 6 years ago

tehnorm commented 6 years ago

If I'm a developer and I'm working along diligently, everything is going great. Publish my code and it heads off to the CI and Packaging servers. Then the 💩 hits the proverbial fan. Package not found. What. It worked on my machine, the tests pass. WTF mate.

After much digging and indirection you find out that a weird interaction of differences between yarn and npm and package-lock.json are causing an edge case to happen.

If I build my package-lock.json on a different machine then I deploy to it never adds the optional dependencies for the architecture that is now being installed on. Which is crazy, since they are in this package, just not include because of the way the files parameter is designed in the package.json

SIGH

Yes, I know I could use yarn. If I was working by myself that would be a non issue, however in a team of developers - I would sooner be the queen of England before that change happened.

Yes, I know I could nuke my package-lock.json before building and packaging, but that removes the controls that package-lock.json is meant for.

This is an attempted fix to simplify the packaging of this library to allow platform to detection to happen in code - as it was seemingly designed.

One point I'm still confused on is it looks like a few of the binaries are git-lfs file pointers? Are these files large enough to warrant this?

My check is total package size is around 5.5mb with this method.

Also, the top google results for 7zip-bin not found are all variations of this issue.

develar commented 6 years ago

Your PR is not fully correct, but I will merge as and then polish.

Thanks for clear and argumentative comment.

develar commented 6 years ago

The real problem will be App-builder where total size is 7mb for binary.

develar commented 6 years ago

npm notice === Tarball Details === npm notice name: 7zip-bin
npm notice version: 4.0.0
npm notice package size: 3.9 MB
npm notice unpacked size: 8.8 MB

develar commented 6 years ago

npm notice === Tarball Details === npm notice name: app-builder-bin
npm notice version: 1.9.0
npm notice package size: 17.1 MB
npm notice unpacked size: 44.5 MB

tehnorm commented 6 years ago

@develar thank you for your work on this package and electron-builder. We make use of both everyday and find them very useful. Until digging into the code on 7zip-bin I had no idea modules could be structured that way! Very cool!

Apologies if the PR came across too strongly - it wasn't intended that way. Sometimes it's hard to convey the correct tone in writing.