AG-16392 old comments, todos, package.json

Squashed commit of the following: commit f1204a450aac5eade785d2de98ef39b8ff3dec20 Author: Slava Leleka <[email protected]> Date: Mon Sep 12 02:42:07 2022 +0300 fix exports in package json commit 7849b2e04eadd353112cdbd36903fc3c13ea6b1f Author: Slava Leleka <[email protected]> Date: Mon Sep 12 02:33:47 2022 +0300 add exports to package json commit 4643570a14e0d58310882b8456bf1ec359fad079 Author: Slava Leleka <[email protected]> Date: Mon Sep 12 02:32:18 2022 +0300 build umd instead of cjs commit 58cd1e307cdb27ba1d84134ce21844f0954d99bd Author: Slava Leleka <[email protected]> Date: Mon Sep 12 02:30:24 2022 +0300 fix package json commit 3f67fec9402a71d9561eaa493e4e326fc9fbf019 Author: Slava Leleka <[email protected]> Date: Fri Sep 9 19:17:00 2022 +0300 remove obsolete comment commit 0eeada6ba318e3f6a60fa0fe1cb19f06d4d22a17 Author: Slava Leleka <[email protected]> Date: Fri Sep 9 18:44:16 2022 +0300 update expectToThrowOnStylesheet() commit c1bc4c30a8f9dab2b384d300517b8d2c2e2ebea4 Merge: e10e870 0bdcefe Author: Slava Leleka <[email protected]> Date: Fri Sep 9 18:43:19 2022 +0300 Merge branch 'epic/AG-3532' into fix/AG-16392 commit e10e8706ece6ed3bb4fc1dbc7cafb53b94b7b4fe Author: Slava Leleka <[email protected]> Date: Fri Sep 9 18:25:55 2022 +0300 fail on at-rules while stylesheet parsing commit 9f67e78aadb6f440e5afa5d185ae4d4541bf0175 Author: Slava Leleka <[email protected]> Date: Fri Sep 9 17:41:44 2022 +0300 fix package name commit 7306296993019f55d216d73cfb41ff03e4b02b46 Author: Slava Leleka <[email protected]> Date: Fri Sep 9 17:40:31 2022 +0300 remove few obsolete comments commit 209607238ad41335d75dcc1340dcad7c62519c50 Author: Slava Leleka <[email protected]> Date: Fri Sep 9 17:22:02 2022 +0300 remove todo about scope as we do not support ie commit 759724a1b3798915ecb913f9aaa37d56faad94db Author: Slava Leleka <[email protected]> Date: Fri Sep 9 17:12:18 2022 +0300 fail on nth-ancestor or upward at selector start
AdguardTeam · Sep 12, 2022 · a243fea · a243fea
1 parent 0bdcefe
commit a243fea
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 59 deletions.
diff --git a/README.md b/README.md
@@ -355,7 +355,6 @@ Pseudo-class `:nth-ancestor()` allows to lookup the *nth* ancestor relative to t
 ```
 subject:nth-ancestor(n)
 ```
-<!-- TODO: make `subject` required in code, add tests -->
 - `subject` — required, standard or extended css selector
 - `n` — required, number >= 1 and < 256, distance to the needed ancestor from the element selected by `subject`
 
@@ -390,7 +389,6 @@ Pseudo-class `:upward()` allows to lookup the ancestor relative to the previousl
 ```
 subject:upward(ancestor)
 ```
-<!-- TODO: make `subject` required in code, add tests -->
 - `subject` — required, standard or extended css selector
 - `ancestor` — required, specification for the ancestor of the element selected by `subject`, can be set as:
   - *number* >= 1 and < 256 for distance to the needed ancestor, same as [`:nth-ancestor()`](#extended-css-nth-ancestor)

diff --git a/package.json b/package.json
@@ -1,12 +1,18 @@
 {
-  "name": "extended-css",
+  "name": "@adguard/extended-css",
   "version": "2.0.0",
-  "description": "Module for applying CSS styles with extended selection properties.",
-  "main": "dist/extended-css.cjs.js",
+  "description": "AdGuard's JavaScript library for applying CSS styles with extended selection properties.",
+  "main": "dist/extended-css.umd.js",
   "module": "dist/extended-css.esm.js",
   "typings": "dist/types",
-  "directories": {
-    "test": "test"
+  "files": [
+    "dist"
+  ],
+  "exports": {
+    "./package.json": "./package.json",
+    ".": {
+      "default": "./dist/extended-css.umd.js"
+    }
   },
   "engines": {
     "node": ">=14"

diff --git a/src/selector/parser.ts b/src/selector/parser.ts
@@ -45,6 +45,8 @@ import {
     REMOVE_PSEUDO_MARKER,
     REGULAR_PSEUDO_CLASSES,
     REGULAR_PSEUDO_ELEMENTS,
+    UPWARD_PSEUDO_CLASS_MARKER,
+    NTH_ANCESTOR_PSEUDO_CLASS_MARKER,
 } from '../common/constants';
 
 // limit applying of wildcard :is and :not pseudo-class only to html children
@@ -55,7 +57,6 @@ const IS_OR_NOT_PSEUDO_SELECTING_ROOT = `html ${ASTERISK}`;
 // https://github.com/AdguardTeam/ExtendedCss/issues/115
 const XPATH_PSEUDO_SELECTING_ROOT = 'body';
 
-
 /**
  * Checks whether the passed token is supported extended pseudo-class
  * @param token
@@ -508,6 +509,14 @@ export const parse = (selector: string): AnySelectorNodeInterface => {
                                  * or   :not(span):not(p)
                                  */
                                 initAst(context, IS_OR_NOT_PSEUDO_SELECTING_ROOT);
+                            } else if (nextTokenValue === UPWARD_PSEUDO_CLASS_MARKER
+                                || nextTokenValue === NTH_ANCESTOR_PSEUDO_CLASS_MARKER) {
+                                /**
+                                 * selector should be specified before :nth-ancestor() or :upward()
+                                 * e.g. ':nth-ancestor(3)'
+                                 * or   ':upward(span)'
+                                 */
+                                throw new Error(`Selector should be specified before :${nextTokenValue}() pseudo-class`); // eslint-disable-line max-len
                             } else {
                                 // make it more obvious if selector starts with pseudo with no tag specified
                                 // e.g. ':has(a)' -> '*:has(a)'

diff --git a/src/selector/utils/absolute-matcher.ts b/src/selector/utils/absolute-matcher.ts
@@ -588,10 +588,6 @@ export const isTextMatched = (argsData: MatcherArgsInterface): boolean => {
     const textContent = getNodeTextContent(domElement);
     let isTextContentMatched;
 
-    /**
-     * TODO: consider adding helper for parsing pseudoArg (string or regexp) later,
-     * seems to be similar for few extended pseudo-classes
-     */
     let pseudoArgToMatch = pseudoArg;
 
     if (pseudoArgToMatch.startsWith(SLASH)

diff --git a/src/selector/utils/relative-predicates.ts b/src/selector/utils/relative-predicates.ts
@@ -68,10 +68,6 @@ export const hasRelativesBySelectorList = (argsData: RelativePredicateArgsInterf
                 const elementSelectorText = element.tagName.toLowerCase();
                 specificity = `${COLON}${REGULAR_PSEUDO_CLASSES.SCOPE}${CHILD_COMBINATOR}${elementSelectorText}`;
             } else {
-                /**
-                 * TODO: figure out something with :scope usage as IE does not support it
-                 * https://developer.mozilla.org/en-US/docs/Web/CSS/:scope#browser_compatibility
-                 */
                 /**
                  * :scope specification is needed for proper descendants selection
                  * as native element.querySelectorAll() does not select exact element descendants

diff --git a/src/stylesheet/parser.ts b/src/stylesheet/parser.ts
@@ -12,6 +12,8 @@ import {
     PSEUDO_PROPERTY_POSITIVE_VALUE,
     DEBUG_PSEUDO_PROPERTY_GLOBAL_VALUE,
     STYLESHEET_ERROR_PREFIX,
+    SLASH,
+    ASTERISK,
 } from '../common/constants';
 
 const DEBUG_PSEUDO_PROPERTY_KEY = 'debug';
@@ -20,6 +22,10 @@ const REGEXP_DECLARATION_END = /[;}]/g;
 const REGEXP_DECLARATION_DIVIDER = /[;:}]/g;
 const REGEXP_NON_WHITESPACE = /\S/g;
 
+// ExtendedCss does not support at-rules
+// https://developer.mozilla.org/en-US/docs/Web/CSS/At-rule
+const AT_RULE_MARKER = '@';
+
 interface Style {
     property: string;
     value: string;
@@ -162,6 +168,9 @@ const parseRemoveSelector = (rawSelector: string): ParsedSelectorData => {
  */
 const parseSelectorPart = (context: Context, extCssDoc: ExtCssDocument): SelectorPartData => {
     let selector = context.selectorBuffer.trim();
+    if (selector.startsWith(AT_RULE_MARKER)) {
+        throw new Error(`At-rules are not supported: '${selector}'.`);
+    }
 
     let removeSelectorData: ParsedSelectorData;
     try {
@@ -406,6 +415,10 @@ const saveToRawResults = (rawResults: RawResults, rawRuleData: RawCssRuleData):
  */
 export const parse = (rawStylesheet: string, extCssDoc: ExtCssDocument): ExtCssRuleData[] => {
     const stylesheet = rawStylesheet.trim();
+    if (stylesheet.includes(`${SLASH}${ASTERISK}`) && stylesheet.includes(`${ASTERISK}${SLASH}`)) {
+        throw new Error(`Comments in stylesheet are not supported: '${stylesheet}'`);
+    }
+
     const context: Context = {
         // any stylesheet should start with selector
         isSelector: true,

diff --git a/test/selector/parser.test.ts b/test/selector/parser.test.ts
@@ -417,9 +417,39 @@ describe('relative extended selectors', () => {
             expect(parse(actual)).toEqual(expected);
         });
 
-        /**
-         * TODO: .banner > :has(span, p), a img.ad
-         */
+        it('selector list: has with selector list as arg + regular selector', () => {
+            const actual = '.banner > :has(span, p), a img.ad';
+            const expected = {
+                type: NodeType.SelectorList,
+                children: [
+                    {
+                        type: NodeType.Selector,
+                        children: [
+                            getRegularSelector('.banner > *'),
+                            {
+                                type: NodeType.ExtendedSelector,
+                                children: [
+                                    {
+                                        type: NodeType.RelativePseudoClass,
+                                        name: 'has',
+                                        children: [
+                                            getSelectorListOfRegularSelectors(['span', 'p']),
+                                        ],
+                                    },
+                                ],
+                            },
+                        ],
+                    },
+                    {
+                        type: NodeType.Selector,
+                        children: [
+                            getRegularSelector('a img.ad'),
+                        ],
+                    },
+                ],
+            };
+            expect(parse(actual)).toEqual(expected);
+        });
     });
 
     describe('if-not', () => {
@@ -1169,10 +1199,6 @@ describe('combined selectors', () => {
                 { isRegular: true, value: '::selection' },
             ];
             expectSingleSelectorAstWithAnyChildren({ actual, expected });
-
-            /**
-             * TODO: check this case is selector
-             */
         });
 
         it(':not():not()::selection', () => {
@@ -2132,4 +2158,22 @@ describe('fail on invalid selector', () => {
         ];
         test.each(invalidSelectors)('%s', (selector) => expectToThrowInput({ selector, error }));
     });
+
+    describe('upward with no specified selector before', () => {
+        const error = 'Selector should be specified before :upward() pseudo-class';
+        const invalidSelectors = [
+            ':upward(1)',
+            ':upward(p[class])',
+        ];
+        test.each(invalidSelectors)('%s', (selector) => expectToThrowInput({ selector, error }));
+    });
+
+    describe('nth-ancestor with no specified selector before', () => {
+        const error = 'Selector should be specified before :nth-ancestor() pseudo-class';
+        const invalidSelectors = [
+            ':nth-ancestor(1)',
+            ':nth-ancestor(p[class])',
+        ];
+        test.each(invalidSelectors)('%s', (selector) => expectToThrowInput({ selector, error }));
+    });
 });
diff --git a/test/stylesheet/parser.test.ts b/test/stylesheet/parser.test.ts
@@ -42,18 +42,30 @@ const expectMultipleRulesParsed = (input: MultipleRuleInput): void => {
     });
 };
 
-interface ToThrowInput {
+interface ToThrowOnSelectorInput {
     selector: string,   // selector for extCss querySelectorAll()
     error: string,      // error text to match
 }
-const expectToThrowInput = (input: ToThrowInput): void => {
+const expectToThrowOnSelector = (input: ToThrowOnSelectorInput): void => {
     const { selector, error } = input;
     expect(() => {
         const extCssDoc = new ExtCssDocument();
         parse(selector, extCssDoc);
     }).toThrow(error);
 };
 
+interface ToThrowOnStylesheetInput {
+    stylesheet: string,   // selector for extCss querySelectorAll()
+    error: string,      // error text to match
+}
+const expectToThrowOnStylesheet = (input: ToThrowOnStylesheetInput): void => {
+    const { stylesheet, error } = input;
+    expect(() => {
+        const extCssDoc = new ExtCssDocument();
+        parse(stylesheet, extCssDoc);
+    }).toThrow(error);
+};
+
 describe('stylesheet parser', () => {
     describe('one rule', () => {
         describe('simple selector + one style declaration', () => {
@@ -562,7 +574,7 @@ describe('stylesheet parser', () => {
                 '.block > span:contains({background: #410e13})',
                 '[-ext-matches-css-before=\'content:  /^[A-Z][a-z]{2}\\s/  \']',
             ];
-            test.each(invalidSelectors)('%s', (selector) => expectToThrowInput({ selector, error }));
+            test.each(invalidSelectors)('%s', (selector) => expectToThrowOnSelector({ selector, error }));
         });
 
         describe('invalid remove pseudo-class', () => {
@@ -576,7 +588,7 @@ describe('stylesheet parser', () => {
                 'div:remove(0)',
                 'div:not([class]):remove(invalid)',
             ];
-            test.each(invalidSelectors)('%s', (selector) => expectToThrowInput({ selector, error }));
+            test.each(invalidSelectors)('%s', (selector) => expectToThrowOnSelector({ selector, error }));
         });
 
         describe('invalid style declaration', () => {
@@ -590,7 +602,30 @@ describe('stylesheet parser', () => {
                 { selector: 'div { display: none; visible }', error: STYLESHEET_ERROR_PREFIX.INVALID_STYLE },
                 { selector: 'div { remove }', error: STYLESHEET_ERROR_PREFIX.INVALID_STYLE },
             ];
-            test.each(toThrowInputs)('%s', (input) => expectToThrowInput(input));
+            test.each(toThrowInputs)('%s', (input) => expectToThrowOnSelector(input));
+        });
+
+        it('fail on comments in stylesheet', () => {
+            const stylesheet = `
+                div:not(.header) { display: none; }
+                /* div:not(.header) { padding: 0; } */
+            `;
+            const error = 'Comments in stylesheet are not supported';
+            expectToThrowOnStylesheet({ stylesheet, error });
+        });
+
+        it('fail on media query in stylesheet', () => {
+            const error = 'At-rules are not supported';
+            let stylesheet;
+
+            stylesheet = '@media (max-width: 768px) { body { padding-top: 50px !important; } }';
+            expectToThrowOnStylesheet({ stylesheet, error });
+
+            stylesheet = `
+                div:not(.header) { display: none; }
+                @media (max-width: 768px) { body { padding-top: 50px !important; } }
+            `;
+            expectToThrowOnStylesheet({ stylesheet, error });
         });
     });
 
@@ -622,10 +657,6 @@ describe('stylesheet parser', () => {
         test.each(testsInputs)('%s', (input) => expectSingleRuleParsed(input));
     });
 
-    /**
-     * TODO: remake
-     * do NOT merge styles
-     */
     describe('merge styles for same selectors', () => {
         describe('single rule as result', () => {
             const testsInputs = [
@@ -730,28 +761,4 @@ describe('stylesheet parser', () => {
             test.each(testsInputs)('%s', (input) => expectMultipleRulesParsed(input));
         });
     });
-
-    /**
-     * TODO: add tests for debug pseudo-property --- 'global'
-     */
-
-    /**
-     * TODO: handle multiple rules with same selector and :
-     *
-     * 1 same style declaration -- return unique -- DONE
-     *
-     * 2 different style declaration:
-     *
-     *   - with multiple css style for same property -- no need
-     *     'div { display: none; }\n div { display: block !important; } ' // should be handled by 'important'
-     *     'div { display: none !important }\n div { display: block !important; }' // conflicting
-     *
-     * 3 fail on comments and mediaquery
-     *   - '#$?#h1 { background-color: #fd3332 !important; /* red header * / }  - with no space before last '/'
-     *   - '#$#@media (max-width: 768px) { body { padding-top: 50px !important; } }'
-     */
-
-    /**
-     * TODO: add tests for invalid css rules
-     */
 });
diff --git a/tools/build.ts b/tools/build.ts
@@ -38,8 +38,9 @@ const prodConfig = {
             banner: libOutputBanner,
         },
         {
-            file: `${prodOutputDir}/${LIB_FILE_NAME}.cjs.js`,
-            format: OutputFormat.CJS,
+            // umd is preferred over cjs to avoid variables renaming in tsurlfilter
+            file: `${prodOutputDir}/${LIB_FILE_NAME}.umd.js`,
+            format: OutputFormat.UMD,
             name: LIBRARY_NAME,
             banner: libOutputBanner,
         },

diff --git a/tools/constants.ts b/tools/constants.ts
@@ -1,7 +1,7 @@
 export enum OutputFormat {
     IIFE = 'iife',
     ESM = 'esm',
-    CJS = 'cjs',
+    UMD = 'umd',
 }
 
 export const LIBRARY_NAME = 'ExtendedCSS';