Generate smaller bundles #94
Conversation
gulp/tasks/build.js
Outdated
| @@ -172,13 +166,13 @@ function compileIndvES2015ModulesToBrowser() { | |||
| }, | |||
| footer: fileName => { | |||
| return isFirebaseApp(fileName) ? ` | |||
| })();` : ` | |||
| })().default;` : ` | |||
There was a problem hiding this comment.
We should probably leave a comment here pointing to babel/babel#2212 for future reference as to why this is needed.
There was a problem hiding this comment.
Thanks for that link, it has a lot of useful info. Adding .default was more of a temporary fix after the changes I made, I want to see if there's a better (proper) way to export/import the namespace.
There was a problem hiding this comment.
Originally I was using babel-plugin-add-module-exports for that reason. If we are going to move away from using the babel-loader then we need to do something to recreate that.
There was a problem hiding this comment.
I added a comment pointing to that url for reference. I finally opted not to use babel-plugin-add-module-exports since it needs CommonJS modules to do its thing, but webpack needs to work with ES2015 modules in order to perform tree shaking.
|
Ok, I'm relatively happy with the results now. Here's the final size diff compared with the current master:
Here's how they compare with the latest published version (4.1.3):
Both These last few days I've also been experimenting with more aggressive settings for UglifyJS's mangling of property names but I've run into so many issues, so I'm setting that approach aside for now. It might be worth revisiting in the future though, unless there's any better options available. P.S.: I didn't mention |
gulp/tasks/build.js
Outdated
| }] | ||
| }, | ||
| plugins: [ | ||
| new CheckerPlugin(), | ||
| new webpack.DefinePlugin({ |
There was a problem hiding this comment.
I diff'ed doing the compression w/ and w/o this. For me I was seeing a little bit better results compiling database without this.
Size Comparison
| File | With webpack.DefinePlugin |
Without webpack.DefinePlugin |
|---|---|---|
| firebase-database.js | 166.19 KB | 166.16 KB |
There was a problem hiding this comment.
Cool, I'll get rid of it then.
| @@ -171,8 +157,9 @@ function compileIndvES2015ModulesToBrowser() { | |||
| `; | |||
| }, | |||
| footer: fileName => { | |||
| // Note: '.default' needed because of https://github.com/babel/babel/issues/2212 | |||
gulp/tasks/build.js
Outdated
| mangle: { | ||
| props: { | ||
| ignore_quoted: true, | ||
| regex: /_$/, |
There was a problem hiding this comment.
Nice find on this, I found that by expanding this regex I was able to get some other small savings. The regex I used included the _ prefix:
i.e.
/_$|^_/I think, once this is in, going through and refactoring property names that don't get exposed (regardless of public or private markings as I know we use some of those for internally public stuff) we can get some more savings.
Size Comparison
| File | Suffix Only Regex | Prefix + Suffix Regex |
|---|---|---|
| firebase-app.js | 16.14 KB (5.61 KB) | 15.7 KB (5.55 KB) |
| firebase-database.js | 166.16 KB (45.55 KB) | 165.8 KB (45.52 KB) |
| firebase-messaging.js | 18.22 KB (5.59 KB) | 18.2 KB (5.59 KB) |
| firebase.js | 371.3 KB (108.1 KB) | 370.49 KB (108.02 KB) |
There was a problem hiding this comment.
Nice! The SDK code only uses _ as a suffix but other imported dependencies must be using it as a prefix too. I'll update it with this change.
|
Let me know before you merge this and I'll squash the commits. |
|
@jsayol no worries we always squash merge so no need to do it manually. |
|
Tests all pass and things are looking fantastic, thanks so much @jsayol! |
|
for my react-app, firebase js takes round 436kb sizes, still too big :(
|
Work in progress, do not merge yet.Edit: Ready for reviewI'm tweaking the build process to generate smaller bundles.
I still want to try a few more things but I'm already seeing some serious improvement.
firebase-database.jsis still not back to pre-TypeScript size but every other bundle is smaller: