fix: properly removes all empty sub-selectors based on pseudo-elements

[rmachado-studocu]

What does this PR do

Previously on PR #129, a fix was introduced to fix the cases where a sub-selector is "empty" after the replacements. While the fix works for cases mentioned in the commit:

Example div:is(:hover, .active) would be turned into div:is(, .active) which caused the parser to emit an Empty sub-selector error.

It does not work if you have multiple cases, example:

.Input_input__c1Di2:hover:not(:focus,:focus-within,:disabled,.Input_input--error__JQLeU), becomes .Input_input__c1Di2:not(,,.Input_input--error__JQLeU) (only the first "empty" case is removed)

In fact, if we have in between cases - e.g. .Input_input__c1Di2:hover:not(:focus,.class1, :focus-within,.class2,:disabled,.Input_input--error__JQLeU) adding the repetition to the RegExp still won't work, because it'll only clear the ones in the edges.

This PR attempts to fix this by extracting all params within parenthesis and extract the sub-selectors.

[rmachado-studocu]

/cc @janicklas-ralph

[rmachado-studocu]

I've had a bit of time to think about this process and I wonder if this should be done at all 🤔 ...

I understand that for critical CSS some pseudo-elements / pseudo-subselectors are not important (such as :focus, :hover, etc) because they're interaction related.

However given an HTML:

<div class="myClass">test</div>
<div class="myClass">test</div>
<div class="myClass">test</div>
<!-- intentionally empty -->
<div class="myClass"></div>
<div class="myClass">test</div>

and a rule such as:

.myClass:not(:first-child) {
  color: blue
}

it'll make a critical css as:

.myClass {
  color: blue
}

which will mean that even the first item of the list will have the blue color, resulting in a different CSS than the expected one, no?

Same could be said if I changed the rule to:

.myClass:is(:empty) {
  background: red
}

instead of only the empty divs being red, they'd all be red because the :is(:empty) would be stripped away according to this new code ...

[rmachado-studocu]

Hmmm sorry to bother @janicklas-ralph, @developit ... but can we merge this? Or at least have a discussion about the previous comment...?

We're currently patching critters on our CI / build process to prevent these issues:

LMK if you have any feedback on these.

[alan-agius4]

test

and a rule such as:
```css
.myClass:not(:first-child) {
  color: blue
}

it'll make a critical css as:

.myClass {
  color: blue
}

which will mean that even the first item of the list will have the blue color, resulting in a different CSS than the expected one, no?

No, that is not how this works. The outputted CSS is not altered as you can see by you own test in this PR. The pseudo selectors are removed only to perform faster comparison during extraction.

[rmachado-studocu]

@janicklas-ralph @developit this PR is open for more than 5 months now. Meanwhile we're still patching our CI with our fix. 🤔

Are you still open for contributions?

[rmachado-studocu]

I guess other PRs were more important ... cool ✌🏻