Refactor mergeStyles (#1575)

- covered with tsdoc - replace another closestByName usage with visitSkip symbol to skip subtree instead of skipping element by ancestor Better review with hidden whitespaces as most code just got bigger indent.
svg · Sep 23, 2021 · 9ebff13 · 9ebff13
1 parent 8af10de
commit 9ebff13
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 59 deletions.
diff --git a/plugins/mergeStyles.js b/plugins/mergeStyles.js
@@ -1,6 +1,10 @@
 'use strict';
 
-const { closestByName, detachNodeFromParent } = require('../lib/xast.js');
+/**
+ * @typedef {import('../lib/types').XastElement} XastElement
+ */
+
+const { visitSkip, detachNodeFromParent } = require('../lib/xast.js');
 const JSAPI = require('../lib/svgo/jsAPI.js');
 
 exports.name = 'mergeStyles';
@@ -12,75 +16,78 @@ exports.description = 'merge multiple style elements into one';
  * Merge multiple style elements into one.
  *
  * @author strarsis <strarsis@gmail.com>
+ *
+ * @type {import('../lib/types').Plugin<void>}
  */
 exports.fn = () => {
+  /**
+   * @type {null | XastElement}
+   */
   let firstStyleElement = null;
   let collectedStyles = '';
   let styleContentType = 'text';
 
-  const enterElement = (node, parentNode) => {
-    // collect style elements
-    if (node.name !== 'style') {
-      return;
-    }
-
-    // skip <style> with invalid type attribute
-    if (
-      node.attributes.type != null &&
-      node.attributes.type !== '' &&
-      node.attributes.type !== 'text/css'
-    ) {
-      return;
-    }
+  return {
+    element: {
+      enter: (node, parentNode) => {
+        // skip <foreignObject> content
+        if (node.name === 'foreignObject') {
+          return visitSkip;
+        }
 
-    // skip <foreignObject> content
-    if (closestByName(node, 'foreignObject')) {
-      return;
-    }
+        // collect style elements
+        if (node.name !== 'style') {
+          return;
+        }
 
-    // extract style element content
-    let css = '';
-    for (const child of node.children) {
-      if (child.type === 'text') {
-        css += child.value;
-      }
-      if (child.type === 'cdata') {
-        styleContentType = 'cdata';
-        css += child.value;
-      }
-    }
+        // skip <style> with invalid type attribute
+        if (
+          node.attributes.type != null &&
+          node.attributes.type !== '' &&
+          node.attributes.type !== 'text/css'
+        ) {
+          return;
+        }
 
-    // remove empty style elements
-    if (css.trim().length === 0) {
-      detachNodeFromParent(node, parentNode);
-      return;
-    }
+        // extract style element content
+        let css = '';
+        for (const child of node.children) {
+          if (child.type === 'text') {
+            css += child.value;
+          }
+          if (child.type === 'cdata') {
+            styleContentType = 'cdata';
+            css += child.value;
+          }
+        }
 
-    // collect css and wrap with media query if present in attribute
-    if (node.attributes.media == null) {
-      collectedStyles += css;
-    } else {
-      collectedStyles += `@media ${node.attributes.media}{${css}}`;
-      delete node.attributes.media;
-    }
+        // remove empty style elements
+        if (css.trim().length === 0) {
+          detachNodeFromParent(node, parentNode);
+          return;
+        }
 
-    // combine collected styles in the first style element
-    if (firstStyleElement == null) {
-      firstStyleElement = node;
-    } else {
-      detachNodeFromParent(node, parentNode);
-      firstStyleElement.children = [
-        new JSAPI(
-          { type: styleContentType, value: collectedStyles },
-          firstStyleElement
-        ),
-      ];
-    }
-  };
+        // collect css and wrap with media query if present in attribute
+        if (node.attributes.media == null) {
+          collectedStyles += css;
+        } else {
+          collectedStyles += `@media ${node.attributes.media}{${css}}`;
+          delete node.attributes.media;
+        }
 
-  return {
-    element: {
-      enter: enterElement,
+        // combine collected styles in the first style element
+        if (firstStyleElement == null) {
+          firstStyleElement = node;
+        } else {
+          detachNodeFromParent(node, parentNode);
+          firstStyleElement.children = [
+            new JSAPI(
+              { type: styleContentType, value: collectedStyles },
+              firstStyleElement
+            ),
+          ];
+        }
+      },
     },
   };
 };
diff --git a/test/plugins/mergeStyles.12.svg b/test/plugins/mergeStyles.12.svg
diff --git a/tsconfig.json b/tsconfig.json
@@ -21,7 +21,6 @@
     "plugins/_applyTransforms.js",
     "plugins/convertPathData.js",
     "plugins/convertStyleToAttrs.js",
-    "plugins/mergeStyles.js",
     "plugins/moveElemsAttrsToGroup.js",
     "plugins/moveGroupAttrsToElems.js",
     "plugins/plugins.js",