-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Comments
I pushed https://github.com/ckeditor/ckeditor5-build-classic/tree/size-analyzer with the analyzer turned on. Just run |
Currently, the result is this: Some observations:
|
@djanosik commented on the code size in #438 (comment):
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. |
@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? |
We do exactly that - we use a modular (multiple files) ES6 build (I think that Lodash dropped those builds in the meantime). It should load only these microfunctions which we really use but that's still a lot.
…Sent from my iPhone
On 28 Jun 2017, at 17:58, Scott Messinger ***@***.***> wrote:
@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?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
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. |
@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.
|
Do your users consume a non-built version of ckeditor? 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. |
@jdalton In my case |
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. |
@jdalton, thanks for chiming in.
Yes and no. CKEditor 5 is both a framework and a set of ready to use builds. The problem is with the framework because:
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.
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.
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. |
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 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 Got this in an Angular and TypeScript 2+ project using Webpack 2 with 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. |
@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. |
This comment has been minimized.
This comment has been minimized.
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. |
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:
It will then transform lines like:
to
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. |
This ticket got seriously outdated. |
@Reinmar were any other of fixes made ? |
It'd be good to know what affects bundle size. We can use https://www.npmjs.com/package/webpack-bundle-analyzer for that.
The text was updated successfully, but these errors were encountered: