From 827538b4cedf5d5a9fce13d0d09f848a03c5e516 Mon Sep 17 00:00:00 2001 From: johnkenny54 <45182853+johnkenny54@users.noreply.github.com> Date: Fri, 1 Nov 2024 11:06:51 -0700 Subject: [PATCH] Add cleanupTextElements plugin (#69) --- lib/builtin.js | 4 +- plugins/cleanupTextElements.js | 267 ++++++++++++++++++++ plugins/plugins-types.d.ts | 1 + plugins/preset-default.js | 2 + plugins/preset-next.js | 2 + test/plugins/_cleanupTextElements.test.js | 20 ++ test/plugins/cleanupTextElements.01.svg.txt | 20 ++ test/plugins/cleanupTextElements.02.svg.txt | 17 ++ test/plugins/cleanupTextElements.03.svg.txt | 25 ++ test/svgo/_index.test.js | 9 +- test/svgo/whitespaces.svg.txt | 15 -- 11 files changed, 358 insertions(+), 24 deletions(-) create mode 100644 plugins/cleanupTextElements.js create mode 100644 test/plugins/_cleanupTextElements.test.js create mode 100644 test/plugins/cleanupTextElements.01.svg.txt create mode 100644 test/plugins/cleanupTextElements.02.svg.txt create mode 100644 test/plugins/cleanupTextElements.03.svg.txt delete mode 100644 test/svgo/whitespaces.svg.txt diff --git a/lib/builtin.js b/lib/builtin.js index 0fe9663..cbfa502 100644 --- a/lib/builtin.js +++ b/lib/builtin.js @@ -6,6 +6,7 @@ import * as addClassesToSVGElement from '../plugins/addClassesToSVGElement.js'; import * as cleanupIds from '../plugins/cleanupIds.js'; import * as cleanupNumericValues from '../plugins/cleanupNumericValues.js'; import * as cleanupStyleAttributes from '../plugins/cleanupStyleAttributes.js'; +import * as cleanupTextElements from '../plugins/cleanupTextElements.js'; import * as cleanupXlink from '../plugins/cleanupXlink.js'; import * as collapseGroups from '../plugins/collapseGroups.js'; import * as combinePaths from '../plugins/combinePaths.js'; @@ -70,10 +71,11 @@ export const builtin = Object.freeze([ cleanupIds, cleanupNumericValues, cleanupStyleAttributes, - combineStyleElements, + cleanupTextElements, cleanupXlink, collapseGroups, combinePaths, + combineStyleElements, convertEllipseToCircle, convertPathData, convertShapeToPath, diff --git a/plugins/cleanupTextElements.js b/plugins/cleanupTextElements.js new file mode 100644 index 0000000..87ac6e6 --- /dev/null +++ b/plugins/cleanupTextElements.js @@ -0,0 +1,267 @@ +export const name = 'cleanupTextElements'; +export const description = 'simplify elements and content'; + +/** + * @type {import('./plugins-types.js').Plugin<'cleanupTextElements'>} + */ +export const fn = (root, params, info) => { + const styleData = info.docData.getStyles(); + if ( + info.docData.hasScripts() || + styleData === null || + styleData.hasStyles() + ) { + return; + } + + /** @type {Map>} */ + const textElsToHoist = new Map(); + + return { + element: { + exit: (element) => { + if (element.name !== 'text') { + return; + } + + // Remove xml:space="preserve" if possible. + if (element.attributes['xml:space'] === 'preserve') { + if (canRemovePreserve(element)) { + delete element.attributes['xml:space']; + } + } + + // Remove any pure whitespace children. + const childrenToDelete = new Set(); + for (const child of element.children) { + switch (child.type) { + case 'cdata': + case 'text': + if (isOnlyWhiteSpace(child.value)) { + childrenToDelete.add(child); + } + break; + } + } + if (childrenToDelete.size > 0) { + element.children = element.children.filter( + (c) => !childrenToDelete.has(c), + ); + } + + // If there is a single child whose content can be hoisted, do so. + const hoistableChild = getHoistableChild(element); + if (hoistableChild) { + element.children = hoistableChild.children; + for (const child of element.children) { + child.parentNode = element; + } + for (const attributeName of ['x', 'y']) { + if (hoistableChild.attributes[attributeName] !== undefined) { + element.attributes[attributeName] = + hoistableChild.attributes[attributeName]; + } + } + } + + // If the element has x/y, and so do all children, remove x/y from . + if ( + element.attributes.x !== undefined && + element.attributes.y !== undefined && + childrenAllHaveXY(element) + ) { + delete element.attributes.x; + delete element.attributes.y; + } + + // If the has no attributes, and all of the children can be hoisted, add this element to the list to be updated + // at the end. + if (Object.keys(element.attributes).length === 0) { + if ( + element.parentNode.type === 'element' && + element.parentNode.name !== 'switch' + ) { + if ( + element.children.every( + (child) => isHoistable(child) !== undefined, + ) + ) { + let textEls = textElsToHoist.get(element.parentNode); + if (!textEls) { + textEls = new Set(); + textElsToHoist.set(element.parentNode, textEls); + } + textEls.add(element); + } + } + } + }, + }, + root: { + exit: () => { + for (const [parent, textEls] of textElsToHoist.entries()) { + /** @type {import('../lib/types.js').XastChild[]} */ + const newChildren = []; + for (const child of parent.children) { + if (child.type !== 'element' || !textEls.has(child)) { + newChildren.push(child); + continue; + } + // Promote all children to elements. + for (const textChild of child.children) { + if (textChild.type !== 'element') { + throw new Error(); + } + textChild.parentNode = parent; + textChild.name = 'text'; + newChildren.push(textChild); + } + } + parent.children = newChildren; + } + }, + }, + }; +}; + +/** + * @param {import('../lib/types.js').XastElement} element + * @returns {boolean} + */ +function canRemovePreserve(element) { + for (const child of element.children) { + switch (child.type) { + case 'cdata': + case 'text': + if (hasSignificantWhiteSpace(child.value)) { + return false; + } + break; + case 'element': + switch (child.name) { + case 'tspan': + if (!canRemovePreserve(child)) { + return false; + } + break; + default: + return false; + } + break; + } + } + return true; +} + +/** + * @param {import('../lib/types.js').XastElement} element + * @returns {boolean} + */ +function childrenAllHaveXY(element) { + for (const child of element.children) { + if (child.type !== 'element') { + return false; + } + if (child.name !== 'tspan') { + return false; + } + if (child.attributes.x === undefined || child.attributes.y === undefined) { + return false; + } + } + return true; +} + +/** + * @param {import('../lib/types.js').XastElement} element + * @returns {import('../lib/types.js').XastElement|undefined} + */ +function getHoistableChild(element) { + if (element.children.length !== 1) { + return; + } + const child = element.children[0]; + return isHoistable(child); +} + +/** + * @param {string} str + * @returns {boolean} + */ +export function hasSignificantWhiteSpace(str) { + let isStart = true; + let lastIsSpace = false; + for (const char of str) { + switch (char) { + case ' ': + case '\n': + case '\t': + if (!isStart) { + // Consective space within text is significant. + if (lastIsSpace) { + return true; + } + } + lastIsSpace = true; + break; + default: + if (isStart) { + if (lastIsSpace) { + // There is space at beginning of string. + return true; + } + isStart = false; + } + lastIsSpace = false; + break; + } + } + if (isStart) { + return false; + } + return lastIsSpace; +} + +/** + * @param {import('../lib/types.js').XastChild} child + * @returns {import('./collapseGroups.js').XastElement|undefined} + */ +function isHoistable(child) { + if (child.type !== 'element') { + return; + } + if (child.children.length !== 1) { + return; + } + if (child.children[0].type !== 'text') { + return; + } + for (const attributeName of Object.keys(child.attributes)) { + switch (attributeName) { + case 'x': + case 'y': + break; + default: + return; + } + } + return child; +} + +/** + * @param {string} str + * @returns {boolean} + */ +function isOnlyWhiteSpace(str) { + for (const char of str) { + switch (char) { + case ' ': + case '\n': + case '\t': + continue; + default: + return false; + } + } + return true; +} diff --git a/plugins/plugins-types.d.ts b/plugins/plugins-types.d.ts index 72ae894..23982a2 100644 --- a/plugins/plugins-types.d.ts +++ b/plugins/plugins-types.d.ts @@ -17,6 +17,7 @@ type DefaultPlugins = { }; cleanupStyleAttributes: void; cleanupStyleElements: void; + cleanupTextElements: void; collapseGroups: void; combinePaths: void; convertEllipseToCircle: void; diff --git a/plugins/preset-default.js b/plugins/preset-default.js index b2ed26c..cd8d74e 100644 --- a/plugins/preset-default.js +++ b/plugins/preset-default.js @@ -1,6 +1,7 @@ import { createPreset } from '../lib/svgo/plugins.js'; import * as cleanupIds from './cleanupIds.js'; import * as cleanupStyleAttributes from './cleanupStyleAttributes.js'; +import * as cleanupTextElements from './cleanupTextElements.js'; import * as cleanupXlink from './cleanupXlink.js'; import * as collapseGroups from './collapseGroups.js'; import * as combineStyleElements from './combineStyleElements.js'; @@ -59,6 +60,7 @@ const presetDefault = createPreset({ removeEmptyContainers, removeUnusedNS, createGroups, + cleanupTextElements, ], }); diff --git a/plugins/preset-next.js b/plugins/preset-next.js index f91524e..382901e 100644 --- a/plugins/preset-next.js +++ b/plugins/preset-next.js @@ -1,6 +1,7 @@ import { createPreset } from '../lib/svgo/plugins.js'; import * as cleanupIds from './cleanupIds.js'; import * as cleanupStyleAttributes from './cleanupStyleAttributes.js'; +import * as cleanupTextElements from './cleanupTextElements.js'; import * as cleanupXlink from './cleanupXlink.js'; import * as collapseGroups from './collapseGroups.js'; import * as combineStyleElements from './combineStyleElements.js'; @@ -59,6 +60,7 @@ const presetNext = createPreset({ removeEmptyContainers, removeUnusedNS, createGroups, + cleanupTextElements, ], }); diff --git a/test/plugins/_cleanupTextElements.test.js b/test/plugins/_cleanupTextElements.test.js new file mode 100644 index 0000000..59df691 --- /dev/null +++ b/test/plugins/_cleanupTextElements.test.js @@ -0,0 +1,20 @@ +import { hasSignificantWhiteSpace } from '../../plugins/cleanupTextElements.js'; + +describe('test for significant whitespace', () => { + /** @type {{in:string,out:boolean}[]} */ + const testCases = [ + { in: '', out: false }, + { in: ' \n\n\t', out: false }, + { in: ' x', out: true }, + { in: 'xxxx', out: false }, + { in: 'xx xx', out: false }, + { in: 'xx xx', out: true }, + { in: 'xxxx\n', out: true }, + ]; + + for (const testCase of testCases) { + it(`${JSON.stringify(testCase.in)}`, () => { + expect(hasSignificantWhiteSpace(testCase.in)).toBe(testCase.out); + }); + } +}); diff --git a/test/plugins/cleanupTextElements.01.svg.txt b/test/plugins/cleanupTextElements.01.svg.txt new file mode 100644 index 0000000..25bb9df --- /dev/null +++ b/test/plugins/cleanupTextElements.01.svg.txt @@ -0,0 +1,20 @@ +Remove xml:space when not needed. + +=== + + + + Here is some text + And Here is some more + + + Also down here + + + +@@@ + + + Here is some textAnd Here is some more + Also down here + diff --git a/test/plugins/cleanupTextElements.02.svg.txt b/test/plugins/cleanupTextElements.02.svg.txt new file mode 100644 index 0000000..c39df22 --- /dev/null +++ b/test/plugins/cleanupTextElements.02.svg.txt @@ -0,0 +1,17 @@ +Hoist multiple s if possible. + +=== + + + + Here is some text + And Here is some more + + + +@@@ + + + Here is some text + And Here is some more + diff --git a/test/plugins/cleanupTextElements.03.svg.txt b/test/plugins/cleanupTextElements.03.svg.txt new file mode 100644 index 0000000..a0df56a --- /dev/null +++ b/test/plugins/cleanupTextElements.03.svg.txt @@ -0,0 +1,25 @@ +Don't hoist children of . + +=== + + + + + Here is some text + And Here is some more + + + Here is some text + And Here is some more + + + + +@@@ + + + + Here is some textAnd Here is some more + Here is some textAnd Here is some more + + diff --git a/test/svgo/_index.test.js b/test/svgo/_index.test.js index a5e3ba3..1493d11 100644 --- a/test/svgo/_index.test.js +++ b/test/svgo/_index.test.js @@ -60,14 +60,7 @@ describe('svgo', () => { }); expect(normalize(result.data)).toStrictEqual(expected); }); - it('should preserve whitespaces between tspan tags', async () => { - const [original, expected] = await parseFixture('whitespaces.svg.txt'); - const result = optimize(original, { - path: 'input.svg', - js2svg: { pretty: true }, - }); - expect(normalize(result.data)).toStrictEqual(expected); - }); + it('should preserve "to" keyframe selector', async () => { const [original, expected] = await parseFixture( 'keyframe-selectors.svg.txt', diff --git a/test/svgo/whitespaces.svg.txt b/test/svgo/whitespaces.svg.txt deleted file mode 100644 index fcc2895..0000000 --- a/test/svgo/whitespaces.svg.txt +++ /dev/null @@ -1,15 +0,0 @@ - - - Another tspan - Inside tspan - outside tspan - - - -@@@ - - - - Another tspan - Inside tspan - outside tspan - -