MariusRumpf / node-lifx

Node.js implementation of the LIFX LAN protocol :bulb:
MIT License
144 stars 28 forks source link

minimize lodash imports. reduces webpack size by half. #69

Closed koush closed 2 years ago

koush commented 5 years ago

Thanks for the great project.

I'm currently using it in a non-node environment with minimal shimming on dgram. Everything works, but the webpack size is quite large. It can be reduced by nearly half by only importing what is necessary from lodash (which fairly large). My particular webpack with these changes went from 880kb to 450kb.

I've also updated it with a more exhaustive list of products from here: https://github.com/futomi/node-lifx-lan/blob/master/lib/products.json

Lodash Doc: https://www.blazemeter.com/blog/the-correct-way-to-import-lodash-libraries-a-benchmark

codecov-io commented 5 years ago

Codecov Report

Merging #69 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #69   +/-   ##
=======================================
  Coverage   60.43%   60.43%           
=======================================
  Files          45       45           
  Lines        1418     1418           
  Branches      226      226           
=======================================
  Hits          857      857           
  Misses        561      561
Impacted Files Coverage Δ
lib/lifx/light.js 77.99% <100%> (ø) :arrow_up:
lib/lifx/packet.js 93.47% <100%> (ø) :arrow_up:
lib/lifx/client.js 88.84% <100%> (ø) :arrow_up:
lib/lifx/packets/stateVersion.js 20% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ffd9df0...ad85de6. Read the comment docs.

ristomatti commented 5 years ago

@koush Nice tweaks! This repo hasn't seen much activity in a long time so don't be disappointed your PR won't make it into the npm package in the near future.

Out of curiosity - what is the use case for reducing the size of a library like this? I'm mot saying smaller wouldn't be better regardless...

koush commented 5 years ago

A typical node environment is a fairly decent desktop or server computer, but I've ported this to duktape, and it will potentially be running on low power embedded solutions. So faster parsing and less overhead is a bit more important than your typical node runtime.

koush commented 5 years ago

@ristomatti Yeah, I'm not averse to forking and publishing my own fork if it comes down to it. I did notice the project is mature (but inactive), so I'll do that if I need to.