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

Correctly prints the size of the chunk during the build statistic print #16734

Merged
merged 1 commit into from
Feb 13, 2020

Conversation

EmaGht
Copy link
Contributor

@EmaGht EmaGht commented Jan 22, 2020

This commits allows the cli to correctly print the sum of the sizes of the assets of a given chunk instead of the size of the first emitted asset

Attached:
-Before.PNG: What was happening before the fix in my local environment
-After.PNG: What's happening after the fix, notice that now the size of the "main-client" is the actual sum of the sizes of main-client.js and initial.css
-webpack.config.js: As requested, my current webpack configuration. This problem happens when i'm using the MiniCssExtractPlugin to extract my initial.css together with the main client

Before.PNG
Before

After.PNG
After

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@EmaGht EmaGht changed the title Update stats.ts Correctly prints the size of the chunk during the build statistic print Jan 22, 2020
@EmaGht EmaGht requested a review from clydin January 22, 2020 17:00
@EmaGht
Copy link
Contributor Author

EmaGht commented Jan 22, 2020

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Member

@clydin clydin left a comment

Choose a reason for hiding this comment

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

Source map files (those that end in .map) should be excluded from the total size.

Can you also update the commit message so that it starts with:
fix(@angular-devkit/build-angular):

Copy link
Contributor Author

@EmaGht EmaGht left a comment

Choose a reason for hiding this comment

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

refactored the accumulation function and excluded .map files!

@EmaGht EmaGht requested a review from clydin January 24, 2020 10:48
@clydin
Copy link
Member

clydin commented Jan 28, 2020

Can you squash the commits?
Otherwise, I think this looks good.

@alan-agius4
Copy link
Collaborator

Can you please also reference #16627 in the commit footer?

Ex:

Fixes: #16627 

@EmaGht
Copy link
Contributor Author

EmaGht commented Jan 29, 2020

@clydin First time squashing, hopefully everything went according to plan 👍
@alan-agius4 done!

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Feb 12, 2020
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Feb 12, 2020
@kyliau kyliau merged commit 2f1a9db into angular:master Feb 13, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants