[Bug]: Invalid CSS is produced for :where(a, :not(a)) selectors

[romainmenke]

Describe the bug

Minification of :where with twice the same class, one of which nested in :not has unexpected results

Input :

:where(a, :not(a)) {
  any: style;
}

Output :

:where(a,:not){any:style}

---

Input :

:where(a, :not(b)) {
  any: style;
}

Output :

:where(a,:not(b)){any:style}

Expected behaviour

Expected :not() to always be a pseudo class function after minification.

Steps to reproduce

https://cssnano.co/playground/#eyJpbnB1dCI6Ijp3aGVyZShhLCA6bm90KGEpKSB7XG4gIGFueTogc3R5bGU7XG59IiwiY29uZmlnIjoiY3NzbmFuby1wcmVzZXQtZGVmYXVsdCJ9

Disabling minifySelectors makes the issue go away, so this seems to be the source.

Version

5.0.8

Preset

default

Environment

Any env fails

Package details

+-- cssnano@5.0.8

[ludofischer]

Confirmed, I think it is a bug in postcss-selector-parser the same as #1216. When the arguments of the nested selector is identical to a previous selector, it just disappears from the AST even before cssnano does any processing.

[romainmenke]

Was this reported in postcss-selector-parser?
I could not find a linked issue.

[ludofischer]

Was this reported in postcss-selector-parser? I could not find a linked issue.

I don't think it was reported. I remember I had started trying to reproduce it but never went beyond cloning the repository.

[romainmenke]

When the arguments of the nested selector is identical to a previous selector, it just disappears from the AST even before cssnano does any processing.

I can not reproduce this in the test suite of postcss-selector-parser :

I tried these tests in this file : https://github.com/postcss/postcss-selector-parser/blob/master/src/__tests__/pseudos.js

This passes and no parts of the AST disappear.

test('issue a', ":where(:not(a),:not(a))", (t, tree) => {
    t.deepEqual(tree.toString(), ':where(:not(a),:not(a))');
    t.deepEqual(tree.nodes[0].nodes[0].type, 'pseudo');
    t.deepEqual(tree.nodes[0].nodes[0].value, ':where');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].type, 'pseudo');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].value, ':not');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].type, 'pseudo');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].value, ':not');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].nodes[0].nodes[0].type, 'tag');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].nodes[0].nodes[0].value, 'a');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].nodes[0].nodes[0].type, 'tag');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].nodes[0].nodes[0].value, 'a');
});

test('issue b', ":where(a,:not(a))", (t, tree) => {
    t.deepEqual(tree.toString(), ':where(a,:not(a))');
    t.deepEqual(tree.nodes[0].nodes[0].type, 'pseudo');
    t.deepEqual(tree.nodes[0].nodes[0].value, ':where');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].type, 'tag');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[0].nodes[0].value, 'a');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].type, 'pseudo');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].value, ':not');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].nodes[0].nodes[0].type, 'tag');
    t.deepEqual(tree.nodes[0].nodes[0].nodes[1].nodes[0].nodes[0].nodes[0].value, 'a');
});

[romainmenke]

https://github.com/cssnano/cssnano/blob/master/packages/postcss-minify-selectors/src/index.js#L136-L146

Extremely likely to be caused by this code.

  selector.walk((child) => {
    if (child.type === 'selector') {
      const childStr = String(child);

      if (!uniques.has(childStr)) {
        uniques.add(childStr);
      } else {
        child.remove();
      }
    }
  });

It seems this code is intended to minify this :

foo, foo {

}

/* becomes */

foo {

}

But in reality it does a full tree walk and will remove pieces of selectors as described in these bugs.

Something like this might work better :

  selector.walk((child) => {
    if (child.type === 'selector' && child.parent) {
      const uniques = new Set();
      child.parent.each((sibling) => {
        const siblingStr = String(sibling);

        if (!uniques.has(siblingStr)) {
          uniques.add(siblingStr);
        } else {
          sibling.remove();
        }
      });
    }
  });

[romainmenke]

Confirmed that this is caused by selector minification in CSSNano and I've submitted a PR with the needed fixes.