-
Notifications
You must be signed in to change notification settings - Fork 524
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
Reduce (drastically) bundle size #547
Comments
Hi @thomasthiebaud , First, you'll want to use the Second, tree-shaking from a direct dep on Finally, we are still working a ticket to provide raw es6 sources everywhere to facilitate tree-sharking / es6-ness of the entire dependency tree. In any case, the first two steps you can do now and should dramatically reduce your bundle! |
Perfect, thank you very much |
@ryan-roemer I did the first two steps, but the size remains the same |
Can we see your webpack config and .babelrc? |
Ah, it may be from lodash in resolve: {
alias: {
// Force use of es6 sources instead of es5 lib.
"victory-core": require.resolve("victory-core/src")
}
} I haven't tested this, and it may need a little more finnagling... |
Yes sure, here are my configuration. I tried the alias, but I have some error (missing loader on a webpack.config.js
project.config.js
.babelrc
|
Oh, yeah, you'll need to amend that |
Does anything change if you put the bundle analyzer plug-in last or at least after the lodash plug in?
… On Apr 2, 2017, at 8:18 AM, Thiebaud Thomas ***@***.***> wrote:
I no longer have the error with
include: [
/node_modules\/victory-core/,
/src/,
],
But the bundle looks like the same
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
No, it remains the same |
Let me pull a branch in Also, I'm seeing Final question -- can you check you actual bundle to make sure that the lodash stuff isn't excluded? |
I tried both with Here are more details about my bundle (classical output from webpack)
The chunk 2 jumped from 200kb to 656kb after installing |
I am already using the |
I'm working in here: https://github.com/FormidableLabs/victory-line-experiments It doesn't quite build yet (need some babel hackery still), but perhaps we can isolate a solution here. Also, I'm tweaking my uglify settings to only do DCE but otherwise preserve prettified source and comments to make direct bundle analysis easier. I may not have a chance to finish getting this building, but should be a good start and exercise for us with "just the problem" focus. |
I've also invited you as a contributor on that if you want to branch and hack more easily... Thanks for rolling through this experiment with us! I'm not so much a React guy as a "build guy", and I've been meaning to get more involved in tuning of Victory for production projects, so this is all great! (assuming we figure everything out now 😉 ) |
Thanks, I will have a look tonight or tomorrow |
OK, https://github.com/FormidableLabs/victory-line-experiments now works as expected! The trick was to apply parsing to both The hackery includes: Babel parsing both // Need to resolve to the **directory** of `src`.
var resolveSrc = function (mod) {
return path.join( path.dirname(require.resolve(mod + "/package.json")), "src");
};
// Need to babel process both `victory-core` and `victory-chart` and alias.
var victoryCoreSrc = resolveSrc("victory-core");
var victoryChartSrc = resolveSrc("victory-chart");
// ...
module: {
loaders: [
{
test: /\.js$/,
include: [
path.resolve("src"),
victoryCoreSrc,
victoryChartSrc
],
loader: "babel-loader"
}
]
}, And aliasing both to src: resolve: {
alias: {
"victory-chart": victoryChartSrc,
"victory-core": victoryCoreSrc
}
}, ( Can you run the numbers here and see if everything looks like what you'd want? |
Cool, I will have a look. I played on the |
I've updated https://github.com/FormidableLabs/victory-line-experiments#analysis---one-off-vs-using-index with a now observed bug (most likely in upstream webpack) whereby tree shaking does not completely work. I've opened up #549 to address this. @thomasthiebaud -- My experiment code should mostly take care of the lodash issues because the plugin should now work to remove root lodash, and most of the honing down. It just will miss lodash methods used by upstream components where webpack2 tree shaking is broken. Hopefully there's enough here to give you a start on significantly honing down your own bundle (especially with new |
make sure downsample is working for plotting functions
Summary
I really love this library, but the bundle size is to huge !!! Even I only want to use
VictoryLine
, I need to import the fullvictory
,victory-chart
andvictory-core
library +lodash
.Here are some things to do in order to reduce bundle size, for
victory
,victory-chart
andvictory-core
. I can do a PR for itlodash
moduleReplace
by
victory-core
moduleReplace
by
index.js
when relevantFor example, add an
index.js
file invictory-core/src/victory-transition
socan by replaced by
Details
I'm trying
victory
with this sampleBut the bundle is way to big !!! I used webpack-bundle-analyser to find why.
Here is the graph I have with the current code
As you can see, even if I am only using
VictoryLine
, the fullvictory-core
,victory-chart
andlodash
are included in my bundleSo I tried to directly install
victory-core
andvictory-chart
and change my code sample toUsing this method I reduced the bundle size and only include
VictoryLine
fromvictory-chart
in my bundle. Here is the new graph I getProblem :
lodash
is still fully included. You are using (line 1)(which include the whole
lodash
library) but you only needvictory-core
is still fully included. You are using (line 4-8)(which include the whole
victory-core
library) but you only needThe text was updated successfully, but these errors were encountered: