GoogleChromeLabs / critters

🦔 A Webpack plugin to inline your critical CSS and lazy-load the rest.
https://npm.im/critters-webpack-plugin
Apache License 2.0
3.42k stars 108 forks source link

ng build cause warnings with pseudo-selectors in :is/:where #102

Closed alan-agius4 closed 1 year ago

alan-agius4 commented 2 years ago
`ng build` inlines critical css but causes warnings when using pseudo selectors inside a :is or :where. ![image](https://user-images.githubusercontent.com/594745/153625324-e2432406-6b71-40ee-bd65-8d55daaa70f8.png) This is not a breaking bug. Usually this only involves states of elements, which is not necessary for critical css. However, it might exclude a state that is relevant. Maybe you consider this out of scope? ## 🔬 Minimal Reproduction https://github.com/kyubisation/ng-pseudo-selector-inlining Place the following in styles.css: ```css .example:is(:hover, .active) { color: green; } ``` ## 🔥 Exception or Error

⠋ Generating index html...1 rules skipped due to selector errors:
  :where(.example):is(,.active) -> Empty sub-selector
ShiJuuRoku commented 2 years ago

+ combinator seem to produce same warning router-outlet + *

p1ngod commented 2 years ago

same as @ShiJuuRoku: .fixed + * { margin-top: 4rem; } results in .fixed+* -> Cannot read property 'type' of undefined

hiepxanh commented 2 years ago

@developit how to fix it sir? I can make an PR :D

developit commented 2 years ago

@hiepxanh best thing to do would be to check for an update in the css-select npm module, which is what Critters uses to parse selectors.

hiepxanh commented 2 years ago

@developit hello sir, I tried to upgrade css-select to 5.1.0 but does not solve the problem. I just wonder where is the root of problem, so I can find and solve it. can you give me some more clue sir?

developit commented 2 years ago

@hiepxanh if the update doesn't add support for it, the only solution would be to intercept and compile/modify the selector before it gets passed to css-select.

hiepxanh commented 2 years ago

@developit https://github.com/GoogleChromeLabs/critters/blob/ec6e1443a75e915e81f4e4ee423cb149c2ae1f50/packages/critters/src/index.js#L490

there is a line 490 in critters/index.js have push this warning. So I guess this is the filter come from our lib.

image there is 3 part here: 1) if it is before/after, we will use it. I wonder what if I add something like allowStrings: "before|after|is|where" maybe we can solve this? 2) there is a regex here, I guest it use to detect the selector? is that :is and :where not match that selector?

p/s: sorry for my annoying. I just use some CSS lib / CSS framework, and they just use some CSS code which cause this warning. I just try to get rid of it 😃

after that, I can solve #103 #104 #99 I think they have relevant

PowerKiKi commented 1 year ago

@developit, you've been active recently on this issue, and there is a very similar issue with possible solution in https://github.com/GoogleChromeLabs/critters/issues/103#issuecomment-1208236366. Could you have a look and offer guidance on how to move forward please ?

alan-agius4 commented 1 year ago

Closing as this should be solved in 0.17.0