Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide es6 build to use with Webpack 2 #256

Labels
Type: Infrastructure 🏠 Requested changes that won't affect core functionality

Comments

@okonet
Copy link

okonet commented May 2, 2016

I'm using Webpack 2 on https://status.postmarkapp.com and would like to reduce the bundle size of the production bundle. ATM there is no easy way to use webpack's tree-shaking since vicotry code get's transpiled to es5 and commonjs.

In the meantime, is there a way to minimize bundle size with CommonJS?

@ryan-roemer
Copy link
Member

@okonet -- We've already audited victory to make sure there are no multiple export includes (which means there shouldn't be any DCE opportunities via tree shaking) aside from like index.js type stuff. So one option is doing one-off includes in your code (so import the component file paths directly from individual parts of victory instead of victory itself).

However, if you want to build off the es6 source, just use webpacks resolve.alias to point to something like:

alias: {
  victory: require.resolve("victory/src")
}

And now you have ES-next source. From there, just make sure your webpack 2 / babel config has the needed / analogous parts of:

And you're building from source!

( @boygirl -- Might be nice to document this and consider adding a package.json:browser field that points to src instead of lib )

@okonet
Copy link
Author

okonet commented May 2, 2016

I'm a bit concerned about the API here, since doing one-off includes will require lots of changes in my source code.

Right now there is no way I could replace

import { VictoryAxis, VictoryArea } from 'victory'

with something like

import VictoryAxis from 'victory/lib/VictoryAxis'
import VictoryArea from 'victory/lib/VictoryArea'

since victory depends on different packaged with their own structure. So this would require me to write

import VictoryAxis from 'victory/lib/components/VictoryAxis'
import VictoryArea from 'victory/lib/components/VictoryArea'

and this will break in case the internals of the library would change in the future. This is why I'm looking for a more robust way to do so. Ideally without rewriting my code (computers should suffer instead of developers).

More to say, there are different packages for different types of visualizations + some core components that require a deep knowledge of the lib structure to use in an optimal way.

I'm wondering if this is something you have on your radar or you don't consider this an issue.

@boygirl
Copy link
Contributor

boygirl commented May 2, 2016

@okonet

We'll look into better support for building from es6 source. In the meantime, you might be able to shave off some size by including the victory-chart package rather than all of victory if you aren't using things like victory-pie.

@ryan-roemer
Copy link
Member

ryan-roemer commented May 2, 2016

Yep, importing individual packages you use will likely shave off bytes.

And you should be able to build our raw ES sources straight up with a little bit of webpack and Babel work. I look forward to hearing back how that experiment goes if you choose to do so.

@ryan-roemer
Copy link
Member

@okonet -- To help our future planning, what would be the most useful enhancement for you?

Thanks!

@okonet
Copy link
Author

okonet commented May 3, 2016

Thanks for the responses!

@boygirl I'm using the pie as well, but not all charts (only area one). So for me it would make sense to ony import these 2 charts. I've been using animation module but had to remove it from the code until #248 is fixed. So, in order to import these 3 modules ATM I have to install 3 different dependencies and import using internal dir structure, which is not optimal IMO.

This makes me think that in terms of DX, the distribution bundle (even with CommonJS modules) would be a huge improvement over the current state.

For the long(er)-term, I think jsnext:main is the better choice. But for the transition time, both could be supported. Saying that, I could not make webpack 2 work with jsnext:main field and gave up at some point.

@icd2k3
Copy link

icd2k3 commented Oct 11, 2016

I'm running into a similar problem where I only want to use <VictoryPie/>, but no matter what I try the victory-chart is always included in my bundle at 370kb (15% of my total bundle size). Is there any way to only include victory-pie? (I'm using webpack 1.13.2)

@ryan-roemer
Copy link
Member

@icd2k3 -- And you've tried npm install --save victory-pie and import VictoryPie from "victory-pie"?

@icd2k3
Copy link

icd2k3 commented Oct 11, 2016

thanks for the quick response @ryan-roemer! import { VictoryPie } from 'victory-pie' works (needs the brackets, though).

Looks like a lot of lodash is being included too... 182kb, but I can live with that for now.

@ryan-roemer
Copy link
Member

ryan-roemer commented Oct 11, 2016

@boygirl -- I think we might finally be at the point of ticketing out for all victory repos:

  • Upgrade to webpack2
  • Produce es6 lib that is "tree shakeable" in addition to es5 lib

And I can help with all of this work. Let me know if you want me to script stuff with multibot for all the repos too!

@icd2k3
Copy link

icd2k3 commented Oct 12, 2016

Here's a breakdown of the file size imports I'm seeing:

https://cl.ly/2I0Z0B2M2V28

For me, looks like lodash is most of the file size for both victory-core and victory-pie. I'll look into ways to optimize.

Overall been loving Victory - thanks for the hard work!

@okonet
Copy link
Author

okonet commented Oct 12, 2016

I believe lodash bundle size is already optimized as of FormidableLabs/builder-victory-component#59 and FormidableLabs/builder-victory-component#62

@icd2k3
Copy link

icd2k3 commented Oct 12, 2016

Thanks @okonet, you're correct.

I was able to shave off some of that lodash size from my own build by resolving victory-core and victory-pie to just use the same version of lodash.

Everything's looking good now and I'm happy with the import size.

That said, I plan on updating to webpack2 soon and @ryan-roemer's post above sounds interesting.

@okonet
Copy link
Author

okonet commented Oct 12, 2016

I've been using webpack 2 but it does not bring a lot in terms of size until more libraries will export es6 sources.

@ryan-roemer
Copy link
Member

Consider using https://github.com/facebook/react-native/tree/master/babel-preset as the base preset to have consumers need to require themselves in root .babelrc.

Ideally we don't have any webpack dependencies (although lodash-webpack-plugin can be recommended as well as process.env.NODE_ENV === "production" stuff).

/cc @jevakallio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment