Skip to content

fix selector ordering with pseudo elements #383

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/selector-specificity/test/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ function calculate(selector) {
return selectorSpecificity(selectorAST);
}

assert.deepEqual(calculate(':before'), { a: 0, b: 0, c: 1 });
assert.deepEqual(calculate('::before'), { a: 0, b: 0, c: 1 });

assert.deepEqual(calculate(':focus'), { a: 0, b: 1, c: 0 });
Comment on lines +10 to +13
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package lacked sufficient coverage for pseudo elements vs classes.


assert.deepEqual(calculate(':is(a + a, b + b + b)'), { a: 0, b: 0, c: 3 });
assert.deepEqual(calculate(':is(.a + .a, .b + .b + .b)'), { a: 0, b: 3, c: 0 });
assert.deepEqual(calculate(':is(#a + #a, #b + #b + #b)'), { a: 3, b: 0, c: 0 });
Expand Down
6 changes: 0 additions & 6 deletions plugins/postcss-cascade-layers/.tape.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ postcssTape(plugin)({
basic: {
message: "supports basic usage",
},
'basic:color': {
message: "supports { color: '<a color>' }",
options: {
color: 'purple'
}
},
atrules: {
message: "supports @keyframes usage",
},
Expand Down
5 changes: 5 additions & 0 deletions plugins/postcss-cascade-layers/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changes to PostCSS Cascade Layers

### Unreleased

- Process CSS after most other plugins to ensure correct analysis and transformation of sugary CSS.
- Fix selector order with `:before` and other pseudo elements.

### 1.0.0 (May 12, 2022)

- Initial version
11 changes: 4 additions & 7 deletions plugins/postcss-cascade-layers/src/compound-selector-order.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,18 @@ export function sortCompoundSelector(node) {

node.nodes.sort((a, b) => {
if (a.type === 'selector' && b.type === 'selector' && a.nodes.length && b.nodes.length) {
return selectorTypeOrder(a.nodes[0].value, a.nodes[0].type) - selectorTypeOrder(b.nodes[0].value, b.nodes[0].type);
return selectorTypeOrder(a.nodes[0], a.nodes[0].type) - selectorTypeOrder(b.nodes[0], b.nodes[0].type);
}

if (a.type === 'selector' && a.nodes.length) {
return selectorTypeOrder(a.nodes[0].value, a.nodes[0].type) - selectorTypeOrder(b.value, b.type);
return selectorTypeOrder(a.nodes[0], a.nodes[0].type) - selectorTypeOrder(b, b.type);
}

if (b.type === 'selector' && b.nodes.length) {
return selectorTypeOrder(a.value, a.type) - selectorTypeOrder(b.nodes[0].value, b.nodes[0].type);
return selectorTypeOrder(a, a.type) - selectorTypeOrder(b.nodes[0], b.nodes[0].type);
}

return selectorTypeOrder(a.value, a.type) - selectorTypeOrder(b.value, b.type);
return selectorTypeOrder(a, a.type) - selectorTypeOrder(b, b.type);
});
}

Expand All @@ -67,9 +67,6 @@ function selectorTypeOrder(selector, type) {
return selectorTypeOrderIndex.pseudoElement;
}

if (type === 'pseudo' && selector && selector.indexOf('::') === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectorParser.isPseudoElement(selector) on line 66 expects a Node and this didn't work before because it was a string.

This code on line 70 was insufficient but did work with strings.

Same change in all other plugins with similar logic.

return selectorTypeOrderIndex.pseudoElement;
}
return selectorTypeOrderIndex[type];
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/postcss-cascade-layers/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const creator: PluginCreator<pluginOptions> = (opts?: pluginOptions) => {

return {
postcssPlugin: 'postcss-cascade-layers',
Once(root: Container, { result }: { result: Result }) {
OnceExit(root: Container, { result }: { result: Result }) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see : #374

This change will likely be merged and released before we can add it to preset-env so adding it here as it is an obvious improvement.


// Warnings
if (options.onRevertLayerKeyword) {
Expand Down
12 changes: 0 additions & 12 deletions plugins/postcss-cascade-layers/test/basic.color.expect.css

This file was deleted.

4 changes: 4 additions & 0 deletions plugins/postcss-cascade-layers/test/basic.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@
target {
color: purple;
}

target:before {
color: pink;
}
4 changes: 4 additions & 0 deletions plugins/postcss-cascade-layers/test/basic.expect.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@
target:not(#\##\##\##\##\##\#) {
color: purple;
}

target:not(#\##\##\##\##\##\#):before {
color: pink;
}
4 changes: 4 additions & 0 deletions plugins/postcss-is-pseudo-class/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changes to PostCSS Is Pseudo Class

### Unreleased

- Fix selector order with `:before` and other pseudo elements.

### 2.0.3 (May 11, 2022)

- Use `@csstools/selector-specificity` for specificity calculations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,26 @@ export function sortCompoundSelector(node) {

node.nodes.sort((a, b) => {
if (a.type === 'selector' && b.type === 'selector' && a.nodes.length && b.nodes.length) {
return selectorTypeOrder(a.nodes[0].value, a.nodes[0].type) - selectorTypeOrder(b.nodes[0].value, b.nodes[0].type);
return selectorTypeOrder(a.nodes[0], a.nodes[0].type) - selectorTypeOrder(b.nodes[0], b.nodes[0].type);
}

if (a.type === 'selector' && a.nodes.length) {
return selectorTypeOrder(a.nodes[0].value, a.nodes[0].type) - selectorTypeOrder(b.value, b.type);
return selectorTypeOrder(a.nodes[0], a.nodes[0].type) - selectorTypeOrder(b, b.type);
}

if (b.type === 'selector' && b.nodes.length) {
return selectorTypeOrder(a.value, a.type) - selectorTypeOrder(b.nodes[0].value, b.nodes[0].type);
return selectorTypeOrder(a, a.type) - selectorTypeOrder(b.nodes[0], b.nodes[0].type);
}

return selectorTypeOrder(a.value, a.type) - selectorTypeOrder(b.value, b.type);
return selectorTypeOrder(a, a.type) - selectorTypeOrder(b, b.type);
});
}

function selectorTypeOrder(selector, type) {
if (type === 'pseudo' && selector && selector.indexOf('::') === 0) {
return selectorTypeOrderIndex['pseudoElement'];
if (parser.isPseudoElement(selector)) {
return selectorTypeOrderIndex.pseudoElement;
}

return selectorTypeOrderIndex[type];
}

Expand Down
12 changes: 10 additions & 2 deletions plugins/postcss-is-pseudo-class/test/basic.css
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,18 @@ foo[baz=":is(.some, .other)"], .ok {
order: 26;
}

:is(.bar, .foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi)) {
:is(a, button):is(:hover:before, :focus) {
order: 27;
}

:is(::after, .foo):is(:hover, :focus) {
:is(a, button):is(:hover, :focus):before {
order: 28;
}

:is(.bar, .foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi)) {
order: 29;
}

:is(::after, .foo):is(:hover, :focus) {
order: 30;
}
Original file line number Diff line number Diff line change
Expand Up @@ -326,26 +326,58 @@ button:focus::before {
order: 26;
}

.bar:not(.something-random) {
a:hover:before {
order: 27;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
a:focus:not(something-random) {
order: 27;
}

:not(.something-random):hover::after {
button:hover:before {
order: 27;
}

button:focus:not(something-random) {
order: 27;
}

a:hover:before {
order: 28;
}

:not(.something-random):focus::after {
a:focus:before {
order: 28;
}

.foo:hover {
button:hover:before {
order: 28;
}

.foo:focus {
button:focus:before {
order: 28;
}

.bar:not(.something-random) {
order: 29;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
order: 29;
}

:not(.something-random):hover::after {
order: 30;
}

:not(.something-random):focus::after {
order: 30;
}

.foo:hover {
order: 30;
}

.foo:focus {
order: 30;
}
44 changes: 38 additions & 6 deletions plugins/postcss-is-pseudo-class/test/basic.expect.css
Original file line number Diff line number Diff line change
Expand Up @@ -326,26 +326,58 @@ button:focus::before {
order: 26;
}

.bar:not(.does-not-exist) {
a:hover:before {
order: 27;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
a:focus:not(does-not-exist) {
order: 27;
}

:not(.does-not-exist):hover::after {
button:hover:before {
order: 27;
}

button:focus:not(does-not-exist) {
order: 27;
}

a:hover:before {
order: 28;
}

:not(.does-not-exist):focus::after {
a:focus:before {
order: 28;
}

.foo:hover {
button:hover:before {
order: 28;
}

.foo:focus {
button:focus:before {
order: 28;
}

.bar:not(.does-not-exist) {
order: 29;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
order: 29;
}

:not(.does-not-exist):hover::after {
order: 30;
}

:not(.does-not-exist):focus::after {
order: 30;
}

.foo:hover {
order: 30;
}

.foo:focus {
order: 30;
}
Original file line number Diff line number Diff line change
Expand Up @@ -326,26 +326,58 @@ button:focus::before {
order: 26;
}

.bar:not(.does-not-exist) {
a:hover:before {
order: 27;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
a:focus:not(does-not-exist) {
order: 27;
}

:not(.does-not-exist):hover::after {
button:hover:before {
order: 27;
}

button:focus:not(does-not-exist) {
order: 27;
}

a:hover:before {
order: 28;
}

:not(.does-not-exist):focus::after {
a:focus:before {
order: 28;
}

.foo:hover {
button:hover:before {
order: 28;
}

.foo:focus {
button:focus:before {
order: 28;
}

.bar:not(.does-not-exist) {
order: 29;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
order: 29;
}

:not(.does-not-exist):hover::after {
order: 30;
}

:not(.does-not-exist):focus::after {
order: 30;
}

.foo:hover {
order: 30;
}

.foo:focus {
order: 30;
}
Loading