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

Analyse and optimise bundle size #435

Closed
Reinmar opened this issue May 3, 2017 · 19 comments
Closed

Analyse and optimise bundle size #435

Reinmar opened this issue May 3, 2017 · 19 comments
Labels
status:discussion type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@Reinmar
Copy link
Member

Reinmar commented May 3, 2017

It'd be good to know what affects bundle size. We can use https://www.npmjs.com/package/webpack-bundle-analyzer for that.

@Reinmar Reinmar added status:discussion type:task This issue reports a chore (non-production change) and other types of "todos". labels May 3, 2017
@Reinmar
Copy link
Member Author

Reinmar commented May 3, 2017

I pushed https://github.com/ckeditor/ckeditor5-build-classic/tree/size-analyzer with the analyzer turned on. Just run npm run build-ckeditor to see the stats.

@Reinmar
Copy link
Member Author

Reinmar commented May 3, 2017

Currently, the result is this:

image

Some observations:

  • Lodash takes awfully lot of space even though we use it quite rarely and even though we use the modular build. It'd be good to update Lodash's build, though, because I think they've been working on this. However, I think that Lodash may affect the build size even more than the analyser shows because I think that it doesn't consider module wrappers boilerplate.
  • SVGs are pretty big so it'll be worth to check how we can improve them. Especially that we know that we've done nothing so far. Something like http://sketchmaster.com/svg-optimizer would be nice if it was an npm package.
  • There are no surprises with the rest. Engine is huge, UI lib comes second (but 1/8th of the engine's size) and then there's the rest.

@Reinmar Reinmar changed the title Analyse bundle size Analyse and optimise bundle size Jun 28, 2017
@Reinmar Reinmar mentioned this issue Jun 28, 2017
3 tasks
@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

@djanosik commented on the code size in #438 (comment):

It's about 100 kB (ES5, gzipped). I wonder if it will be possible to make it a bit smaller. 100 kB is not too much, but mobile devices often run on slow connections. See report.

  • Do you need to transpile code to ES5? ES6 version works on all devices which we support and is muuuch smaller. As you can see in https://github.com/ckeditor/ckeditor5-build-classic/blob/master/webpack.config.js we use Babili and we don't transpile the code at all. We will always use only these ES* features which work natively in all supported browsers (with the exception of IE11 if we'll ever support it) so you should be safe with dropping transpilation.
  • We're also considering dropping Lodash because, as the above analysis and Consider dropping lodash #361 suggests, it adds a lot to the code size while we use just a few functions.
  • There's a chance that Webpack 3 (see Upgrade all Webpack usage to Webpack 3 #475) will reduce the code size of our bundles too, but we didn't test it yet. We also saw in the past that Rollup's bundles were smaller than Webpack's and the last time we checked it was possible to bundle CKEditor using Rollup.
  • SVGs contain a lot of garbage still and we're planning to improve them (Clean up the .svg icon files in the project #474).
  • There are other minor code reduction possibilities – e.g. we want to transform CKEditorError() calls to shorter versions (every error contains an id and a message – the plan is to remove the second part and publish errors in the API docs site).

So, you can try some of these methods and we're certainly planning to try the rest at some point. Combined together, they should save some good percentage.

However, there are some limits. CKEditor 5 is a super complex piece of software and, while we always had code size in mind, the main goal is to produce a stable, maintainable, extensible and powerful editor. OT, collaboration, all the abstractions, open and extensible mechanisms don't come cheap but if we dropped them, we would've created just yet another simple editor. We couldn't also remove them from the engine to make loading them optional because they are the core of the engine. Without them you don't have features like undo or some live selection's behaviours. Or some conversion features. Etc. In other words – architecture costs. I don't think that we'll be able to save anything here.

@scottmessinger
Copy link

@Reinmar lodash has an es6 build -- with webpack/rollup, isn't it possible to tree shake and end up only importing the parts of lodash that you're using?

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017 via email

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

Anyway, this needs to be researched now again (the build we use is 1y+ old).

According to what @jdalton tweeted to me (I can't find that tweet, I found this only ;|) we should be using babel-plugin-lodash, which I don't want to use. I don't like exposing dependencies of our dependencies in such a way. Every developer who would be building/integrating CKEditor would need to include that plugin to get reasonable build size).

So, if there's no other way to integrate lodash in an optimised way I'll be voting for removing it completely. Especially, if we'll confirm that it'd reduce the code size a lot.

@djanosik
Copy link

@Reinmar Architecture is what I like the most about CKE5. But I need to support IE11 and that's why I wanted to transpile to ES5. That might be a deal-breaker for now. Nevermind.

  • I think lodash is good for building applications, but not so much for reusable components. I think you should remove it.
  • I've tried to use Webpack 3 with ModuleConcatenation plugin. The size is about 90 kB in my case.

@jdalton
Copy link

jdalton commented Jun 28, 2017

Do your users consume a non-built version of ckeditor?
If not, then using babel-plugin-lodash and lodash-webpack-plugin is fine for generating a dist build. That's what they're designed for.

Something to keep in mind is Lodash kitchen sink is 24kB min+gzip. That's the absolute worst case. Bundle analyzers tend to show raw disk use which is uncompressed dev builds with inline documentation and gives a falsey inflated since of size.

@djanosik
Copy link

@jdalton In my case lodash is 12.13 kB gzipped. It's almost 25 % of ckeditor5-engine (52.55 kB gzipped).

@jdalton
Copy link

jdalton commented Jun 28, 2017

Using lodash-webpack-plugin would reduce that significantly. It removes many heavier bits, like iteratee shorthands which saves ~5 kB alone, allowing you to opt-in to the feature sets you want to use.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

@jdalton, thanks for chiming in.

Do your users consume a non-built version of ckeditor?

Yes and no. CKEditor 5 is both a framework and a set of ready to use builds. The problem is with the framework because:

  • We want to stay as bundler agnostic as possible – it should be possible to integrate CKEditor with as many enviroments as possible. The more bundler dependencies the bigger risk that something will blow up.
  • Maintanance – it'd already require from us some time to add necessary Babel plugins in all places where we build CKEditor. We can't ask the users to do the same... at least not too often so we try to keep the list of dependencies short.
  • Time frame – the code we're working and (from some point in time) its dependencies will stay with us for 10+ years. We must reduce the risk of depending too much on external libraries.

So the modular multi-file ES6 build was ok for us. We have it available in https://github.com/ckeditor/ckeditor5-utils/tree/master/src/lib/lodash and all other packages can use it. No build-time dependencies. Perhaps not the clearest solution, but easy to live with and pretty well optimised.

Something to keep in mind is Lodash kitchen sink is 24kB min+gzip. That's the absolute worst case. Bundle analyzers tend to show raw disk use which is uncompressed dev builds with inline documentation and gives a falsey inflated since of size.

I'm aware of that. I'm also not criticising Lodash for the code size (or anything else) – I know how it is and I'm sure you're constantly thinking about it :).

The thing is, though, that we use ~10-15 functions from Lodash and we use them to a veeery limitted extent. From what I can see even with the optimised modular build we're simply including a lot of code which we may not need. So, I think that at some point we'll check if my assumptions are true. If we're able to save 10kb (of gzipped, minified size) and avoid adding another dependency this will be the way for us to go. Especially, taken the integration issues.

I think lodash is good for building applications, but not so much for reusable components.

Or in other words – one library should not depend on another library.

@jdalton
Copy link

jdalton commented Jun 28, 2017

Or in other words – one library should not depend on another library.

I donno, Async and Redux are built with Lodash and they're doing alright. Lodash does utils well so you're free to get back to building cool things. Which methods are you using? I can audit them and let you know of recommended alternatives.

@Koslun
Copy link

Koslun commented Jun 29, 2017

Though @jdalton can probably offer further optimization strategies, I know I got a modular build by just importing each individual function that I need from the regular lodash package available on npm. Here are some examples functions I'm using in the app:

const map = require('lodash/map');
const find = require('lodash/find');
const forEach = require('lodash/forEach');
const invert = require('lodash/invert');
const pullAt = require('lodash/pullAt');
const reduce = require('lodash/reduce');

Not sure if this would be an improvement at all to what you're using now since I'm not sure what functions you are using compared to my bundle. But looking at BundleAnalyzerPlugin I got Lodash down to 8.3 KB minified + gzipped (28.49 KB without gzip). With another lazy-loaded bundle containing 2 KB minified + gzipped Lodash functions.

Got this in an Angular and TypeScript 2+ project using Webpack 2 with awesome-typescript-loader and UglifyJsPlugin targetting es5.

Regardless I would also consider that Lodash currently is by far the most downloaded npm package, along with Async. So even if you do use some small parts of Lodash it might very likely be that many of your users use some of the same parts of Lodash anyway. This would to a certain extent mitigate the potential savings you could make from custom solutions.

If going the route of using Lodash as a dependency you might want to consider to instead use peer-dependencies, like Angular does with RxJS. As there otherwise could be the potential for a user using a different version of Lodash than this library, leaving two versions of Lodash in a user's project and negating the possible advantage of re-using code.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 29, 2017

@jdalton: Thanks for the info about Async and Redux. We'll check how they integrate lodash.

@Koslun: There are definitely benefits from using lodash (less work for us, possibility to share utils with other packages integrated in someone's app), so if we'll be able to find the right balance for us, we'd be happy to keep lodash. I was also worried about versions mismatch, so we could indeed consider using lodash as a peer dependency.

@anasanzari

This comment has been minimized.

@Reinmar Reinmar added this to the backlog milestone Feb 14, 2019
@jondavidjohn
Copy link

I built a tool to help you analyze webpack bundles for size regressions, and report them directly to GitHub PRs. It's free for open source, so it might be worth checking out and helpful in this scenario.

https://packtracker.io

image

@zamber
Copy link

zamber commented Sep 29, 2020

Hi, not sure where to leave this and I don't want to create another issue for this but I have some advice for lodash.

It's viable to use regular lodash and import functions as usual and have it tree-shakeable by adding this to babel plugins:

        ['transform-imports', {
            lodash: {
                transform: 'lodash/${member}',
                preventFullImport: true,
            },
        }],

It will then transform lines like:

import { merge, reduce } from 'lodash';

to

import { merge } from 'lodash/merge';
import { reduce } from 'lodash/reduce';

Then regular lodash can be added as a peer dependency and essentially the reason for using lodash-es is eliminated. It would be better UX for developers because lodash-es is rarely used anywhere (lodash-es 3 300 000 vs lodash 35 000 000 weekly downloads on npm).

Also lodash-es is not actively maintained. The last update is 1y old and it has known security vulnerabilities (affecting backend apps but explain that to your corporate security checking team...). Their release process sucks https://github.com/lodash/lodash/issues/4879

Currently to get rid of doubled lodash and lodash-es we have to transpile it to get the babel plugin above working along with an alias in webpack resolving lodash-es as lodash. This adds over 20s to our build time just to remove an outdated, less used lodash variation and have a version that's recent.

Before that we used lodash-es but the lack of updates tipped the scales for us. Also we had another dependency using regular lodash so we ended up with 2 versions of it in our project. This marginal gain on bundle size for ckeditor ripples down and makes life harder for developers using it, requiring transpilation of ckeditor sources or inflating bundle size for other people not caring enough to check what lodash variations their dependencies may use.

EDIT:

After checking out your source code I would lean on using regular lodash with specific imports because you don't even bundle or transpile some packages.

It's not used much and i feel that choosing a less used lodash variant that has maintenance issues and then asking users to use it just to get tree-shaking working is wrong.

It's 10x more probable (looking at npm stats) that people will already use regular lodash or have a dependency import it than that someone will dig into the ckeditor5-utils changelog to learn that they should use lodash-es.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 8, 2020

This ticket got seriously outdated.

@Reinmar Reinmar closed this as completed Oct 8, 2020
@Reinmar Reinmar removed this from the backlog milestone Oct 8, 2020
@karanshah229
Copy link

@Reinmar were any other of fixes made ?
Can you provide a closing state on this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:discussion type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

9 participants