Move removeEmptyText functionality to removeEmptyContainers. (#79)

lib/builtin.js

@@ -38,7 +38,6 @@ import * as removeEditorsNSData from '../plugins/removeEditorsNSData.js';
 import * as removeElementsByAttr from '../plugins/removeElementsByAttr.js';
 import * as removeEmptyAttrs from '../plugins/removeEmptyAttrs.js';
 import * as removeEmptyContainers from '../plugins/removeEmptyContainers.js';
-import * as removeEmptyText from '../plugins/removeEmptyText.js';
 import * as removeHiddenElems from '../plugins/removeHiddenElems.js';
 import * as removeMetadata from '../plugins/removeMetadata.js';
 import * as removeNonInheritableGroupAttrs from '../plugins/removeNonInheritableGroupAttrs.js';
@@ -103,7 +102,6 @@ export const builtin = Object.freeze([
   removeElementsByAttr,
   removeEmptyAttrs,
   removeEmptyContainers,
-  removeEmptyText,
   removeHiddenElems,
   removeMetadata,
   removeNonInheritableGroupAttrs,

lib/svgo/tools.js

@@ -65,6 +65,19 @@ export const decodeSVGDatauri = (str) => {
   return str;
 };
 
+/**
+ * @param {Map<import('../types.js').XastParent,Set<import('../types.js').XastChild>>} childrenToDeleteByParent
+ * @param {import('../types.js').XastChild} child
+ */
+export function addChildToDelete(childrenToDeleteByParent, child) {
+  let childrenToDelete = childrenToDeleteByParent.get(child.parentNode);
+  if (!childrenToDelete) {
+    childrenToDelete = new Set();
+    childrenToDeleteByParent.set(child.parentNode, childrenToDelete);
+  }
+  childrenToDelete.add(child);
+}
+
 /**
  * @typedef {{
  *   noSpaceAfterFlags?: boolean,
@@ -124,6 +137,16 @@ export const cleanupOutData = (data, params, command) => {
   return str;
 };
 
+/**
+ * @param {Map<import('../types.js').XastParent,Set<import('../types.js').XastChild>>} childrenToDeleteByParent
+ */
+export function deleteChildren(childrenToDeleteByParent) {
+  // For each parent, delete no longer needed children.
+  for (const [parent, childrenToDelete] of childrenToDeleteByParent) {
+    parent.children = parent.children.filter((c) => !childrenToDelete.has(c));
+  }
+}
+
 /**
  * @param {number} n
  * @param {number} m

plugins/_collections.js

@@ -52,19 +52,6 @@ export const elemsGroups = {
     'solidColor',
     'symbol',
   ]),
-  container: new Set([
-    'a',
-    'defs',
-    'foreignObject',
-    'g',
-    'marker',
-    'mask',
-    'missing-glyph',
-    'pattern',
-    'svg',
-    'switch',
-    'symbol',
-  ]),
   textContentChild: new Set(['altGlyph', 'textPath', 'tref', 'tspan']),
   lightSource: new Set([
     'feDiffuseLighting',

plugins/plugins-types.d.ts

@@ -100,11 +100,6 @@ type DefaultPlugins = {
   };
   removeEmptyAttrs: void;
   removeEmptyContainers: void;
-  removeEmptyText: {
-    text?: boolean;
-    tspan?: boolean;
-    tref?: boolean;
-  };
   removeHiddenElems: {
     isHidden?: boolean;
     displayNone?: boolean;

plugins/preset-default.js

@@ -20,7 +20,6 @@ import * as removeDesc from './removeDesc.js';
 import * as removeDoctype from './removeDoctype.js';
 import * as removeEditorsNSData from './removeEditorsNSData.js';
 import * as removeEmptyContainers from './removeEmptyContainers.js';
-import * as removeEmptyText from './removeEmptyText.js';
 import * as removeHiddenElems from './removeHiddenElems.js';
 import * as removeMetadata from './removeMetadata.js';
 import * as removeNonInheritableGroupAttrs from './removeNonInheritableGroupAttrs.js';
@@ -50,7 +49,6 @@ const presetDefault = createPreset({
     removeNonInheritableGroupAttrs,
     removeUselessStrokeAndFill,
     removeHiddenElems,
-    removeEmptyText,
     minifyTransforms,
     convertEllipseToCircle,
     moveElemsStylesToGroup,

plugins/preset-next.js

@@ -20,7 +20,6 @@ import * as removeDesc from './removeDesc.js';
 import * as removeDoctype from './removeDoctype.js';
 import * as removeEditorsNSData from './removeEditorsNSData.js';
 import * as removeEmptyContainers from './removeEmptyContainers.js';
-import * as removeEmptyText from './removeEmptyText.js';
 import * as removeHiddenElems from './removeHiddenElems.js';
 import * as removeMetadata from './removeMetadata.js';
 import * as removeNonInheritableGroupAttrs from './removeNonInheritableGroupAttrs.js';
@@ -50,7 +49,6 @@ const presetNext = createPreset({
     removeNonInheritableGroupAttrs,
     removeUselessStrokeAndFill,
     removeHiddenElems,
-    removeEmptyText,
     minifyTransforms,
     convertEllipseToCircle,
     moveElemsStylesToGroup,

plugins/removeEmptyContainers.js

@@ -1,101 +1,107 @@
-import { elemsGroups } from './_collections.js';
+import {
+  addChildToDelete,
+  deleteChildren,
+  getHrefId,
+} from '../lib/svgo/tools.js';
 import { detachNodeFromParent } from '../lib/xast.js';
-import { findReferences } from '../lib/svgo/tools.js';
-
-/**
- * @typedef {import('../lib/types.js').XastElement} XastElement
- * @typedef {import('../lib/types.js').XastParent} XastParent
- */
 
 export const name = 'removeEmptyContainers';
 export const description = 'removes empty container elements';
 
+const removableEls = new Set([
+  'a',
+  'defs',
+  'foreignObject',
+  'g',
+  'marker',
+  'mask',
+  'missing-glyph',
+  'pattern',
+  'switch',
+  'symbol',
+  'text',
+  'tspan',
+]);
+
 /**
- * Remove empty containers.
+ * Remove empty containers and text elements.
  *
  * @see https://www.w3.org/TR/SVG11/intro.html#TermContainerElement
  *
- * @example
- * <defs/>
- *
- * @example
- * <g><marker><a/></marker></g>
- *
- * @author Kir Belevich
- *
  * @type {import('./plugins-types.js').Plugin<'removeEmptyContainers'>}
  */
 export const fn = () => {
   const removedIds = new Set();
-  /**
-   * @type {Map<string, {node:XastElement,parent:XastParent}[]>}
-   */
+  /** @type {Map<string, import('../lib/types.js').XastElement[]>} */
   const usesById = new Map();
 
   return {
     element: {
-      enter: (node, parentNode) => {
-        if (node.name === 'use') {
+      enter: (element) => {
+        if (element.name === 'use') {
           // Record uses so those referencing empty containers can be removed.
-          for (const [name, value] of Object.entries(node.attributes)) {
-            const ids = findReferences(name, value);
-            for (const id of ids) {
-              let references = usesById.get(id);
-              if (references === undefined) {
-                references = [];
-                usesById.set(id, references);
-              }
-              references.push({ node: node, parent: parentNode });
+          const id = getHrefId(element);
+          if (id) {
+            let references = usesById.get(id);
+            if (references === undefined) {
+              references = [];
+              usesById.set(id, references);
             }
+            references.push(element);
           }
         }
       },
-      exit: (node, parentNode) => {
+      exit: (element, parentNode) => {
         // remove only empty non-svg containers
-        if (
-          node.name === 'svg' ||
-          !elemsGroups.container.has(node.name) ||
-          node.children.length !== 0
-        ) {
+        if (!removableEls.has(element.name) || element.children.length !== 0) {
           return;
         }
         // empty patterns may contain reusable configuration
         if (
-          node.name === 'pattern' &&
-          Object.keys(node.attributes).length !== 0
+          element.name === 'pattern' &&
+          Object.keys(element.attributes).length !== 0
         ) {
           return;
         }
         // The <g> may not have content, but the filter may cause a rectangle
         // to be created and filled with pattern.
-        if (node.name === 'g' && node.attributes.filter != null) {
+        if (element.name === 'g' && element.attributes.filter != null) {
           return;
         }
         // empty <mask> hides masked element
-        if (node.name === 'mask' && node.attributes.id != null) {
+        if (element.name === 'mask' && element.attributes.id != null) {
           return;
         }
         if (parentNode.type === 'element' && parentNode.name === 'switch') {
           return;
         }
 
-        detachNodeFromParent(node, parentNode);
-        if (node.attributes.id) {
-          removedIds.add(node.attributes.id);
+        // TODO: Change the way this works so that parent removes empty children. We can't queue them for deletion in
+        // root exit; this is running in element exit so that nested empty elements are removed from bottom up, the nesting
+        // would be hard to detect in root exit.
+        detachNodeFromParent(element, parentNode);
+        if (element.attributes.id) {
+          removedIds.add(element.attributes.id);
         }
       },
     },
     root: {
       exit: () => {
         // Remove any <use> elements that referenced an empty container.
+
+        /** @type {Map<import('../lib/types.js').XastParent,Set<import('../lib/types.js').XastChild>>} */
+        const childrenToDelete = new Map();
+
         for (const id of removedIds) {
-          const uses = usesById.get(id);
-          if (uses) {
-            for (const use of uses) {
-              detachNodeFromParent(use.node, use.parent);
+          const usingEls = usesById.get(id);
+          if (usingEls) {
+            for (const element of usingEls) {
+              addChildToDelete(childrenToDelete, element);
             }
           }
         }
+
+        deleteChildren(childrenToDelete);
       },
     },
   };

plugins/removeHiddenElems.js

@@ -1,5 +1,9 @@
 import { parsePathCommands, PathParseError } from '../lib/pathutils.js';
-import { getEllipseProperties } from '../lib/svgo/tools.js';
+import {
+  addChildToDelete,
+  deleteChildren,
+  getEllipseProperties,
+} from '../lib/svgo/tools.js';
 import { elemsGroups } from './_collections.js';
 
 export const name = 'removeHiddenElems';
@@ -44,12 +48,7 @@ export const fn = (root, params, info) => {
    * @param {import('../lib/types.js').XastElement} element
    */
   function removeElement(element) {
-    let childrenToDelete = childrenToDeleteByParent.get(element.parentNode);
-    if (!childrenToDelete) {
-      childrenToDelete = new Set();
-      childrenToDeleteByParent.set(element.parentNode, childrenToDelete);
-    }
-    childrenToDelete.add(element);
+    addChildToDelete(childrenToDeleteByParent, element);
   }
 
   /**
@@ -200,12 +199,7 @@ export const fn = (root, params, info) => {
     },
     root: {
       exit: () => {
-        // For each parent, delete no longer needed children.
-        for (const [parent, childrenToDelete] of childrenToDeleteByParent) {
-          parent.children = parent.children.filter(
-            (c) => !childrenToDelete.has(c),
-          );
-        }
+        deleteChildren(childrenToDeleteByParent);
       },
     },
   };

test/plugins/removeEmptyText.01.svg.txt → test/plugins/removeEmptyContainers.08.svg.txt

@@ -6,6 +6,4 @@
 
 @@@
 
-<svg xmlns="http://www.w3.org/2000/svg">
-    <g/>
-</svg>
+<svg xmlns="http://www.w3.org/2000/svg"/>