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

Issue with PurgeCSS in production mode #337

Closed
javiereguiluz opened this issue Jun 18, 2018 · 10 comments
Closed

Issue with PurgeCSS in production mode #337

javiereguiluz opened this issue Jun 18, 2018 · 10 comments
Labels

Comments

@javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented Jun 18, 2018

After fixing some errors with PurgeCSS and Encore (#330) now I have another issue.

I use the /* purgecss ignore */, /* purgecss start ignore */ and /* purgecss end ignore */ comments to tell PurgeCSS not to delete some styles (I'm aware of the whitelist and whitelistPatterns options. I use them too, but I can't use them in some cases).

When I run yarn encore dev everything works but when I run yarn encore production those styles are not preserved by PurgeCSS. Maybe the reason is that Encore deletes comments in production and therefore PurgeCSS never sees them? If that's true, what can we do to solve this? Thanks!

@Lyrkan
Copy link
Collaborator

@Lyrkan Lyrkan commented Jun 20, 2018

Hi @javiereguiluz,

I don't know really how that plugin works but I think you may be right about that issue being caused by the removal of comments in production mode...

That's part of the minification process which is triggered by the minimized option of the css-loader:

{
loader: 'css-loader',
options: {
minimize: webpackConfig.isProduction(),
sourceMap: webpackConfig.useSourceMaps,
// when using @import, how many loaders *before* css-loader should
// be applied to those imports? This defaults to 0. When postcss-loader
// is used, we set it to 1, so that postcss-loader is applied
// to @import resources.
importLoaders: usePostCssLoader ? 1 : 0
}

Based on its documentation you can use CSSNano options instead of boolean, so maybe there is a way to minimize CSS files but keep some of the comments.

However... Encore doesn't allow to change these options easily yet. You can still do it by modifying the generated Webpack config object manually but that makes the webpack.config.js file way more complex than it should be. That'll probably be solved once #335 gets merged since it adds an Encore.configureCssLoader(callback) method.

@Lyrkan Lyrkan added the question label Jun 20, 2018
@javiereguiluz
Copy link
Member Author

@javiereguiluz javiereguiluz commented Jun 21, 2018

Thanks for the detailed answer! I don't know anything about the internals of Encore, but the simplest solution to me would be to be able to control the order in which things are executed. If I could run PurgeCSS plugin before Encore's minify plugins, this problem would be solved. Thanks!

@stof
Copy link
Member

@stof stof commented Jun 21, 2018

@javiereguiluz encore does not have a minifying plugin for CSS. The CSS Loader does the minification of CSS.

@javiereguiluz
Copy link
Member Author

@javiereguiluz javiereguiluz commented Jun 21, 2018

@stof as you can see I know nothing about Encore internals 😝 Anyway, can we control the order of things somehow? Thanks!

@stof
Copy link
Member

@stof stof commented Jun 21, 2018

@javiereguiluz yo can control orders of plugins (well, partially, as core plugins don't have priorities allowing to hook in the middle unfortunately, defeating the feature). But that won't help you in this case, as minimizing CSS is not performed by a plugin but by the loader itself.

And as said by @Lyrkan, configuring the CSS loader is not possible easily currently. It will be solved by #335. For now, it would require transforming the generated config (which is probably a pain for this case)

@Lyrkan
Copy link
Collaborator

@Lyrkan Lyrkan commented Jun 21, 2018

@javiereguiluz To extend a bit more on what @stof said:

  • loaders: they basically take some data as an input, processes them and output the result. You can define chains of loaders in Webpack (what Encore does) that are used based on the kind of file being imported into your code. For instance if you import a Sass file, it may go through the sass-loader (which will convert it to CSS), then through the postcss-loader (which will transform it using various plugins), then through the css-loader (which will do multiple things such as resolving URLs, minifying the code, etc.)

  • plugins: they are an entirely different thing. They can use some hooks that Webpack and other loaders/plugins expose to do whatever they want to (clean your build directory, generate a manifest of your assets, display a "build ended" notification, etc.). For instance, the purgecss-webpack-plugin seems to use the additional-assets hook... which is triggered way after all the assets have already been processed by loaders (this is one of the last hooks handled by Webpack).

@javiereguiluz
Copy link
Member Author

@javiereguiluz javiereguiluz commented Jun 21, 2018

@Lyrkan perfectly explained! Thanks.

I wish things worked differently though. For example, instead of minifying CSS contents when loading them, I'd prefer to have a MinifyPlugin() executed as the last step of the whole process, after all the other transformations have taken place. But this is how things are, so let's focus on finding a solution around them instead of trying to change them.

@stof
Copy link
Member

@stof stof commented Jun 21, 2018

Well, as we said, we already have the solution: #335 gives access to configuring the CSS loader, which would allow to configure the options of the minification (allowing to keep comments)

@tomasmcm
Copy link

@tomasmcm tomasmcm commented Jan 4, 2019

I also had this issue with webpack, purgecss and rails. My fix was to change the ignore comments to:
/*! purgecss start ignore */ and /*! purgecss end ignore */. This way they are not removed.

@javiereguiluz
Copy link
Member Author

@javiereguiluz javiereguiluz commented Jan 7, 2019

@tomasmcm thanks! You are right and your solution works and it's the most straightforward one. If anyone needs more control over the CSS loader, #335 is now merged. So, I'm closing this as fixed. Thank you all!

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.