Visx: Add module support for tree shaking

Created on 10 Sep 2017  Â·  13Comments  Â·  Source: airbnb/visx

Add support for the "module" field in package.json so that webpack/rollup can do proper tree shaking. If you use @vx currently with Rollup, you will get an error by default due to the only availability being commonjs that it cannot understand, so you have to add all the exports of each sub-library of @vx explicitly in rollup.config.js:

https://github.com/rollup/rollup-plugin-commonjs#custom-named-exports

The result is that if you import one part of a @vx/shape, for example, you have to import its entirety.

I believe this can be as simple as building each library twice: Once as it currently is (default es5 transpiling by babel), and a second build into another folder, such as module, where the modules flag is turned off for babel in the babelrc:

"presets": [["es2015", { "modules": false }], "react", "stage-0"],

https://babeljs.io/docs/plugins/preset-es2015/#options

🏠internal

All 13 comments

Ah yes, i've been putting off getting this going. I'll take a swing at this soon. Will definitely have it sorted out by v1.0.0.

for now you should be able to use absolute paths:

import LinePath from '@vx/shape/build/shapes/LinePath';

The "module" field is nonstandard and should not be used.

"Proper tree-shaking" should not, and does not, require ES Modules; it can work equally well with (static, top-level) CJS - although popular bundlers may not have chosen to build support for that yet.

Using deep paths is always the best practice.

Separately, if you want to follow the actual thing that node will be doing (which is what the ecosystem will also end up doing), you should generate .mjs files alongside all your .js files, and the .mjs files need to use only syntax that's in node v8.5 (or whatever version ends up first landing unflagged ESM support).

We might also want to look into adding pure-module: true to our modules for Webpack 4. See d3 issue for details

@techniq i believe it's now sideEffects: false.

Three shaking won't work using "deep paths" references to individual component modules on the app side unless the library also uses "deep paths" within the library code.

@knoopx tree-shaking isn’t needed when using deep paths (absolute paths - starting with / - are never a good idea)

Yes I was referring to "deep paths". The library should use deep paths too in order to be able to use "deep paths" on the app side, otherwise you get everything bundled even when you are only importing a single module.

For example I'm using only these modules:

import AxisBottom from '@vx/axis/build/axis/AxisBottom'
import AxisLeft from '@vx/axis/build/axis/AxisLeft'
import Group from '@vx/group/build/Group'
import Line from '@vx/shape/build/shapes/Line'
import LinePath from '@vx/shape/build/shapes/LinePath'
import Point from '@vx/point/build/Point'
import scaleLinear from '@vx/scale/build/scales/linear'
import ScaleSVG from '@vx/responsive/build/components/ScaleSVG'

However my bundle contains a bunch more than the ones I'm actually using:

image

And that happens because the library itself is not using "deep paths", nor proper "three shaking" to import modules it depends on:

https://github.com/hshoff/vx/blob/master/packages/vx-shape/src/shapes/LinePath.js#L5

That’s my fault. Will get that fixed. Thanks @knoopx

@hshoff I personally think it would be easier to distribute vx also as a module and let the "tree shaking" do the hard work as this issue suggest. Anyway thanks for taking the "deep path" feedback into account.

Yup, absolutely. I need to update the build tooling.

For me, “tree-shaking helps” is an indication that architecture is sub-par, and that something should be improved.

It seems like if internal usage in vx changes to use deep paths instead of using “manifest exports” - ie, a file that imports a bunch of things and re-exports them - that the bundle size issues will be addressed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

roxanagirdu picture roxanagirdu  Â·  4Comments

elisechant picture elisechant  Â·  3Comments

EfosaSO picture EfosaSO  Â·  4Comments

bernatfortet picture bernatfortet  Â·  4Comments

crackcomm picture crackcomm  Â·  5Comments