From 16f77efba69a11fb37a43c83af8e39c1534dffea Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Tue, 22 Jun 2021 09:18:26 +1000 Subject: [PATCH] Fix :where/:is in scoped selectors (#204) --- .changeset/funny-pets-laugh.md | 5 ++ packages/css/package.json | 2 +- packages/css/src/validateSelector.test.ts | 61 +++++++++++++++++ packages/css/src/validateSelector.ts | 81 ++++++++++++----------- yarn.lock | 16 ++--- 5 files changed, 117 insertions(+), 48 deletions(-) create mode 100644 .changeset/funny-pets-laugh.md create mode 100644 packages/css/src/validateSelector.test.ts diff --git a/.changeset/funny-pets-laugh.md b/.changeset/funny-pets-laugh.md new file mode 100644 index 000000000..5566eabf5 --- /dev/null +++ b/.changeset/funny-pets-laugh.md @@ -0,0 +1,5 @@ +--- +'@vanilla-extract/css': patch +--- + +Ensure `:where`/`:is` selectors are supported when validating scoped selectors diff --git a/packages/css/package.json b/packages/css/package.json index 527752b16..f4a4e8ccc 100644 --- a/packages/css/package.json +++ b/packages/css/package.json @@ -36,7 +36,7 @@ "@emotion/hash": "^0.8.0", "@vanilla-extract/private": "^1.0.0", "chalk": "^4.1.1", - "css-selector-parser": "^1.4.1", + "css-what": "^5.0.1", "cssesc": "^3.0.0", "csstype": "^3.0.7", "dedent": "^0.7.0", diff --git a/packages/css/src/validateSelector.test.ts b/packages/css/src/validateSelector.test.ts new file mode 100644 index 000000000..89516b1b3 --- /dev/null +++ b/packages/css/src/validateSelector.test.ts @@ -0,0 +1,61 @@ +import { validateSelector } from './validateSelector'; + +describe('validateSelector', () => { + describe('valid selectors', () => { + const validSelectors = [ + '.target', + '.target, .target', + '.target:hover', + '.target:hover:focus', + '.target:where(:hover, :focus)', + '.target:where(:hover, :focus), .target', + '.target:is(:hover, :focus)', + '.target:hover:focus:not(.a)', + '.target:hover:focus:where(:not(.a, .b))', + '.target:hover:focus:is(:not(.a, .b))', + '.target.a', + '.a.target', + '.a.target.b', + '.a.b.target', + '.a .target', + '.a .target:hover', + '.a > .target', + '.a ~ .target', + '.a + .target', + '.a > .b ~ .target', + '.a > .b + .target:hover', + '.a:where(.b, .c) > .target', + '.a:is(.b, .c) > .target', + '.target, .foo .target', + ]; + + validSelectors.forEach((selector) => + it(selector, () => { + expect(() => validateSelector(selector, 'target')).not.toThrow(); + }), + ); + }); + + describe('invalid selectors', () => { + const invalidSelectors = [ + '.a', + '.target .a', + '.target, .a', + '.a, .target', + '.target, .target, .a', + '.a .target .b', + '.target :hover', + '.a .target :hover', + '.target > .a', + '.target + .a', + '.target ~ .a', + '.target:where(:hover, :focus) .a', + ]; + + invalidSelectors.forEach((selector) => + it(selector, () => { + expect(() => validateSelector(selector, 'target')).toThrow(); + }), + ); + }); +}); diff --git a/packages/css/src/validateSelector.ts b/packages/css/src/validateSelector.ts index 0f72a0b40..0809e8c65 100644 --- a/packages/css/src/validateSelector.ts +++ b/packages/css/src/validateSelector.ts @@ -1,4 +1,4 @@ -import { CssSelectorParser } from 'css-selector-parser'; +import { parse } from 'css-what'; import cssesc from 'cssesc'; import dedent from 'dedent'; @@ -7,12 +7,6 @@ function escapeRegex(string: string) { return string.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); } -const parser = new CssSelectorParser(); -parser.registerSelectorPseudos('has'); -parser.registerNestingOperators('>', '+', '~'); -parser.registerAttrEqualityMods('^', '$', '*', '~'); -parser.enableSubstitutes(); - export const validateSelector = (selector: string, targetClassName: string) => { const replaceTarget = () => { const targetRegex = new RegExp( @@ -22,45 +16,54 @@ export const validateSelector = (selector: string, targetClassName: string) => { return selector.replace(targetRegex, '&'); }; - return selector.split(',').map((selectorPart) => { - let currentRule; + let selectorParts: ReturnType; - try { - const result = parser.parse(selectorPart); + try { + selectorParts = parse(selector); + } catch (err) { + throw new Error(`Invalid selector: ${replaceTarget()}`); + } - if (result.type === 'ruleSet') { - currentRule = result.rule; - } else { - throw new Error(); - } - } catch (err) { - throw new Error(`Invalid selector: ${replaceTarget()}`); - } + selectorParts.forEach((tokens) => { + try { + for (let i = tokens.length - 1; i >= -1; i--) { + if (!tokens[i]) { + throw new Error(); + } - while (currentRule.rule) { - currentRule = currentRule.rule; - } + const token = tokens[i]; - const targetRule = currentRule; + if ( + token.type === 'child' || + token.type === 'parent' || + token.type === 'sibling' || + token.type === 'adjacent' || + token.type === 'descendant' + ) { + throw new Error(); + } - if ( - !Array.isArray(targetRule.classNames) || - !targetRule.classNames.find( - (className: string) => className === targetClassName, - ) - ) { + if ( + token.type === 'attribute' && + token.name === 'class' && + token.value === targetClassName + ) { + return; // Found it + } + } + } catch (err) { throw new Error( dedent` - Invalid selector: ${replaceTarget()} - - Style selectors must target the '&' character (along with any modifiers), e.g. ${'`${parent} &`'} or ${'`${parent} &:hover`'}. - - This is to ensure that each style block only affects the styling of a single class. - - If your selector is targeting another class, you should move it to the style definition for that class, e.g. given we have styles for 'parent' and 'child' elements, instead of adding a selector of ${'`& ${child}`'}) to 'parent', you should add ${'`${parent} &`'} to 'child'). - - If your selector is targeting something global, use the 'globalStyle' function instead, e.g. if you wanted to write ${'`& h1`'}, you should instead write 'globalStyle(${'`${parent} h1`'}, { ... })' - `, + Invalid selector: ${replaceTarget()} + + Style selectors must target the '&' character (along with any modifiers), e.g. ${'`${parent} &`'} or ${'`${parent} &:hover`'}. + + This is to ensure that each style block only affects the styling of a single class. + + If your selector is targeting another class, you should move it to the style definition for that class, e.g. given we have styles for 'parent' and 'child' elements, instead of adding a selector of ${'`& ${child}`'}) to 'parent', you should add ${'`${parent} &`'} to 'child'). + + If your selector is targeting something global, use the 'globalStyle' function instead, e.g. if you wanted to write ${'`& h1`'}, you should instead write 'globalStyle(${'`${parent} h1`'}, { ... })' + `, ); } }); diff --git a/yarn.lock b/yarn.lock index c840101bd..a4f603a0e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3191,7 +3191,7 @@ __metadata: "@types/dedent": ^0.7.0 "@vanilla-extract/private": ^1.0.0 chalk: ^4.1.1 - css-selector-parser: ^1.4.1 + css-what: ^5.0.1 cssesc: ^3.0.0 csstype: ^3.0.7 dedent: ^0.7.0 @@ -5352,13 +5352,6 @@ __metadata: languageName: node linkType: hard -"css-selector-parser@npm:^1.4.1": - version: 1.4.1 - resolution: "css-selector-parser@npm:1.4.1" - checksum: 1f5332e601c9bb402d804b7561dfe067cf50888c62c5c66aa9754b13e29d50d29b1b1e0798cdda7235eac2e83b1320e42f597b0976c893fe182c0f9c7a2dac59 - languageName: node - linkType: hard - "css-unit-converter@npm:^1.1.1": version: 1.1.2 resolution: "css-unit-converter@npm:1.1.2" @@ -5373,6 +5366,13 @@ __metadata: languageName: node linkType: hard +"css-what@npm:^5.0.1": + version: 5.0.1 + resolution: "css-what@npm:5.0.1" + checksum: 051bcda396ef25fbc58f66a0c9b54c3bd11f5b8a9f9cdf138865c3bff029fddb6df8fffb487a079110d691856385769fe4e9345262fabeb7a09783dd6f6a7bc2 + languageName: node + linkType: hard + "css.escape@npm:^1.5.1": version: 1.5.1 resolution: "css.escape@npm:1.5.1"