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

* 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 <malfait.robin@gmail.com>

.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

.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=//

.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

.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

.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

.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

.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

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
 

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
 }

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()
           })
         }

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()}`

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)
-}

src/util/pseudoElements.js

@@ -0,0 +1,170 @@
+/** @typedef {import('postcss-selector-parser').Root} Root */
+/** @typedef {import('postcss-selector-parser').Selector} Selector */
+/** @typedef {import('postcss-selector-parser').Pseudo} Pseudo */
+/** @typedef {import('postcss-selector-parser').Node} Node */
+
+// There are some pseudo-elements that may or may not be:
+
+// **Actionable**
+// Zero or more user-action pseudo-classes may be attached to the pseudo-element itself
+// structural-pseudo-classes are NOT allowed but we don't make
+// The spec is not clear on whether this is allowed or not — but in practice it is.
+
+// **Terminal**
+// It MUST be placed at the end of a selector
+//
+// This is the required in the spec. However, some pseudo elements are not "terminal" because
+// they represent a "boundary piercing" that is compiled out by a build step.
+
+// **Jumpable**
+// Any terminal element may "jump" over combinators when moving to the end of the selector
+//
+// This is a backwards-compat quirk of :before and :after variants.
+
+/** @typedef {'terminal' | 'actionable' | 'jumpable'} PseudoProperty */
+
+/** @type {Record<string, PseudoProperty[]>} */
+let elementProperties = {
+  '::after': ['terminal', 'jumpable'],
+  '::backdrop': ['terminal'],
+  '::before': ['terminal', 'jumpable'],
+  '::cue': ['terminal'],
+  '::cue-region': ['terminal'],
+  '::first-letter': ['terminal', 'jumpable'],
+  '::first-line': ['terminal', 'jumpable'],
+  '::grammar-error': ['terminal'],
+  '::marker': ['terminal'],
+  '::part': ['terminal', 'actionable'],
+  '::placeholder': ['terminal'],
+  '::selection': ['terminal'],
+  '::slotted': ['terminal'],
+  '::spelling-error': ['terminal'],
+  '::target-text': ['terminal'],
+
+  // other
+  '::file-selector-button': ['terminal', 'actionable'],
+  '::-webkit-progress-bar': ['terminal', 'actionable'],
+
+  // Webkit scroll bar pseudo elements can be combined with user-action pseudo classes
+  '::-webkit-scrollbar': ['terminal', 'actionable'],
+  '::-webkit-scrollbar-button': ['terminal', 'actionable'],
+  '::-webkit-scrollbar-thumb': ['terminal', 'actionable'],
+  '::-webkit-scrollbar-track': ['terminal', 'actionable'],
+  '::-webkit-scrollbar-track-piece': ['terminal', 'actionable'],
+  '::-webkit-scrollbar-corner': ['terminal', 'actionable'],
+  '::-webkit-resizer': ['terminal', 'actionable'],
+
+  // 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.
+  ':after': ['terminal', 'jumpable'],
+  ':before': ['terminal', 'jumpable'],
+  ':first-letter': ['terminal', 'jumpable'],
+  ':first-line': ['terminal', 'jumpable'],
+
+  // The default value is used when the pseudo-element is not recognized
+  // Because it's not recognized, we don't know if it's terminal or not
+  // So we assume it can't be moved AND can have user-action pseudo classes attached to it
+  __default__: ['actionable'],
+}
+
+/**
+ * @param {Selector} sel
+ * @returns {Selector}
+ */
+export function movePseudos(sel) {
+  let [pseudos] = movablePseudos(sel)
+
+  // Remove all pseudo elements from their respective selectors
+  pseudos.forEach(([sel, pseudo]) => sel.removeChild(pseudo))
+
+  // Re-add them to the end of the selector in the correct order.
+  // This moves terminal pseudo elements to the end of the
+  // selector otherwise the selector will not be valid.
+  //
+  // Examples:
+  //  - `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`
+  //
+  // The selector `::before:hover` does not work but we
+  // can make it work for you by flipping the order.
+  sel.nodes.push(...pseudos.map(([, pseudo]) => pseudo))
+
+  return sel
+}
+
+/** @typedef {[sel: Selector, pseudo: Pseudo, attachedTo: Pseudo | null]} MovablePseudo */
+/** @typedef {[pseudos: MovablePseudo[], lastSeenElement: Pseudo | null]} MovablePseudosResult */
+
+/**
+ * @param {Selector} sel
+ * @returns {MovablePseudosResult}
+ */
+function movablePseudos(sel) {
+  /** @type {MovablePseudo[]} */
+  let buffer = []
+
+  /** @type {Pseudo | null} */
+  let lastSeenElement = null
+
+  for (let node of sel.nodes) {
+    if (node.type === 'combinator') {
+      buffer = buffer.filter(([, node]) => propertiesForPseudo(node).includes('jumpable'))
+      lastSeenElement = null
+    } else if (node.type === 'pseudo') {
+      if (isMovablePseudoElement(node)) {
+        lastSeenElement = node
+        buffer.push([sel, node, null])
+      } else if (lastSeenElement && isAttachablePseudoClass(node, lastSeenElement)) {
+        buffer.push([sel, node, lastSeenElement])
+      } else {
+        lastSeenElement = null
+      }
+
+      for (let sub of node.nodes ?? []) {
+        let [movable, lastSeenElementInSub] = movablePseudos(sub)
+        lastSeenElement = lastSeenElementInSub || lastSeenElement
+        buffer.push(...movable)
+      }
+    }
+  }
+
+  return [buffer, lastSeenElement]
+}
+
+/**
+ * @param {Node} node
+ * @returns {boolean}
+ */
+function isPseudoElement(node) {
+  return node.value.startsWith('::') || elementProperties[node.value] !== undefined
+}
+
+/**
+ * @param {Node} node
+ * @returns {boolean}
+ */
+function isMovablePseudoElement(node) {
+  return isPseudoElement(node) && propertiesForPseudo(node).includes('terminal')
+}
+
+/**
+ * @param {Node} node
+ * @param {Pseudo} pseudo
+ * @returns {boolean}
+ */
+function isAttachablePseudoClass(node, pseudo) {
+  if (node.type !== 'pseudo') return false
+  if (isPseudoElement(node)) return false
+
+  return propertiesForPseudo(pseudo).includes('actionable')
+}
+
+/**
+ * @param {Pseudo} pseudo
+ * @returns {PseudoProperty[]}
+ */
+function propertiesForPseudo(pseudo) {
+  return elementProperties[pseudo.value] ?? elementProperties.__default__
+}

tests/apply.test.js

@@ -2428,7 +2428,7 @@ crosscheck(({ stable, oxide }) => {
     })
   })
 
-  stable.test('::ng-deep pseudo element is left alone', () => {
+  stable.test('::ng-deep, ::deep, ::v-deep pseudo elements are left alone', () => {
     let config = {
       darkMode: 'class',
       content: [
@@ -2442,6 +2442,12 @@ crosscheck(({ stable, oxide }) => {
       ::ng-deep .foo .bar {
         @apply font-bold;
       }
+      ::v-deep .foo .bar {
+        @apply font-bold;
+      }
+      ::deep .foo .bar {
+        @apply font-bold;
+      }
     `
 
     return run(input, config).then((result) => {
@@ -2449,12 +2455,19 @@ crosscheck(({ stable, oxide }) => {
         ::ng-deep .foo .bar {
           font-weight: 700;
         }
+        ::v-deep .foo .bar {
+          font-weight: 700;
+        }
+        ::deep .foo .bar {
+          font-weight: 700;
+        }
       `)
     })
   })
 
   // 1. `::ng-deep` is deprecated
-  // 2. It uses invalid selector syntax that Lightning CSS does not support
+  // 2. `::deep` and `::v-deep` are non-standard
+  // 3. They all use invalid selector syntax that Lightning CSS does not support
   // It may be enough for Oxide to not support it at all
-  oxide.test.todo('::ng-deep pseudo element is left alone')
+  oxide.test.todo('::ng-deep, ::deep, ::v-deep pseudo elements are left alone')
 })

tests/format-variant-selector.test.js

@@ -350,6 +350,8 @@ crosscheck(() => {
       ${':where(&::file-selector-button) :is(h1, h2, h3, h4)'} | ${':where(&::file-selector-button) :is(h1, h2, h3, h4)'}
       ${'#app :is(.dark &::before)'}                           | ${'#app :is(.dark &)::before'}
       ${'#app :is(:is(.dark &)::before)'}                      | ${'#app :is(:is(.dark &))::before'}
+      ${'#app :is(.foo::file-selector-button)'}                | ${'#app :is(.foo)::file-selector-button'}
+      ${'#app :is(.foo::-webkit-progress-bar)'}                | ${'#app :is(.foo)::-webkit-progress-bar'}
     `('should translate "$before" into "$after"', ({ before, after }) => {
       let result = finalizeSelector('.a', [{ format: before, isArbitraryVariant: false }], {
         candidate: 'a',

tests/parallel-variants.test.js

@@ -28,7 +28,7 @@ crosscheck(() => {
         .test\:font-medium ::test {
           font-weight: 500;
         }
-        .hover\:test\:font-black :hover::test {
+        .hover\:test\:font-black ::test:hover {
           font-weight: 900;
         }
         .test\:font-bold::test {
@@ -37,7 +37,7 @@ crosscheck(() => {
         .test\:font-medium::test {
           font-weight: 500;
         }
-        .hover\:test\:font-black:hover::test {
+        .hover\:test\:font-black::test:hover {
           font-weight: 900;
         }
       `)
@@ -77,10 +77,10 @@ crosscheck(() => {
         .test\:font-medium::test {
           font-weight: 500;
         }
-        .hover\:test\:font-black :hover::test {
+        .hover\:test\:font-black ::test:hover {
           font-weight: 900;
         }
-        .hover\:test\:font-black:hover::test {
+        .hover\:test\:font-black::test:hover {
           font-weight: 900;
         }
       `)
@@ -126,10 +126,10 @@ crosscheck(() => {
         .test\:font-medium::test {
           font-weight: 500;
         }
-        .hover\:test\:font-black :hover::test {
+        .hover\:test\:font-black ::test:hover {
           font-weight: 900;
         }
-        .hover\:test\:font-black:hover::test {
+        .hover\:test\:font-black::test:hover {
           font-weight: 900;
         }
       `)