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

2 main problems with @critical{} and critical-selector: this; #21

Closed
awacode21 opened this issue Aug 22, 2017 · 8 comments
Closed

2 main problems with @critical{} and critical-selector: this; #21

awacode21 opened this issue Aug 22, 2017 · 8 comments
Labels

Comments

@awacode21
Copy link

awacode21 commented Aug 22, 2017

Dear Zach Green,

There are 2 major problems with this postcss-critical-css plugin, which makes it useless at the moment.

First:

when you use @critical {} around your css classes you get a versioning problem with the following error:
Your current PostCSS version is 5.2.5, but postcss-critical uses 6.0.9. Perhaps this is the source of the error below. Fatal error: Cannot set property 'parent' of undefined

Fact here is, my postcss version is 6.0.9 so the latest one. But it throws this error. I guess it is a weird dependency issue within the plugin? I ensured that it is not only a problem on my machine. I asked two other big web projects which also are working with your plugin and facing the same issue. We all three have completely different code base.

Second:

When using critical-selector: this, and the decision not to minify the output (for example, if you want to do other things with the code before you have it over another tool minified), the critical css classes are written 5 times into the output file. So there is a huge amount of duplicated code.

Your plugin is a great thing and we really would like to use it, but at the moment it is simply not working right.

We would be very, very happy if that can be fixed.

Best Regards, Annick

@zgreen
Copy link
Owner

zgreen commented Aug 22, 2017

Thanks for the bug report, I'll take a look.

@zgreen
Copy link
Owner

zgreen commented Aug 22, 2017

@awacode21 Can you paste your package.json, and the code that calls postcss with postcss-critical-css?

@awacode21
Copy link
Author

awacode21 commented Aug 23, 2017

@zgreen thanks so much for your quick reply. For some reason i guess i got the versioning problem fixed? I updated all occurances of postcss plugins and grunt-postcss and now the first problem seems to work again. What still is not working well is the critical-selector: this selector.
When using it that way:
bildschirmfoto 2017-08-23 um 13 27 23

it ends up in generating:
bildschirmfoto 2017-08-23 um 13 27 35

My package.json is:
bildschirmfoto 2017-08-23 um 13 35 17

and plugin configuration is:
bildschirmfoto 2017-08-23 um 13 36 01

@awacode21
Copy link
Author

awacode21 commented Aug 23, 2017

@zgreen One thing i recognize on console is the message:
bildschirmfoto 2017-08-23 um 13 56 55
i don't know if this is important, but maybe it is somehow related to the problem

@zgreen
Copy link
Owner

zgreen commented Aug 23, 2017

Confirmed there's a bug here. It's specific to setting minify: false, as you noted.

@zgreen zgreen added bug and removed investigating labels Aug 23, 2017
@Teun87
Copy link

Teun87 commented Jan 18, 2018

Any updates about this issue? I would also love to use this solution, but my critical.css suffers under the same (duplicate css) code. It keeps on copying the same code every time when I run it.

@zgreen
Copy link
Owner

zgreen commented Jan 18, 2018

Yeah, I'll see what I can do. Opened a branch with the failing test here: https://github.com/zgreen/postcss-critical-css/tree/21/bug/multiplied-output

@zgreen
Copy link
Owner

zgreen commented Jan 19, 2018

@awacode21 @Teun87 latest release should fix this issue; give it a go if you like. Cheers.

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

No branches or pull requests

3 participants