Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

Indebanvdhamer/performance #67

Merged
merged 3 commits into from Aug 31, 2018
Merged

Conversation

indebanvdhamer
Copy link
Member

@indebanvdhamer indebanvdhamer commented Aug 31, 2018

Motivation

Webpack has pretty bad performance when it comes to the scss-loader. This PR improves performance by removing some of the default options, since they're not always needed. They can still be passed via the .webpacker.js file.

Changes

  • Improve (default) performance for the postcss loader by removing postcss-preset-env.
  • Add support for the lodash treeshaking plugin to decrease bundle sizes

@ghost
Copy link

ghost commented Aug 31, 2018

Danger run resulted in 2 warnings; to find out more, see the checks page.

Generated by 🚫 dangerJS

@coveralls
Copy link

Pull Request Test Coverage Report for Build 51

  • 7 of 7 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 75.319%

Totals Coverage Status
Change from base Build 49: 0.1%
Covered Lines: 122
Relevant Lines: 164

💛 - Coveralls

@coveralls
Copy link

coveralls commented Aug 31, 2018

Pull Request Test Coverage Report for Build 60

  • 7 of 7 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 75.319%

Totals Coverage Status
Change from base Build 49: 0.1%
Covered Lines: 122
Relevant Lines: 164

💛 - Coveralls

{
loader: 'sass-loader',
options: {
data: `
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

renanborgez
renanborgez previously approved these changes Aug 31, 2018
@@ -0,0 +1,3 @@
const LodashModuleReplacementPlugin = require('lodash-webpack-plugin');
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure how useful this file is.

Can we do

module.exports = require('lodash-webpack-plugin')

?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could. I was just trying to keep consistent with the file structure of the other plugins. Let's discuss.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that is reasonable.

dicearr
dicearr previously approved these changes Aug 31, 2018
Copy link
Contributor

@dicearr dicearr left a comment

Choose a reason for hiding this comment

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

I think that part of the last commit could have been done with git revert 48e48c2. Other than that 👍

@Amri91
Copy link
Contributor

Amri91 commented Aug 31, 2018

This is a breaking change, should be increase the version in the same merge to master?

Amri91
Amri91 previously approved these changes Aug 31, 2018
@renanborgez
Copy link
Contributor

This is a breaking change, should be increase the version in the same merge to master?

Yes

@indebanvdhamer indebanvdhamer merged commit 9dfa1b7 into master Aug 31, 2018
@indebanvdhamer indebanvdhamer deleted the indebanvdhamer/performance branch August 31, 2018 09:33
@indebanvdhamer indebanvdhamer mentioned this pull request Aug 31, 2018
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.

None yet

6 participants