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

Display JS and CSS bundle sizes after build #229

Merged
merged 3 commits into from
Jul 27, 2016

Conversation

lvwrence
Copy link
Contributor

@lvwrence lvwrence commented Jul 27, 2016

This aims to fix #228.

We grab the bundle info from webpack stats, check their extensions and print their names filesizes if they match '.js' or '.css'. This ends up looping over the bundle twice but I doubt we care about performance here.

What it looks like:
screen shot 2016-07-26 at 6 25 42 pm

@ghost
Copy link

ghost commented Jul 27, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@@ -9,6 +9,7 @@

process.env.NODE_ENV = 'production';

var filesize = require('filesize');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want gzip-size, actually.

Copy link
Contributor Author

@lvwrence lvwrence Jul 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that the assets have already been gzipped, so I just needed to print out their filesizes? If that's not the case, does this mean we have to read every file to get their estimated gzipped size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I get it now. Gonna get some food and come back to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using gzip-size now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we probably still want filesize for human readable file sizes.

@ghost
Copy link

ghost commented Jul 27, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed label Jul 27, 2016
function logBuildSize(assets, extension) {
for (var i = 0; i < assets.length; i++) {
var asset = assets[i];
if (asset['name'].endsWith(extension)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sup @lvwrence
nit: access the props using dot notation asset.name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol hey! what a coincidence seeing you here. done!

@ghost ghost added the CLA Signed label Jul 27, 2016
@@ -48,6 +58,9 @@ webpack(config).run(function(err, stats) {
console.log(' hs');
console.log(' ' + openCommand + ' http:https://localhost:8080');
console.log();
var assets = stats.toJson()['assets'];
logBuildSize(assets, '.js');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would pass dot free extensions and prepend the dot in logBuildSize.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@lvwrence lvwrence force-pushed the display-gzipped-bundle-sizes branch from a498252 to 52a8605 Compare July 27, 2016 20:32
@ghost ghost added the CLA Signed label Jul 27, 2016
@gaearon
Copy link
Contributor

gaearon commented Jul 27, 2016

When you’re ready please update the screenshot, thanks!

@lvwrence
Copy link
Contributor Author

@gaearon Output looks like this now. Happy to change the formatting/color.

screen shot 2016-07-27 at 2 33 16 pm

@ghost ghost added the CLA Signed label Jul 27, 2016
@gaearon gaearon added this to the 0.2.0 milestone Jul 27, 2016
@ghost ghost added the CLA Signed label Jul 27, 2016
@gaearon gaearon merged commit ca7d227 into facebook:master Jul 27, 2016
@gaearon
Copy link
Contributor

gaearon commented Jul 27, 2016

Thanks! We can tweak minor visual details later.

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display gzipped size for JS and CSS after builds
4 participants