From e3a9d5f53bd9f8aadb0bee7ed9c20ea69bb8fcc3 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 7 Apr 2023 10:45:47 -0400 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20move=20unknown=20pseudo-element?= =?UTF-8?q?s=20to=20the=20end=20of=20selectors=20(#10962)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .github/workflows/ci-stable.yml | 5 +- .github/workflows/ci.yml | 5 +- .github/workflows/integration-tests-oxide.yml | 5 +- .../workflows/integration-tests-stable.yml | 5 +- .github/workflows/prepare-release.yml | 5 +- .github/workflows/release-insiders-oxide.yml | 5 +- .github/workflows/release-insiders-stable.yml | 5 +- CHANGELOG.md | 2 +- jest/global-setup.js | 3 + src/lib/expandApplyAtRules.js | 10 +- src/util/applyImportantSelector.js | 7 +- src/util/formatVariantSelector.js | 129 +------------ src/util/pseudoElements.js | 170 ++++++++++++++++++ tests/apply.test.js | 19 +- tests/format-variant-selector.test.js | 2 + tests/parallel-variants.test.js | 12 +- 16 files changed, 232 insertions(+), 157 deletions(-) create mode 100644 src/util/pseudoElements.js diff --git a/.github/workflows/ci-stable.yml b/.github/workflows/ci-stable.yml index 60d6abb9a0c8..592c6e3e3604 100644 --- 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 index 29b96700c8e8..3e2433d3b7ae 100644 --- 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 index b9b3e513d6f2..9da82b4b873b 100644 --- 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 index 86329d4044bd..b2820719656b 100644 --- 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 index 4ec646df71c6..2ad36d3dd670 100644 --- 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 index 9e1dd9a0edc5..e074d064d0ae 100644 --- 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 index 956473f696c2..f8bb322d412f 100644 --- 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 index ebd2f590cacf..dea972556ff4 100644 --- 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 index 65a8dd4dd32a..b28ac7377b86 100644 --- 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 index c6d53d80e29f..3763809e003e 100644 --- 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} 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 index d85efcad2fd0..ff9ec4f4b343 100644 --- 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 index 8857d8fc55e1..e3016e804779 100644 --- 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 new file mode 100644 index 000000000000..9f0e54c67948 --- /dev/null +++ b/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} */ +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__ +} diff --git a/tests/apply.test.js b/tests/apply.test.js index 15cb4176880d..e4af6ee58a77 100644 --- a/tests/apply.test.js +++ b/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') }) diff --git a/tests/format-variant-selector.test.js b/tests/format-variant-selector.test.js index e8318dab4a76..0cd0b05ffd76 100644 --- a/tests/format-variant-selector.test.js +++ b/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', diff --git a/tests/parallel-variants.test.js b/tests/parallel-variants.test.js index 02e2776b5ecc..a4d4dfc8f1b2 100644 --- a/tests/parallel-variants.test.js +++ b/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; } `)