From 27bef1a954d7a709e2d54d8d926d36aed8a79f5e Mon Sep 17 00:00:00 2001
From: Bogdan Chadkin <trysound@yandex.ru>
Date: Sun, 28 Mar 2021 11:20:17 +0300
Subject: [PATCH] Add "visitor" plugins support (#1454)

Visitor is a simple pattern which helps to avoid many type checks
and provide both "perItem" and "perItemReverse" functionality without
fragmentation.

The most important case is an ability to define state which in many
plugins specified either on module level or by polluting `params`.

In this diff I added visit and detachFromParent utilities and refactored
new mergeStyles plugin with it.

Also fixed bug when cdata content is merged into "text" node which is
not always valid.
---
 lib/svgo/plugins.js             |  10 +++
 lib/xast.js                     |  33 +++++++++
 lib/xast.test.js                | 108 +++++++++++++++++++++++++++++
 plugins/mergeStyles.js          | 118 ++++++++++++++++----------------
 test/plugins/mergeStyles.11.svg |  24 +++++++
 tsconfig.json                   |   1 +
 6 files changed, 234 insertions(+), 60 deletions(-)
 create mode 100644 lib/xast.test.js
 create mode 100644 test/plugins/mergeStyles.11.svg

diff --git a/lib/svgo/plugins.js b/lib/svgo/plugins.js
index 4e1926cd..ed418134 100644
--- a/lib/svgo/plugins.js
+++ b/lib/svgo/plugins.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const { visit } = require('../xast.js');
+
 /**
  * Plugins engine.
  *
@@ -34,6 +36,14 @@ module.exports = function (data, info, plugins) {
       case 'full':
         data = full(data, info, group);
         break;
+      case 'visitor':
+        for (const plugin of group) {
+          if (plugin.active) {
+            const visitor = plugin.fn(data, plugin.params, info);
+            visit(data, visitor);
+          }
+        }
+        break;
     }
   }
   return data;
diff --git a/lib/xast.js b/lib/xast.js
index 8a8118bc..9800e2ac 100644
--- a/lib/xast.js
+++ b/lib/xast.js
@@ -51,3 +51,36 @@ const traverse = (node, fn) => {
   }
 };
 exports.traverse = traverse;
+
+const visit = (node, visitor) => {
+  const callbacks = visitor[node.type];
+  if (callbacks && callbacks.enter) {
+    callbacks.enter(node);
+  }
+  // visit root children
+  if (node.type === 'root') {
+    // copy children array to not loose cursor when children is spliced
+    for (const child of node.children) {
+      visit(child, visitor);
+    }
+  }
+  // visit element children if still attached to parent
+  if (node.type === 'element') {
+    if (node.parentNode.children.includes(node)) {
+      for (const child of node.children) {
+        visit(child, visitor);
+      }
+    }
+  }
+  if (callbacks && callbacks.exit) {
+    callbacks.exit(node);
+  }
+};
+exports.visit = visit;
+
+const detachNodeFromParent = (node) => {
+  const parentNode = node.parentNode;
+  // avoid splice to not break for loops
+  parentNode.children = parentNode.children.filter((child) => child !== node);
+};
+exports.detachNodeFromParent = detachNodeFromParent;
diff --git a/lib/xast.test.js b/lib/xast.test.js
new file mode 100644
index 00000000..0008b936
--- /dev/null
+++ b/lib/xast.test.js
@@ -0,0 +1,108 @@
+'use strict';
+
+const { expect } = require('chai');
+const { visit, detachNodeFromParent } = require('./xast.js');
+
+const getAst = () => {
+  const ast = {
+    type: 'root',
+    children: [
+      {
+        type: 'element',
+        name: 'g',
+        attributes: {},
+        children: [
+          {
+            type: 'element',
+            name: 'rect',
+            attributes: {},
+            children: [],
+          },
+          {
+            type: 'element',
+            name: 'circle',
+            attributes: {},
+            children: [],
+          },
+        ],
+      },
+      {
+        type: 'element',
+        name: 'ellipse',
+        attributes: {},
+        children: [],
+      },
+    ],
+  };
+  ast.children[0].parentNode = ast;
+  ast.children[0].children[0].parentNode = ast.children[0];
+  ast.children[0].children[1].parentNode = ast.children[0];
+  ast.children[1].parentNode = ast;
+  return ast;
+};
+
+describe('xast', () => {
+  it('enter into nodes', () => {
+    const root = getAst();
+    const entered = [];
+    visit(root, {
+      root: {
+        enter: (node) => {
+          entered.push(node.type);
+        },
+      },
+      element: {
+        enter: (node) => {
+          entered.push(`${node.type}:${node.name}`);
+        },
+      },
+    });
+    expect(entered).to.deep.equal([
+      'root',
+      'element:g',
+      'element:rect',
+      'element:circle',
+      'element:ellipse',
+    ]);
+  });
+
+  it('exit from nodes', () => {
+    const root = getAst();
+    const exited = [];
+    visit(root, {
+      root: {
+        exit: (node) => {
+          exited.push(node.type);
+        },
+      },
+      element: {
+        exit: (node) => {
+          exited.push(`${node.type}:${node.name}`);
+        },
+      },
+    });
+    expect(exited).to.deep.equal([
+      'element:rect',
+      'element:circle',
+      'element:g',
+      'element:ellipse',
+      'root',
+    ]);
+  });
+
+  it('skip entering children if node is detached', () => {
+    const root = getAst();
+    const entered = [];
+    visit(root, {
+      element: {
+        enter: (node) => {
+          entered.push(node.name);
+          if (node.name === 'g') {
+            detachNodeFromParent(node);
+          }
+        },
+      },
+    });
+    expect(entered).to.deep.equal(['g', 'ellipse']);
+  });
+});
diff --git a/plugins/mergeStyles.js b/plugins/mergeStyles.js
index 48f2ba1d..c57a5029 100644
--- a/plugins/mergeStyles.js
+++ b/plugins/mergeStyles.js
@@ -1,87 +1,85 @@
 'use strict';
 
-const { querySelectorAll, closestByName } = require('../lib/xast.js');
-const { getCssStr, setCssStr } = require('../lib/css-tools');
+const { closestByName, detachNodeFromParent } = require('../lib/xast.js');
+const JSAPI = require('../lib/svgo/jsAPI.js');
 
-exports.type = 'full';
+exports.type = 'visitor';
 exports.active = true;
 exports.description = 'merge multiple style elements into one';
 
 /**
  * Merge multiple style elements into one.
  *
- * @param {Object} document document element
- *
  * @author strarsis <strarsis@gmail.com>
  */
-exports.fn = function (document) {
-  // collect <style/>s with valid type attribute (preserve order)
-  const styleElements = querySelectorAll(document, 'style');
+exports.fn = () => {
+  let firstStyleElement = null;
+  let collectedStyles = '';
+  let styleContentType = 'text';
 
-  // no <styles/>s, nothing to do
-  if (styleElements.length === 0) {
-    return document;
-  }
+  const enterElement = (node) => {
+    // collect style elements
+    if (node.name !== 'style') {
+      return;
+    }
 
-  const styles = [];
-  for (const styleElement of styleElements) {
+    // skip <style> with invalid type attribute
     if (
-      styleElement.attributes.type &&
-      styleElement.attributes.type !== 'text/css'
+      node.attributes.type != null &&
+      node.attributes.type !== '' &&
+      node.attributes.type !== 'text/css'
     ) {
-      // skip <style> with invalid type attribute
-      continue;
+      return;
     }
 
-    if (closestByName(styleElement, 'foreignObject')) {
-      // skip <foreignObject> content
-      continue;
+    // skip <foreignObject> content
+    if (closestByName(node, 'foreignObject')) {
+      return;
     }
 
-    const cssString = getCssStr(styleElement);
+    // 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;
+      }
+    }
 
-    styles.push({
-      styleElement: styleElement,
+    // remove empty style elements
+    if (css.trim().length === 0) {
+      detachNodeFromParent(node);
+      return;
+    }
 
-      mq: styleElement.attributes.media,
-      cssStr: cssString,
-    });
-  }
-
-  const collectedStyles = [];
-  for (let styleNo = 0; styleNo < styles.length; styleNo += 1) {
-    const style = styles[styleNo];
-
-    if (style.mq) {
-      const wrappedStyles = `@media ${style.mq}{${style.cssStr}}`;
-      collectedStyles.push(wrappedStyles);
+    // collect css and wrap with media query if present in attribute
+    if (node.attributes.media == null) {
+      collectedStyles += css;
     } else {
-      collectedStyles.push(style.cssStr);
+      collectedStyles += `@media ${node.attributes.media}{${css}}`;
+      delete node.attributes.media;
     }
 
-    // remove all processed style elements – except the first one
-    if (styleNo > 0) {
-      removeFromParent(style.styleElement);
+    // combine collected styles in the first style element
+    if (firstStyleElement == null) {
+      firstStyleElement = node;
+    } else {
+      detachNodeFromParent(node);
+      firstStyleElement.children = [
+        new JSAPI(
+          { type: styleContentType, value: collectedStyles },
+          firstStyleElement
+        ),
+      ];
     }
-  }
-  const collectedStylesString = collectedStyles.join('');
+  };
 
-  // combine collected styles in the first style element
-  const firstStyle = styles[0];
-  delete firstStyle.styleElement.attributes.media; // remove media mq attribute as CSS media queries are used
-  if (collectedStylesString.trim().length > 0) {
-    setCssStr(firstStyle.styleElement, collectedStylesString);
-  } else {
-    removeFromParent(firstStyle.styleElement);
-  }
-
-  return document;
+  return {
+    element: {
+      enter: enterElement,
+    },
+  };
 };
-
-function removeFromParent(element) {
-  const parentElement = element.parentNode;
-  return parentElement.children.splice(
-    parentElement.children.indexOf(element),
-    1
-  );
-}
diff --git a/test/plugins/mergeStyles.11.svg b/test/plugins/mergeStyles.11.svg
new file mode 100644
index 00000000..3983fee4
--- /dev/null
+++ b/test/plugins/mergeStyles.11.svg
@@ -0,0 +1,24 @@
+Convert content to cdata if any style element contains cdata
+
+===
+
+<svg id="test" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100">
+    <style>
+        .st0 { fill: yellow; }
+    </style>
+    <style>
+        <![CDATA[
+            .st1 { fill: red; }
+        ]]>
+    </style>
+</svg>
+
+@@@
+
+<svg id="test" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100">
+    <style>
+        <![CDATA[.st0 { fill: yellow; }
+            .st1 { fill: red; }
+        ]]>
+    </style>
+</svg>
diff --git a/tsconfig.json b/tsconfig.json
index 1e39c1a8..254f0254 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -36,6 +36,7 @@
     "plugins/convertStyleToAttrs.js",
     "plugins/convertTransform.js",
     "plugins/mergePaths.js",
+    "plugins/mergeStyles.js",
     "plugins/moveElemsAttrsToGroup.js",
     "plugins/moveGroupAttrsToElems.js",
     "plugins/plugins.js",