Improve text node handling (#68)

lib/builtin.js

@@ -3,10 +3,7 @@ import presetNext from '../plugins/preset-next.js';
 import presetNone from '../plugins/preset-none.js';
 import * as addAttributesToSVGElement from '../plugins/addAttributesToSVGElement.js';
 import * as addClassesToSVGElement from '../plugins/addClassesToSVGElement.js';
-import * as cleanupAttrs from '../plugins/cleanupAttrs.js';
-import * as cleanupEnableBackground from '../plugins/cleanupEnableBackground.js';
 import * as cleanupIds from '../plugins/cleanupIds.js';
-import * as cleanupListOfValues from '../plugins/cleanupListOfValues.js';
 import * as cleanupNumericValues from '../plugins/cleanupNumericValues.js';
 import * as cleanupStyleAttributes from '../plugins/cleanupStyleAttributes.js';
 import * as cleanupXlink from '../plugins/cleanupXlink.js';
@@ -70,10 +67,7 @@ export const builtin = Object.freeze([
   presetNone,
   addAttributesToSVGElement,
   addClassesToSVGElement,
-  cleanupAttrs,
-  cleanupEnableBackground,
   cleanupIds,
-  cleanupListOfValues,
   cleanupNumericValues,
   cleanupStyleAttributes,
   combineStyleElements,

lib/parser.js

@@ -4,16 +4,15 @@
  * @typedef {import('./types.js').XastDoctype} XastDoctype
  * @typedef {import('./types.js').XastComment} XastComment
  * @typedef {import('./types.js').XastRoot} XastRoot
- * @typedef {import('./types.js').XastElement} XastElement
  * @typedef {import('./types.js').XastCdata} XastCdata
  * @typedef {import('./types.js').XastText} XastText
  * @typedef {import('./types.js').XastParent} XastParent
  * @typedef {import('./types.js').XastChild} XastChild
  */
 
 import SAX from 'sax';
-import { textElems } from '../plugins/_collections.js';
 import { SVGOError } from './svgo/tools.js';
+import { elemsGroups } from '../plugins/_collections.js';
 
 export class SvgoParserError extends Error {
   /**
@@ -200,9 +199,7 @@ export const parseSvg = (data, from) => {
       }
     }
 
-    /**
-     * @type {XastElement}
-     */
+    /** @type {import('./types.js').XastElement} */
     const element = {
       type: 'element',
       parentNode: current,
@@ -255,26 +252,18 @@ export const parseSvg = (data, from) => {
   sax.ontext = (text) => {
     if (current.type === 'element') {
       // prevent trimming of meaningful whitespace inside textual tags
-      if (foreignLevel > 0 || textElems.has(current.name)) {
-        /**
-         * @type {XastText}
-         */
+      if (
+        elemsGroups.characterData.has(current.name) ||
+        current.name.includes(':') ||
+        foreignLevel > 0
+      ) {
+        /** @type {XastText} */
         const node = {
           type: 'text',
           parentNode: current,
           value: text,
         };
         pushToContent(node);
-      } else if (/\S/.test(text)) {
-        /**
-         * @type {XastText}
-         */
-        const node = {
-          type: 'text',
-          parentNode: current,
-          value: text.trim(),
-        };
-        pushToContent(node);
       }
     }
   };

lib/stringifier.js

@@ -1,9 +1,6 @@
-import { textElems } from '../plugins/_collections.js';
-
 /**
  * @typedef {import('./types.js').XastParent} XastParent
  * @typedef {import('./types.js').XastRoot} XastRoot
- * @typedef {import('./types.js').XastElement} XastElement
  * @typedef {import('./types.js').XastInstruction} XastInstruction
  * @typedef {import('./types.js').XastDoctype} XastDoctype
  * @typedef {import('./types.js').XastText} XastText
@@ -12,17 +9,17 @@ import { textElems } from '../plugins/_collections.js';
  * @typedef {import('./types.js').StringifyOptions} StringifyOptions
  * @typedef {{
  *   indent: string,
- *   textContext: ?XastElement,
+ *   textContext: ?import('./types.js').XastElement,
  *   indentLevel: number,
  *   foreignLevel: number,
  *   eolLen: number
  * }} State
  * @typedef {Required<StringifyOptions>} Options
  */
 
-/**
- * @type {(char: string) => string}
- */
+import { elemsGroups } from '../plugins/_collections.js';
+
+/** @type {(char: string) => string} */
 const encodeEntity = (char) => {
   return entities[char];
 };
@@ -156,7 +153,7 @@ const createIndent = (config, state) => {
  * @param {State} state
  */
 const formatEndTag = (tagEnd, config, state) => {
-  if (config.pretty && state.foreignLevel > 0) {
+  if (config.pretty && state.foreignLevel > 0 && tagEnd.endsWith('\n')) {
     return tagEnd.substring(0, tagEnd.length - state.eolLen);
   }
   return tagEnd;
@@ -202,7 +199,7 @@ const stringifyCdata = (node, config, state) => {
 };
 
 /**
- * @type {(node: XastElement, config: Options, state: State) => string}
+ * @type {(node: import('./types.js').XastElement, config: Options, state: State) => string}
  */
 const stringifyElement = (node, config, state) => {
   // empty element and short tag
@@ -242,7 +239,10 @@ const stringifyElement = (node, config, state) => {
       tagCloseStart = defaults.tagCloseStart;
       tagCloseEnd = defaults.tagCloseEnd;
       openIndent = '';
-    } else if (textElems.has(node.name)) {
+    } else if (
+      elemsGroups.characterData.has(node.name) ||
+      node.name.includes(':')
+    ) {
       tagOpenEnd = defaults.tagOpenEnd;
       tagCloseStart = defaults.tagCloseStart;
       enableCloseIndent = false;
@@ -277,7 +277,7 @@ const stringifyElement = (node, config, state) => {
 };
 
 /**
- * @type {(node: XastElement, config: Options) => string}
+ * @type {(node: import('./types.js').XastElement, config: Options) => string}
  */
 const stringifyAttributes = (node, config) => {
   let attrs = '';

lib/svgo.test.js

@@ -75,22 +75,22 @@ test('warn when user tries enable plugins in preset', () => {
         name: 'preset-default',
         params: {
           overrides: {
-            cleanupListOfValues: true,
+            removeViewBox: true,
           },
         },
       },
     ],
     js2svg: { pretty: true, indent: 2 },
   });
   expect(warn)
-    .toBeCalledWith(`You are trying to configure cleanupListOfValues which is not part of preset-default.
+    .toBeCalledWith(`You are trying to configure removeViewBox which is not part of preset-default.
 Try to put it before or after, for example
 
 plugins: [
   {
     name: 'preset-default',
   },
-  'cleanupListOfValues'
+  'removeViewBox'
 ]
 `);
   warn.mockRestore();

plugins/_collections.js

@@ -11,6 +11,17 @@ export const elemsGroups = {
     'animateTransform',
     'set',
   ]),
+  characterData: new Set([
+    // <a> can only contain character data when it is a child of <text> - see https://svgwg.org/svg2-draft/linking.html#AElement
+    'a',
+    'desc',
+    'script',
+    'style',
+    'text',
+    'textPath',
+    'title',
+    'tspan',
+  ]),
   descriptive: new Set(['desc', 'metadata', 'title']),
   shape: new Set([
     'circle',
@@ -54,18 +65,6 @@ export const elemsGroups = {
     'switch',
     'symbol',
   ]),
-  textContent: new Set([
-    'a',
-    'altGlyph',
-    'altGlyphDef',
-    'altGlyphItem',
-    'glyph',
-    'glyphRef',
-    'text',
-    'textPath',
-    'tref',
-    'tspan',
-  ]),
   textContentChild: new Set(['altGlyph', 'textPath', 'tref', 'tspan']),
   lightSource: new Set([
     'feDiffuseLighting',
@@ -100,12 +99,6 @@ export const elemsGroups = {
   ]),
 };
 
-/**
- * Elements where adding or removing whitespace may effect rendering, metadata,
- * or semantic meaning.
- */
-export const textElems = new Set([...elemsGroups.textContent, 'title']);
-
 export const pathElems = new Set(['glyph', 'missing-glyph', 'path']);
 
 /**
@@ -400,6 +393,7 @@ export const elems = {
     attrs: new Set([
       'class',
       'externalResourcesRequired',
+      'href',
       'style',
       'target',
       'transform',
@@ -2517,7 +2511,6 @@ export const uselessShapeProperties = new Set([
 
 export default {
   elemsGroups,
-  textElems,
   pathElems,
   attrsGroups,
   attrsGroupsDefaults,