directus / v8-archive

Directus Database API — Wraps Custom SQL Databases with a REST/GraphQL API
https://docs.directus.io/api/reference.html
507 stars 203 forks source link

Reduce generated bundle size #2281

Open jooola opened 4 years ago

jooola commented 4 years ago

I'd like to make the app a bit faster. The first thing I proposed is to lazyload the components in the router (see directus/app#2573). Now I'd like to reduce the bundle size.

Using webpack-bundle-analyzer I generated the following schema: DEV: image PROD: image

slug

The first big file is ./node_modules/unicode/category/So.js. It is a dependency of the slug package: https://www.npmjs.com/package/slug

The first question is: Do we really need to support Unicode for the 4 function calls in the project ? image

TinyMCE

TinyMCE also has a nice size, I would recommend to lazy load this dependency from the wysiwyg interface component, so that the library is only loaded when we need it and also split in a separate file.

Maybe this could be done for all the interfaces that rely on external dependencies, like leaflet, codemirror, ...

lodash

Then we have lodash /node_modules/lodash/lodash.js. It seems every lodash functions are imported individually, but we need to find out whether the entire lib is bundled. https://stackoverflow.com/questions/43479464/how-to-import-a-single-lodash-function

rijkvanzanten commented 4 years ago

I'd like to make the app a bit faster.

The total download size of a fresh open of Directus currently with caches turned off is about 3.5 MB. While it's always a good thing to reduce the amount of code shipped in the initial request, I would to point out that there are way more speed improvements to gain in the API side of things that will drastically improve the performance of the application.

I would also like to point out that the initial load time is way less important in a purely back-end admin focused app like this than the load times while using the app. For example, waiting an extra second or two while opening the app for the first time is wait less annoying than having to wait to watch interfaces and other UI elements pop into place sporadically.

While I'm totally down to decrease overall bundle size by optimizing and getting rid of dependencies we might not fully need, I'm a bit hesitant to go down the rabbithole that is micro-optimizations in load order. In Directus, I'm currently more in camp "load everything up front, have the app be super stable and fast after" than camp "make initial load as fast as possible" (@benhaynes)

benhaynes commented 4 years ago

Thanks for the research and feedback, @jooola — very helpful assessment! We're all very excited to optimize the app in general. ❤️

On these specific changes, my opinion is that the priority should be getting the App as fast and responsive as possible during usage... even if at the sight expense of upfront load time. (as Rijk said).

With that key goal in mind, let's see what optimiations are best.

rijkvanzanten commented 4 years ago

Also, the graphs above are a little misleading. The only scripts downloaded on first load are:

script.js
chunk-vendors
app

(script.js is the user custom JS file, empty by default)

image

which together are not nearly half a MB gzipped.

All other little chunks are loaded asynchronously already. Realistically speaking, on initial load on fresh caches, the graph looks more like:

Untitled


That all taken into account, we do see (like you correctly mentioned above) that that Unicode SO file is gigantic. Swapping out slug sounds like a very good first step.

Do we really need to support Unicode for the 4 function calls in the project ?

Yes; someone raised an issue before where things like ß and ü weren't properly handled in the slug interface (this was a while ago in v6). We have to keep in mind that Directus is used by an international audience, and the slug interface is a very often used interface, so it should gracefully handle whatever people throw at it. That being said, I couldn't care less about the slug interface automatically transform things like < to less-than or to recycling

TinyMCE also has a nice size, I would recommend to lazy load this dependency from the wysiwyg interface component, so that the library is only loaded when we need it and also split in a separate file. Maybe this could be done for all the interfaces that rely on external dependencies, like leaflet, codemirror, ...

The interfaces themselves are loaded asynchronously. Loading the imports of those asynchronously imported interfaces asynchronously wouldn't do that much.

Then we have lodash /node_modules/lodash/lodash.js. It is a dependency of the slug package: https://www.npmjs.com/package/slug It seems every lodash functions are imported individually, but we need to find out whether the entire lib is bundled. https://stackoverflow.com/questions/43479464/how-to-import-a-single-lodash-function

Lodash is currently imported as a whole, and injected to window._ in main.js. This is done on purpose to give custom extensions served from the API the ability to use it. I'm down to keep the conversation going on whether or not to remove that from the main app. Keep in mind that removing it from the main app means that people have to redownload and import it in their custom extensions and interfaces, increasing the size of those drastically. I don't have statistics on how often lodash is used in those custom extensions, but it is an important thing to keep in mind.

rijkvanzanten commented 4 years ago

Another also: I really appreciate your enthusiasm and eagerness to jump right into PRs 🙂 Apologies if it felt like I shot down your effort too harshly. Just wanted to make sure you wouldn't spend hours cleaning up legacy code only to learn that things were done certain ways on purpose!

benhaynes commented 4 years ago

Another thought I had (maybe worth discussing) is: could we start start asynchronously loading all the JS, CSS, and other files, directly after the login page initially loads?

I feel like there will always be at least three to five seconds after the login page loads but before the user finishes authenticating. Isn't that a non-obtrusive time we could asnc load the rest of the data?

Is it wrong in some way to load data before someone commits to logging in? 🤔

rijkvanzanten commented 4 years ago

Yeah! That's what I meant by https://github.com/directus/app/issues/2495 too :)

jooola commented 4 years ago

Yes; someone raised an issue before where things like ß and ü weren't properly handled in the slug interface (this was a while ago in v6). We have to keep in mind that Directus is used by an international audience, and the slug interface is a very often used interface, so it should gracefully handle whatever people throw at it. That being said, I couldn't care less about the slug interface automatically transform things like < to less-than or ♲ to recycling

The end of your comment just suggest that we might use another library that don't contains all the fancy emojis. This needs some more research but a replacement could be https://www.npmjs.com/package/slugify

Another also: I really appreciate your enthusiasm and eagerness to jump right into PRs slightly_smiling_face Apologies if it felt like I shot down your effort too harshly. Just wanted to make sure you wouldn't spend hours cleaning up legacy code only to learn that things were done certain ways on purpose!

Don't apologize, it is my mistake not to raise an issue first and directly dive into the code, it a common mistake that needs to be reminded time to time. I simply felt like coding so I don't raised any issue/questions.

Agree on the rest of your answerss, at least this issue tells us the current state of the bundle size which is always good.

rijkvanzanten commented 4 years ago

From a first (arguably dumb / too limited) test, slugify seems to cover the two basics that were a problem before:

image

Based on bundlephobia.com, moving from slug to slugify should decrease our initial load bundle size by 102.4kB (gzipped). That sounds like an easy win @jooola

jooola commented 4 years ago

Based on bundlephobia.com, moving from slug to slugify should decrease our initial load bundle size by 102.4kB (gzipped). That sounds like an easy win @jooola

Haha, yeah, small win. Not sure it compensate the loose of certainty/stability regarding slug creation.

I don't think I have anything to add on this matter, has been a nice try.

everyx commented 4 years ago

@rijkvanzanten @jooola does slugify support CJK string?

Looks like it's not support CJK now, https://github.com/sindresorhus/slugify/issues/8 , maybe we can use it when this feature is done in slugify:

Could we just leave CJK characters unchanged? Like Hello你好 -> hello-你好