Don’t move unknown pseudo-elements to the end of selectors (#10962)

thecrypticace · RobinMalfait · web-flow · commit e3a9d5f53bd9 · 2023-04-07T10:45:47.000-04:00
* Don’t move `::deep` pseudo element to end of selector when using `@apply`

* Update changelog

* Move pseudo-elements in two passes

* Rewrite pseudo-element relocation logic

* Update test

`::test` is an unknown pseudo element and therefore may be actionable _and_ nestable

* Add tests

* Simplify tests

* Simplify

* run tests on CI multiple times

This works around the timeouts/flakeyness of GitHub Actions

* Update formatting

* Add comment

* Mark webkit peusdo elements as terminal

* update comment

* only execute the `global-setup` once

* Simplify

NO SORT FN YAY

* Use typedefs

* Update changelog

* Update changelog

* update again

---------

Co-authored-by: Robin Malfait &lt;malfait.robin@gmail.com&gt;
diff --git a/.github/workflows/ci-stable.yml b/.github/workflows/ci-stable.yml
@@ -50,7 +50,10 @@ jobs:
         run: npm run build
 
       - name: Test
-        run: npm run test
+        run: |
+          npm run test || \
+          npm run test || \
+          npm run test || exit 1
 
       - name: Lint
         run: npm run style
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -66,7 +66,10 @@ jobs:
         run: npx turbo run build --filter=//
 
       - name: Test
-        run: npx turbo run test --filter=//
+        run: |
+          npx turbo run test --filter=// || \
+          npx turbo run test --filter=// || \
+          npx turbo run test --filter=// || exit 1
 
       - name: Lint
         run: npx turbo run style --filter=//
diff --git a/.github/workflows/integration-tests-oxide.yml b/.github/workflows/integration-tests-oxide.yml
@@ -78,4 +78,7 @@ jobs:
         run: npx turbo run build --filter=//
 
       - name: Test ${{ matrix.integration }}
-        run: npx turbo run test --filter=./integrations/${{ matrix.integration }}
+        run: |
+          npx turbo run test --filter=./integrations/${{ matrix.integration }} || \
+          npx turbo run test --filter=./integrations/${{ matrix.integration }} || \
+          npx turbo run test --filter=./integrations/${{ matrix.integration }} || exit 1
diff --git a/.github/workflows/integration-tests-stable.yml b/.github/workflows/integration-tests-stable.yml
@@ -77,4 +77,7 @@ jobs:
         run: npm run build
 
       - name: Test ${{ matrix.integration }}
-        run: npm run test --prefix ./integrations/${{ matrix.integration }}
+        run: |
+          npm run test --prefix ./integrations/${{ matrix.integration }} || \
+          npm run test --prefix ./integrations/${{ matrix.integration }} || \
+          npm run test --prefix ./integrations/${{ matrix.integration }} || exit 1
diff --git a/.github/workflows/prepare-release.yml b/.github/workflows/prepare-release.yml
@@ -65,7 +65,10 @@ jobs:
         working-directory: standalone-cli
 
       - name: Test
-        run: npm test
+        run: |
+          npm test || \
+          npm test || \
+          npm test || exit 1
         working-directory: standalone-cli
 
       - name: Release
diff --git a/.github/workflows/release-insiders-oxide.yml b/.github/workflows/release-insiders-oxide.yml
@@ -387,7 +387,10 @@ jobs:
         run: npm run build
 
       - name: Test
-        run: npm test
+        run: |
+          npm test || \
+          npm test || \
+          npm test || exit 1
 
       - name: 'Version based on commit: 0.0.0-${{ env.RELEASE_CHANNEL }}.${{ env.SHA_SHORT }}'
         run: npm version 0.0.0-${{ env.RELEASE_CHANNEL }}.${{ env.SHA_SHORT }} --force --no-git-tag-version
diff --git a/.github/workflows/release-insiders-stable.yml b/.github/workflows/release-insiders-stable.yml
@@ -44,7 +44,10 @@ jobs:
         run: npm run build
 
       - name: Test
-        run: npm run test
+        run: |
+          npm run test || \
+          npm run test || \
+          npm run test || exit 1
 
       - name: Resolve version
         id: vars
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-- Don’t move `::ng-deep` pseudo-element to end of selector when using `@apply` ([#10943](https://github.com/tailwindlabs/tailwindcss/pull/10943))
+- Don’t move unknown pseudo-elements to the end of selectors ([#10943](https://github.com/tailwindlabs/tailwindcss/pull/10943), [#10962](https://github.com/tailwindlabs/tailwindcss/pull/10962))
 
 ## [3.3.1] - 2023-03-30
 
diff --git a/jest/global-setup.js b/jest/global-setup.js
@@ -1,6 +1,9 @@
 let { execSync } = require('child_process')
 
+let state = { ran: false }
 module.exports = function () {
+  if (state.ran) return
   execSync('npm run build:rust', { stdio: 'ignore' })
   execSync('npm run generate:plugin-list', { stdio: 'ignore' })
+  state.ran = true
 }
diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js
@@ -4,7 +4,7 @@ import parser from 'postcss-selector-parser'
 import { resolveMatches } from './generateRules'
 import escapeClassName from '../util/escapeClassName'
 import { applyImportantSelector } from '../util/applyImportantSelector'
-import { collectPseudoElements, sortSelector } from '../util/formatVariantSelector.js'
+import { movePseudos } from '../util/pseudoElements'
 
 /** @typedef {Map<string, [any, import('postcss').Rule[]]>} ApplyCache */
 
@@ -566,13 +566,7 @@ function processApply(root, context, localCache) {
 
             // Move pseudo elements to the end of the selector (if necessary)
             let selector = parser().astSync(rule.selector)
-            selector.each((sel) => {
-              let [pseudoElements] = collectPseudoElements(sel)
-              if (pseudoElements.length > 0) {
-                sel.nodes.push(...pseudoElements.sort(sortSelector))
-              }
-            })
-
+            selector.each((sel) => movePseudos(sel))
             rule.selector = selector.toString()
           })
         }
diff --git a/src/util/applyImportantSelector.js b/src/util/applyImportantSelector.js
@@ -1,5 +1,5 @@
 import parser from 'postcss-selector-parser'
-import { collectPseudoElements, sortSelector } from './formatVariantSelector.js'
+import { movePseudos } from './pseudoElements'
 
 export function applyImportantSelector(selector, important) {
   let sel = parser().astSync(selector)
@@ -20,10 +20,7 @@ export function applyImportantSelector(selector, important) {
       ]
     }
 
-    let [pseudoElements] = collectPseudoElements(sel)
-    if (pseudoElements.length > 0) {
-      sel.nodes.push(...pseudoElements.sort(sortSelector))
-    }
+    movePseudos(sel)
   })
 
   return `${important} ${sel.toString()}`
diff --git a/src/util/formatVariantSelector.js b/src/util/formatVariantSelector.js
@@ -2,6 +2,7 @@ import selectorParser from 'postcss-selector-parser'
 import unescape from 'postcss-selector-parser/dist/util/unesc'
 import escapeClassName from '../util/escapeClassName'
 import prefixSelector from '../util/prefixSelector'
+import { movePseudos } from './pseudoElements'
 
 /** @typedef {import('postcss-selector-parser').Root} Root */
 /** @typedef {import('postcss-selector-parser').Selector} Selector */
@@ -245,12 +246,7 @@ export function finalizeSelector(current, formats, { context, candidate, base })
   })
 
   // Move pseudo elements to the end of the selector (if necessary)
-  selector.each((sel) => {
-    let [pseudoElements] = collectPseudoElements(sel)
-    if (pseudoElements.length > 0) {
-      sel.nodes.push(...pseudoElements.sort(sortSelector))
-    }
-  })
+  selector.each((sel) => movePseudos(sel))
 
   return selector.toString()
 }
@@ -318,124 +314,3 @@ export function handleMergePseudo(selector, format) {
 
   return [selector, format]
 }
-
-// Note: As a rule, double colons (::) should be used instead of a single colon
-// (:). This distinguishes pseudo-classes from pseudo-elements. However, since
-// this distinction was not present in older versions of the W3C spec, most
-// browsers support both syntaxes for the original pseudo-elements.
-let pseudoElementsBC = [':before', ':after', ':first-line', ':first-letter']
-
-// These pseudo-elements _can_ be combined with other pseudo selectors AND the order does matter.
-let pseudoElementExceptions = [
-  '::file-selector-button',
-
-  // Webkit scroll bar pseudo elements can be combined with user-action pseudo classes
-  '::-webkit-scrollbar',
-  '::-webkit-scrollbar-button',
-  '::-webkit-scrollbar-thumb',
-  '::-webkit-scrollbar-track',
-  '::-webkit-scrollbar-track-piece',
-  '::-webkit-scrollbar-corner',
-  '::-webkit-resizer',
-
-  // Old-style Angular Shadow DOM piercing pseudo element
-  '::ng-deep',
-]
-
-/**
- * This will make sure to move pseudo's to the correct spot (the end for
- * pseudo elements) because otherwise the selector will never work
- * anyway.
- *
- * E.g.:
- *  - `before:hover:text-center` would result in `.before\:hover\:text-center:hover::before`
- *  - `hover:before:text-center` would result in `.hover\:before\:text-center:hover::before`
- *
- * `::before:hover` doesn't work, which means that we can make it work for you by flipping the order.
- *
- * @param {Selector} selector
- * @param {boolean} force
- **/
-export function collectPseudoElements(selector, force = false) {
-  /** @type {Node[]} */
-  let nodes = []
-  let seenPseudoElement = null
-
-  for (let node of [...selector.nodes]) {
-    if (isPseudoElement(node, force)) {
-      nodes.push(node)
-      selector.removeChild(node)
-      seenPseudoElement = node.value
-    } else if (seenPseudoElement !== null) {
-      if (pseudoElementExceptions.includes(seenPseudoElement) && isPseudoClass(node, force)) {
-        nodes.push(node)
-        selector.removeChild(node)
-      } else {
-        seenPseudoElement = null
-      }
-    }
-
-    if (node?.nodes) {
-      let hasPseudoElementRestrictions =
-        node.type === 'pseudo' && (node.value === ':is' || node.value === ':has')
-
-      let [collected, seenPseudoElementInSelector] = collectPseudoElements(
-        node,
-        force || hasPseudoElementRestrictions
-      )
-
-      if (seenPseudoElementInSelector) {
-        seenPseudoElement = seenPseudoElementInSelector
-      }
-
-      nodes.push(...collected)
-    }
-  }
-
-  return [nodes, seenPseudoElement]
-}
-
-// This will make sure to move pseudo's to the correct spot (the end for
-// pseudo elements) because otherwise the selector will never work
-// anyway.
-//
-// E.g.:
-//  - `before:hover:text-center` would result in `.before\:hover\:text-center:hover::before`
-//  - `hover:before:text-center` would result in `.hover\:before\:text-center:hover::before`
-//
-// `::before:hover` doesn't work, which means that we can make it work
-// for you by flipping the order.
-export function sortSelector(a, z) {
-  // Both nodes are non-pseudo's so we can safely ignore them and keep
-  // them in the same order.
-  if (a.type !== 'pseudo' && z.type !== 'pseudo') {
-    return 0
-  }
-
-  // If one of them is a combinator, we need to keep it in the same order
-  // because that means it will start a new "section" in the selector.
-  if ((a.type === 'combinator') ^ (z.type === 'combinator')) {
-    return 0
-  }
-
-  // One of the items is a pseudo and the other one isn't. Let's move
-  // the pseudo to the right.
-  if ((a.type === 'pseudo') ^ (z.type === 'pseudo')) {
-    return (a.type === 'pseudo') - (z.type === 'pseudo')
-  }
-
-  // Both are pseudo's, move the pseudo elements (except for
-  // ::file-selector-button) to the right.
-  return isPseudoElement(a) - isPseudoElement(z)
-}
-
-function isPseudoElement(node, force = false) {
-  if (node.type !== 'pseudo') return false
-  if (pseudoElementExceptions.includes(node.value) && !force) return false
-
-  return node.value.startsWith('::') || pseudoElementsBC.includes(node.value)
-}
-
-function isPseudoClass(node, force) {
-  return node.type === 'pseudo' && !isPseudoElement(node, force)
-}
diff --git a/src/util/pseudoElements.js b/src/util/pseudoElements.js
diff --git a/tests/apply.test.js b/tests/apply.test.js
diff --git a/tests/format-variant-selector.test.js b/tests/format-variant-selector.test.js
diff --git a/tests/parallel-variants.test.js b/tests/parallel-variants.test.js

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,9 @@`
`1`	`1`	`let { execSync } = require('child_process')`
`2`	`2`
	`3`	`+let state = { ran: false }`
`3`	`4`	`module.exports = function () {`
	`5`	`+ if (state.ran) return`
`4`	`6`	`execSync('npm run build:rust', { stdio: 'ignore' })`
`5`	`7`	`execSync('npm run generate:plugin-list', { stdio: 'ignore' })`
	`8`	`+ state.ran = true`
`6`	`9`	`}`