FormidableLabs / builder-victory-component

Builder archetype for Victory components
MIT License
1 stars 6 forks source link

Add babel and webpack lodash plugins #59

Closed okonet closed 8 years ago

okonet commented 8 years ago

This PR added 2 lodash plugins in order to improve DX and reduce the file size.

From now on it's possible to import lodash modules using import { map, filter } from "lodash" syntax instead of cherry-picking them without size penalties.

Also, the distributive file size is reduced by ~15%.

Before:

[builder:proc:start] Command: webpack --bail --config node_modules/builder-victory-component/config/webpack/webpack.config.dev.js --colors
Hash: 6eca23c94c3a283b5c5b
Version: webpack 1.13.0
Time: 3762ms
         Asset     Size  Chunks             Chunk Names
    victory.js   982 kB       0  [emitted]  main
victory.js.map  1.22 MB       0  [emitted]  main
    + 484 hidden modules

After:

[builder:proc:start] Command: webpack --bail --config node_modules/builder-victory-component/config/webpack/webpack.config.dev.js --colors
Hash: 58f87e564ac06112b7b5
Version: webpack 1.13.0
Time: 3723ms
         Asset     Size  Chunks             Chunk Names
    victory.js   836 kB       0  [emitted]  main
victory.js.map  1.03 MB       0  [emitted]  main
    + 364 hidden modules
ryan-roemer commented 8 years ago

@okonet -- The dev build isn't our main target. We'd ideally like min+gz numbers as that's the "real deal". I'm running some, but if you have some right now would be great to add to your description.

okonet commented 8 years ago

The reason I'm posting dev numbers is the fact I can't build current master without errors having this module symlinked. Not sure how to test it otherwise.

ryan-roemer commented 8 years ago

@okonet -- I usually test via the wonderful npm pack which creates a tarball which you can then npm install. I'll put up full shell steps and experiment results in a few minutes.

ryan-roemer commented 8 years ago

Summary

Here are all my numbers:

Size master pr/59 pr/59 *
Dev 983180 836806 837055
Dev + Gz 197954 170474 167686
Min 304265 286025 286268
Min + Gz 81565 75620 75952

*: Remove OccurrenceOrderPlugin from pr/59

Thoughts:

So, I'm good with this from an infrastructure / size standpoint. I'll leave you to @boygirl for a substantive evaluation.

Thanks for this contribution!

Base

$ cd builder-victory-component
$ git checkout master
$ cd ..

$ npm pack builder-victory-component
$ mv builder-victory-component-2.1.3.tgz builder-victory-component-master-2.1.3.tgz

$ npm pack builder-victory-component/dev
$ mv builder-victory-component-dev-2.1.3.tgz builder-victory-component-dev-master-2.1.3.tgz

$ cd victory
$ npm install
$ npm install \
  ../builder-victory-component-master-2.1.3.tgz \
  ../builder-victory-component-dev-master-2.1.3.tgz

$ builder run build-dist

$ cat dist/victory.js | wc -c
  983180
$ cat dist/victory.js | gzip -c | wc -c
  197954
$ cat dist/victory.min.js | wc -c
  304265
$ cat dist/victory.min.js | gzip -c | wc -c
   81565

PR

$ cd builder-victory-component
$ git checkout pr/59
$ cd ..

$ npm pack builder-victory-component
$ mv builder-victory-component-2.1.3.tgz builder-victory-component-pr59-2.1.3.tgz

$ npm pack builder-victory-component/dev
$ mv builder-victory-component-dev-2.1.3.tgz builder-victory-component-dev-pr59-2.1.3.tgz

$ cd victory
$ npm install
$ npm install \
  ../builder-victory-component-pr59-2.1.3.tgz \
  ../builder-victory-component-dev-pr59-2.1.3.tgz

$ builder run build-dist

$ cat dist/victory.js | wc -c
  836806
$ cat dist/victory.js | gzip -c | wc -c
  170474
$ cat dist/victory.min.js | wc -c
  286025
$ cat dist/victory.min.js | gzip -c | wc -c
   75620

Additional experiment: No OccurenceOrderPlugin

$ builder run build-dist

$ cat dist/victory.js | wc -c
  837055
$ cat dist/victory.js | gzip -c | wc -c
  167686
$ cat dist/victory.min.js | wc -c
  286268
$ cat dist/victory.min.js | gzip -c | wc -c
   75952
boygirl commented 8 years ago

@okonet thanks for taking on this task! I will update the lodash imports across victory repos in the course of my work for the upcoming release (Friday).

boygirl commented 8 years ago

: / I just realized that babel-plugin-lodash isn't supported in Node < 0.12. @ryan-roemer thoughts on keeping this change and dropping Node 0.10 from our travis matrix?

ryan-roemer commented 8 years ago

Yeah we can drop 0.10 from Travis. But, this should only affect git installs of victory components, not installs from NPM, so we should perhaps update our docs with that nuanced guideline?

boygirl commented 8 years ago

Will do!