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

Use only safe css optimizations #1673

Conversation

hashrocketeer
Copy link

OptimizeCSSAssetsPlugin uses a number of optimizations,
see this list.
The optimization in particular that caused us harm was
the
zindex
optimization which compresses zindex numbers (100, 200 -> 1, 2) for all
css that is processed by webpack. Not all css in a Rails app is
processed by webpack. The asset pipeline can still process stylesheets
in addition to css that is included into the code itself.

Including this css optimization into the framework was a breaking change
for us.

Co-authored-by: Chris Erin chris.erin@gmail.com
Co-authored-by: Ryan Messner rpmessner@gmail.com

OptimizeCSSAssetsPlugin uses a number of optimizations,
see this [list](https://cssnano.co/guides/optimisations).
The optimization in particular that caused us harm was
the
[zindex](https://github.com/cssnano/cssnano/tree/master/packages/postcss-zindex)
optimization which compresses zindex numbers (100, 200 -> 1, 2) for all
css that is processed by webpack.  Not all css in a Rails app is
processed by webpack.  The asset pipeline can still process stylesheets
in addition to css that is included into the code itself.

Including this css optimization into the framework was a breaking change
for us.

Co-authored-by: Chris Erin <chris.erin@gmail.com>
Co-authored-by: Ryan Messner <rpmessner@gmail.com>
@gauravtiwari
Copy link
Member

gauravtiwari commented Aug 29, 2018

Thanks @hashrocketeer

Makes sense, seems like option safe is removed now from the plugin: NMFR/optimize-css-assets-webpack-plugin@784677f

We might have to do something like this:

const safeParser = require('postcss-safe-parser');

new OptimizeCssAssetsPlugin({
        cssProcessorOptions: { 
          parser: safeParser,
          discardComments: {
            removeAll: true
          }
        }
      })

Could you please investigate and update the PR (if you have time)?

@chriserin
Copy link

@gauravtiwari yeah thanks for the update. I've been looking through both cssnano and OptimizeCssAssetsPlugin, but nothing jumps out to me. I'll keep looking, I have seen threads where people talk about zindex being turned off by default in cssnano but that's not the case.

@gauravtiwari
Copy link
Member

@chriserin Hey! have you found anything on this?

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

Successfully merging this pull request may close these issues.

3 participants