Skip to content
This repository has been archived by the owner on Apr 8, 2020. It is now read-only.

feat(css minimize): add CSS minification in production build for all templates. #852

Merged
merged 1 commit into from
Apr 11, 2017
Merged

Conversation

etimesg
Copy link
Contributor

@etimesg etimesg commented Apr 9, 2017

As discussed on #844, this PR allows webpack to compress all CSS using the minimize property from the css-loader plugin. It only applies when --env.prod is provided.

Please note this will also compress any css passed through webpack, therefore you should expect a small reduction in size on files other than vendor.css. Out of curiosity, I'm posting a comparison for each template, showing all the affected files their corresponding sizes before and after this change.

Finally, I also would like to highlight the following passage from the css-loader project:

In some cases the minification is destructive to the css, so you can provide some options to it....

Although I couldn't find any errors while testing the templates, I encourage everyone to read the css-loader documentation in case of a css compression error.

  • Angular2Spa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    main-client.js 12.2kB 11.1kB 1.1kB (9.02%)
    main-server.js 150kB 146kB 4kB (2.67%)
    Total 477.2kB 443.1kB 34.1kB (7.15%)
  • AureliaSpa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    app.js 26.4kB 26.1kB 0.3kB (1.14%)
    Total 341.4kB 312.1kB 29.3kB (8.58%)
  • KnockoutSpa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    0.js 1.17kB 1.12kB 0.05kB (4.27%)
    1.js 2.01kB 1.98kB 0.03kB (1.49%)
    2.js 0.577kB 0.563kB 0.014kB (2.43%)
    main.js 5.99kB 5.90kB 0.9kB (1.5%)
    site.css 1.52kB 0.729kB 0.791kB (52%)
    Total 326.2kB 296.2kB 29.9kB (9.19%)
  • ReactReduxSpa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    main-server.js 94.2kB 93kB 1.2kB (1.27%)
    site.css 1.52kB 0.729kB 0.791kB (52%)
    Total 410.7kB 379.7kB 30.9kB (7.55%)
  • ReactSpa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    site.css 1.52kB 0.729kB 0.791kB (52%)
    Total 316.5kB 286.7kB 29.7kB (9.41%)
  • VueSpa Template

    File css-loader css-loader?minimize Diff (%)
    vendor.css 315kB 286kB 29kB (9.21%)
    main.js 21.8kB 21.6kB 0.2kB (0.9%)
    Total 336.8kB 307.6kB 29.2kB (8.67%)

Please, feel free to recommend/perform any necessary modifications.

@dnfclas
Copy link

dnfclas commented Apr 9, 2017

@etimesg,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all .NET Foundation-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

@dnfclas
Copy link

dnfclas commented Apr 9, 2017

@etimesg, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Thanks very much for this! I really appreciate your thoroughness in providing all the stats on the difference this makes.

Just a few minor tweaks and we can get this merged :)

@@ -18,7 +18,7 @@ module.exports = (env) => {
rules: [
{ test: /\.ts$/, include: /ClientApp/, use: ['awesome-typescript-loader?silent=true', 'angular2-template-loader'] },
{ test: /\.html$/, use: 'html-loader?minimize=false' },
{ test: /\.css$/, use: ['to-string-loader', 'css-loader'] },
{ test: /\.css$/, use: [ 'to-string-loader', isDevBuild ? 'css-loader' : 'css-loader?minimize' ] },
Copy link
Member

Choose a reason for hiding this comment

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

In a few cases you've added spaces inside the [...] array, such as in this file, but not in some other cases (e.g., for ReactSpa's webpack.config.js).

Would you be able to make the changes consistent with each other? Personally I'd prefer not having the extra spaces in any of them, but if you prefer them to be there, that's OK as long as they are all the same :) Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Actually don't worry about this. It's a super-minor issue. I'll make a follow-up tweak to the spacing later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @SteveSanderson. In truthiness, the differences in spaces also bothered me, until I realized other parts of the templates were different, so I decide to keep it as it was. But now that I'm aware of your preferences for this project I'll make sure to use "no array spaces" as the standard on future PRs.

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Looks great - thanks! I really appreciate seeing all the stats on the difference it makes :)

@SteveSandersonMS
Copy link
Member

@etimesg In case you got a notification with some other review comments from me, you can ignore it, because I realised my comments were wrong so I deleted them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants