Covert removeHiddenElems with types (#1532)

Covered removeHiddenElems plugin and big part of our code
- path parser and stringifier
- style manager

lib/path.js

@@ -1,5 +1,10 @@
 'use strict';
 
+/**
+ * @typedef {import('./types').PathDataItem} PathDataItem
+ * @typedef {import('./types').PathDataCommand} PathDataCommand
+ */
+
 // Based on https://www.w3.org/TR/SVG11/paths.html#PathDataBNF
 
 const argsCountPerCommand = {
@@ -26,14 +31,14 @@ const argsCountPerCommand = {
 };
 
 /**
- * @param {string} c
+ * @type {(c: string) => c is PathDataCommand}
  */
 const isCommand = (c) => {
   return c in argsCountPerCommand;
 };
 
 /**
- * @param {string} c
+ * @type {(c: string) => boolean}
  */
 const isWsp = (c) => {
   const codePoint = c.codePointAt(0);
@@ -46,7 +51,7 @@ const isWsp = (c) => {
 };
 
 /**
- * @param {string} c
+ * @type {(c: string) => boolean}
  */
 const isDigit = (c) => {
   const codePoint = c.codePointAt(0);
@@ -61,9 +66,7 @@ const isDigit = (c) => {
  */
 
 /**
- * @param {string} string
+ * @type {(string: string, cursor: number) => [number, number | null]}
- * @param {number} cursor
- * @return {[number, number | null]}
  */
 const readNumber = (string, cursor) => {
   let i = cursor;
@@ -130,10 +133,16 @@ const readNumber = (string, cursor) => {
 };
 
 /**
- * @param {string} string
+ * @type {(string: string) => Array<PathDataItem>}
  */
 const parsePathData = (string) => {
+  /**
+   * @type {Array<PathDataItem>}
+   */
   const pathData = [];
+  /**
+   * @type {null | PathDataCommand}
+   */
   let command = null;
   let args = /** @type {number[]} */ ([]);
   let argsCount = 0;
@@ -232,15 +241,9 @@ const parsePathData = (string) => {
 exports.parsePathData = parsePathData;
 
 /**
- * @typedef {{
+ * @type {(number: number, precision?: number) => string}
- *   number: number;
- *   precision?: number;
- * }} StringifyNumberOptions
  */
-/**
+const stringifyNumber = (number, precision) => {
- * @param {StringifyNumberOptions} param
- */
-const stringifyNumber = ({ number, precision }) => {
   if (precision != null) {
     const ratio = 10 ** precision;
     number = Math.round(number * ratio) / ratio;
@@ -250,31 +253,22 @@ const stringifyNumber = ({ number, precision }) => {
 };
 
 /**
- * @typedef {{
- *   command: string;
- *   args: number[];
- *   precision?: number;
- *   disableSpaceAfterFlags?: boolean;
- * }} StringifyArgsOptions
- */
-/**
- *
  * Elliptical arc large-arc and sweep flags are rendered with spaces
  * because many non-browser environments are not able to parse such paths
  *
- * @param {StringifyArgsOptions} param
+ * @type {(
+ *   command: string,
+ *   args: number[],
+ *   precision?: number,
+ *   disableSpaceAfterFlags?: boolean
+ * ) => string}
  */
-const stringifyArgs = ({
+const stringifyArgs = (command, args, precision, disableSpaceAfterFlags) => {
-  command,
-  args,
-  precision,
-  disableSpaceAfterFlags,
-}) => {
   let result = '';
   let prev = '';
   for (let i = 0; i < args.length; i += 1) {
     const number = args[i];
-    const numberString = stringifyNumber({ number, precision });
+    const numberString = stringifyNumber(number, precision);
     if (
       disableSpaceAfterFlags &&
       (command === 'A' || command === 'a') &&
@@ -298,21 +292,15 @@ const stringifyArgs = ({
 };
 
 /**
- *
  * @typedef {{
- *   command: string;
+ *   pathData: Array<PathDataItem>;
- *   args: number[];
- * }} Command
- */
-/**
- * @typedef {{
- *   pathData: Command[];
  *   precision?: number;
  *   disableSpaceAfterFlags?: boolean;
  * }} StringifyPathDataOptions
  */
+
 /**
- * @param {StringifyPathDataOptions} param
+ * @type {(options: StringifyPathDataOptions) => string}
  */
 const stringifyPathData = ({ pathData, precision, disableSpaceAfterFlags }) => {
   // combine sequence of the same commands
@@ -322,6 +310,9 @@ const stringifyPathData = ({ pathData, precision, disableSpaceAfterFlags }) => {
     if (i === 0) {
       combined.push({ command, args });
     } else {
+      /**
+       * @type {PathDataItem}
+       */
       const last = combined[combined.length - 1];
       // match leading moveto with following lineto
       if (i === 1) {
@@ -349,8 +340,7 @@ const stringifyPathData = ({ pathData, precision, disableSpaceAfterFlags }) => {
   let result = '';
   for (const { command, args } of combined) {
     result +=
-      command +
+      command + stringifyArgs(command, args, precision, disableSpaceAfterFlags);
-      stringifyArgs({ command, args, precision, disableSpaceAfterFlags });
   }
   return result;
 };

lib/style.js

@@ -1,19 +1,40 @@
 'use strict';
 
+/**
+ * @typedef {import('css-tree').Rule} CsstreeRule
+ * @typedef {import('./types').Specificity} Specificity
+ * @typedef {import('./types').StylesheetRule} StylesheetRule
+ * @typedef {import('./types').StylesheetDeclaration} StylesheetDeclaration
+ * @typedef {import('./types').ComputedStyles} ComputedStyles
+ * @typedef {import('./types').XastRoot} XastRoot
+ * @typedef {import('./types').XastElement} XastElement
+ * @typedef {import('./types').XastParent} XastParent
+ * @typedef {import('./types').XastChild} XastChild
+ */
+
 const stable = require('stable');
 const csstree = require('css-tree');
+// @ts-ignore not defined in @types/csso
 const specificity = require('csso/lib/restructure/prepare/specificity');
 const { visit, matches } = require('./xast.js');
-const { compareSpecificity } = require('./css-tools.js');
 const {
   attrsGroups,
   inheritableAttrs,
   presentationNonInheritableGroupAttrs,
 } = require('../plugins/_collections.js');
 
+// @ts-ignore not defined in @types/csstree
+const csstreeWalkSkip = csstree.walk.skip;
+
+/**
+ * @type {(ruleNode: CsstreeRule, dynamic: boolean) => StylesheetRule}
+ */
 const parseRule = (ruleNode, dynamic) => {
   let selectors;
   let selectorsSpecificity;
+  /**
+   * @type {Array<StylesheetDeclaration>}
+   */
   const declarations = [];
   csstree.walk(ruleNode, (cssNode) => {
     if (cssNode.type === 'SelectorList') {
@@ -27,17 +48,20 @@ const parseRule = (ruleNode, dynamic) => {
         }
       });
       selectors = csstree.generate(newSelectorsNode);
-      return csstree.walk.skip;
+      return csstreeWalkSkip;
     }
     if (cssNode.type === 'Declaration') {
       declarations.push({
         name: cssNode.property,
         value: csstree.generate(cssNode.value),
-        important: cssNode.important,
+        important: cssNode.important === true,
       });
-      return csstree.walk.skip;
+      return csstreeWalkSkip;
     }
   });
+  if (selectors == null || selectorsSpecificity == null) {
+    throw Error('assert');
+  }
   return {
     dynamic,
     selectors,
@@ -46,31 +70,43 @@ const parseRule = (ruleNode, dynamic) => {
   };
 };
 
+/**
+ * @type {(css: string, dynamic: boolean) => Array<StylesheetRule>}
+ */
 const parseStylesheet = (css, dynamic) => {
+  /**
+   * @type {Array<StylesheetRule>}
+   */
   const rules = [];
   const ast = csstree.parse(css);
   csstree.walk(ast, (cssNode) => {
     if (cssNode.type === 'Rule') {
       rules.push(parseRule(cssNode, dynamic || false));
-      return csstree.walk.skip;
+      return csstreeWalkSkip;
     }
     if (cssNode.type === 'Atrule') {
       if (cssNode.name === 'keyframes') {
-        return csstree.walk.skip;
+        return csstreeWalkSkip;
       }
       csstree.walk(cssNode, (ruleNode) => {
         if (ruleNode.type === 'Rule') {
           rules.push(parseRule(ruleNode, dynamic || true));
-          return csstree.walk.skip;
+          return csstreeWalkSkip;
         }
       });
-      return csstree.walk.skip;
+      return csstreeWalkSkip;
     }
   });
   return rules;
 };
 
+/**
+ * @type {(stylesheet: Array<StylesheetRule>, node: XastElement) => ComputedStyles}
+ */
 const computeOwnStyle = (stylesheet, node) => {
+  /**
+   * @type {ComputedStyles}
+   */
   const computedStyle = {};
   const importantStyles = new Map();
 
@@ -107,6 +143,7 @@ const computeOwnStyle = (stylesheet, node) => {
   }
 
   // collect inline styles
+  // @ts-ignore node.style is hidden from pubilc usage
   for (const [name, { value, priority }] of node.style.properties) {
     const computed = computedStyle[name];
     const important = priority === 'important';
@@ -126,7 +163,31 @@ const computeOwnStyle = (stylesheet, node) => {
   return computedStyle;
 };
 
+/**
+ * Compares two selector specificities.
+ * extracted from https://github.com/keeganstreet/specificity/blob/master/specificity.js#L211
+ *
+ * @type {(a: Specificity, b: Specificity) => number}
+ */
+const compareSpecificity = (a, b) => {
+  for (var i = 0; i < 4; i += 1) {
+    if (a[i] < b[i]) {
+      return -1;
+    } else if (a[i] > b[i]) {
+      return 1;
+    }
+  }
+
+  return 0;
+};
+
+/**
+ * @type {(root: XastRoot) => Array<StylesheetRule>}
+ */
 const collectStylesheet = (root) => {
+  /**
+   * @type {Array<StylesheetRule>}
+   */
   const stylesheet = [];
   // find and parse all styles
   visit(root, {
@@ -159,11 +220,16 @@ const collectStylesheet = (root) => {
 };
 exports.collectStylesheet = collectStylesheet;
 
+/**
+ * @type {(stylesheet: Array<StylesheetRule>, node: XastElement) => ComputedStyles}
+ */
 const computeStyle = (stylesheet, node) => {
   // collect inherited styles
   const computedStyles = computeOwnStyle(stylesheet, node);
   let parent = node;
+  // @ts-ignore parentNode is forbidden in public usage
   while (parent.parentNode && parent.parentNode.type !== 'root') {
+    // @ts-ignore parentNode is forbidden in public usage
     const inheritedStyles = computeOwnStyle(stylesheet, parent.parentNode);
     for (const [name, computed] of Object.entries(inheritedStyles)) {
       if (
@@ -175,9 +241,9 @@ const computeStyle = (stylesheet, node) => {
         computedStyles[name] = { ...computed, inherited: true };
       }
     }
+    // @ts-ignore parentNode is forbidden in public usage
     parent = parent.parentNode;
   }
-
   return computedStyles;
 };
 exports.computeStyle = computeStyle;

lib/types.ts

@@ -27,7 +27,7 @@ type XastText = {
   value: string;
 };
 
-type XastElement = {
+export type XastElement = {
   type: 'element';
   name: string;
   attributes: Record<string, string>;
@@ -42,7 +42,7 @@ export type XastChild =
   | XastText
   | XastElement;
 
-type XastRoot = {
+export type XastRoot = {
   type: 'root';
   children: Array<XastChild>;
 };
@@ -53,12 +53,12 @@ export type XastNode = XastRoot | XastChild;
 
 type VisitorNode<Node> = {
   enter?: (node: Node, parentNode: XastParent) => void;
-  leave?: (node: Node, parentNode: XastParent) => void;
+  exit?: (node: Node, parentNode: XastParent) => void;
 };
 
 type VisitorRoot = {
   enter?: (node: XastRoot, parentNode: null) => void;
-  leave?: (node: XastRoot, parentNode: null) => void;
+  exit?: (node: XastRoot, parentNode: null) => void;
 };
 
 export type Visitor = {
@@ -72,3 +72,58 @@ export type Visitor = {
 };
 
 export type Plugin<Params> = (root: XastRoot, params: Params) => null | Visitor;
+
+export type Specificity = [number, number, number, number];
+
+export type StylesheetDeclaration = {
+  name: string;
+  value: string;
+  important: boolean;
+};
+
+export type StylesheetRule = {
+  dynamic: boolean;
+  selectors: string;
+  specificity: Specificity;
+  declarations: Array<StylesheetDeclaration>;
+};
+
+type StaticStyle = {
+  type: 'static';
+  inherited: boolean;
+  value: string;
+};
+
+type DynamicStyle = {
+  type: 'dynamic';
+  inherited: boolean;
+};
+
+export type ComputedStyles = Record<string, StaticStyle | DynamicStyle>;
+
+export type PathDataCommand =
+  | 'M'
+  | 'm'
+  | 'Z'
+  | 'z'
+  | 'L'
+  | 'l'
+  | 'H'
+  | 'h'
+  | 'V'
+  | 'v'
+  | 'C'
+  | 'c'
+  | 'S'
+  | 's'
+  | 'Q'
+  | 'q'
+  | 'T'
+  | 't'
+  | 'A'
+  | 'a';
+
+export type PathDataItem = {
+  command: PathDataCommand;
+  args: Array<number>;
+};

lib/xast.js

@@ -76,11 +76,12 @@ const traverse = (node, fn) => {
 exports.traverse = traverse;
 
 /**
- * @type {(node: any, visitor: any, parentNode: any) => void}
+ * @type {(node: XastNode, visitor: Visitor, parentNode?: any) => void}
  */
 const visit = (node, visitor, parentNode) => {
   const callbacks = visitor[node.type];
   if (callbacks && callbacks.enter) {
+    // @ts-ignore hard to infer
     callbacks.enter(node, parentNode);
   }
   // visit root children
@@ -99,6 +100,7 @@ const visit = (node, visitor, parentNode) => {
     }
   }
   if (callbacks && callbacks.exit) {
+    // @ts-ignore hard to infer
     callbacks.exit(node, parentNode);
   }
 };

package-lock.json

@@ -912,6 +912,21 @@
         "@babel/types": "^7.3.0"
       }
     },
+    "@types/css-tree": {
+      "version": "1.0.6",
+      "resolved": "https://registry.npmjs.org/@types/css-tree/-/css-tree-1.0.6.tgz",
+      "integrity": "sha512-zjSMDm4C7J1azi9SdT1XwNaVCzeHZx+Y5AVebcg/mrtUULNxZjeamc8oQpUDKpaudM5R+Sy7RFvm2xphfwN64w==",
+      "dev": true
+    },
+    "@types/csso": {
+      "version": "4.2.0",
+      "resolved": "https://registry.npmjs.org/@types/csso/-/csso-4.2.0.tgz",
+      "integrity": "sha512-tGMZcJGgeIA67qbbm/ZKUi7ZyiTETEL0X4opRpFzULbIE1Kyt6EfOVSctw4N/WrEJZwmKw0J+1i/OApm6/CAOQ==",
+      "dev": true,
+      "requires": {
+        "@types/css-tree": "*"
+      }
+    },
     "@types/estree": {
       "version": "0.0.39",
       "resolved": "https://registry.npmjs.org/@types/estree/-/estree-0.0.39.tgz",

package.json

@@ -104,6 +104,8 @@
     "@rollup/plugin-commonjs": "^17.1.0",
     "@rollup/plugin-json": "^4.1.0",
     "@rollup/plugin-node-resolve": "^11.2.1",
+    "@types/css-tree": "^1.0.6",
+    "@types/csso": "^4.2.0",
     "@types/jest": "^27.0.1",
     "del": "^6.0.0",
     "eslint": "^7.32.0",

plugins/removeHiddenElems.js

@@ -27,10 +27,25 @@ exports.description =
  * - polyline with empty points
  * - polygon with empty points
  *
- * @param {Object} root
- * @param {Object} params
- *
  * @author Kir Belevich
+ *
+ * @type {import('../lib/types').Plugin<{
+ *   isHidden: boolean,
+ *   displayNone: boolean,
+ *   opacity0: boolean,
+ *   circleR0: boolean,
+ *   ellipseRX0: boolean,
+ *   ellipseRY0: boolean,
+ *   rectWidth0: boolean,
+ *   rectHeight0: boolean,
+ *   patternWidth0: boolean,
+ *   patternHeight0: boolean,
+ *   imageWidth0: boolean,
+ *   imageHeight0: boolean,
+ *   pathEmptyD: boolean,
+ *   polylineEmptyPoints: boolean,
+ *   polygonEmptyPoints: boolean,
+ * }>}
  */
 exports.fn = (root, params) => {
   const {

tsconfig.json

@@ -34,7 +34,6 @@
     "plugins/removeDimensions.js",
     "plugins/removeEditorsNSData.js",
     "plugins/removeEmptyAttrs.js",
-    "plugins/removeHiddenElems.js",
     "plugins/removeNonInheritableGroupAttrs.js",
     "plugins/removeOffCanvasPaths.js",
     "plugins/removeUnknownsAndDefaults.js",