From 5b7c91dc8b202f2ca29de21b2704115415bca7ee Mon Sep 17 00:00:00 2001 From: Seth Falco Date: Sun, 26 Nov 2023 15:06:23 +0000 Subject: [PATCH 1/5] fix(removeHiddenElems): remove hidden definitions usage --- plugins/removeHiddenElems.js | 110 ++++++++++++++++++-------- test/plugins/removeHiddenElems.13.svg | 15 ++++ 2 files changed, 91 insertions(+), 34 deletions(-) create mode 100644 test/plugins/removeHiddenElems.13.svg diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 1e6c8a1eb..c09117000 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -1,6 +1,7 @@ 'use strict'; /** + * @typedef {import('../lib/types').XastChild} XastChild * @typedef {import('../lib/types').XastElement} XastElement * @typedef {import('../lib/types').XastParent} XastParent */ @@ -66,11 +67,40 @@ exports.fn = (root, params) => { */ const nonRenderedNodes = new Map(); + /** + * IDs for removed hidden definitions. + * + * @type {Set} + */ + const removedDefIds = new Set(); + + /** + * @param {XastChild} node + * @param {XastParent} parentNode + */ + function removeElement(node, parentNode) { + if ( + node.type === 'element' && + node.attributes.id != null && + parentNode.type === 'element' && + parentNode.name === 'defs' + ) { + removedDefIds.add(node.attributes.id); + } + + detachNodeFromParent(node, parentNode); + } + visit(root, { element: { enter: (node, parentNode) => { // transparent non-rendering elements still apply where referenced if (nonRendering.includes(node.name)) { + if (node.attributes.id == null) { + detachNodeFromParent(node, parentNode); + return visitSkip; + } + nonRenderedNodes.set(node, parentNode); return visitSkip; } @@ -84,8 +114,7 @@ exports.fn = (root, params) => { computedStyle.opacity.type === 'static' && computedStyle.opacity.value === '0' ) { - detachNodeFromParent(node, parentNode); - return; + removeElement(node, parentNode); } }, }, @@ -105,7 +134,7 @@ exports.fn = (root, params) => { // keep if any descendant enables visibility querySelector(node, '[visibility=visible]') == null ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -122,7 +151,7 @@ exports.fn = (root, params) => { // markers with display: none still rendered node.name !== 'marker' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -138,7 +167,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.r === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -154,7 +183,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.rx === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -170,7 +199,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.ry === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -186,7 +215,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.width === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -203,7 +232,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.height === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -218,7 +247,7 @@ exports.fn = (root, params) => { node.name === 'pattern' && node.attributes.width === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -233,7 +262,7 @@ exports.fn = (root, params) => { node.name === 'pattern' && node.attributes.height === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -248,7 +277,7 @@ exports.fn = (root, params) => { node.name === 'image' && node.attributes.width === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -263,7 +292,7 @@ exports.fn = (root, params) => { node.name === 'image' && node.attributes.height === '0' ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -274,12 +303,12 @@ exports.fn = (root, params) => { // if (pathEmptyD && node.name === 'path') { if (node.attributes.d == null) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } const pathData = parsePathData(node.attributes.d); if (pathData.length === 0) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } // keep single point paths for markers @@ -288,7 +317,7 @@ exports.fn = (root, params) => { computedStyle['marker-start'] == null && computedStyle['marker-end'] == null ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } return; @@ -304,7 +333,7 @@ exports.fn = (root, params) => { node.name === 'polyline' && node.attributes.points == null ) { - detachNodeFromParent(node, parentNode); + removeElement(node, parentNode); return; } @@ -318,31 +347,44 @@ exports.fn = (root, params) => { node.name === 'polygon' && node.attributes.points == null ) { - detachNodeFromParent(node, parentNode); - return; + removeElement(node, parentNode); } }, exit: (node, parentNode) => { - if (node.name !== 'svg' || parentNode.type !== 'root') { + if ( + node.name === 'defs' && + node.children.length === 0 + ) { + removeElement(node, parentNode); return; } - for (const [ - nonRenderedNode, - nonRenderedParent, - ] of nonRenderedNodes.entries()) { - if (nonRenderedNode.attributes.id == null) { - detachNodeFromParent(node, nonRenderedParent); - continue; - } + + if (node.name === 'use') { + const referencesRemovedDef = Object.entries(node.attributes).some( + ([attrKey, attrValue]) => + (attrKey === 'href' || attrKey.endsWith(':href')) && + removedDefIds.has(attrValue.slice(attrValue.indexOf('#') + 1).trim()) + ); - const selector = referencesProps - .map((attr) => `[${attr}="url(#${nonRenderedNode.attributes.id})"]`) - .join(','); + if (referencesRemovedDef) { + detachNodeFromParent(node, parentNode); + } + } - const element = querySelector(root, selector); - if (element == null) { - detachNodeFromParent(node, nonRenderedParent); + if (node.name === 'svg' && parentNode.type === 'root') { + for (const [ + nonRenderedNode, + nonRenderedParent, + ] of nonRenderedNodes.entries()) { + const selector = referencesProps + .map((attr) => `[${attr}="url(#${nonRenderedNode.attributes.id})"]`) + .join(','); + + const element = querySelector(root, selector); + if (element == null) { + detachNodeFromParent(node, nonRenderedParent); + } } } }, diff --git a/test/plugins/removeHiddenElems.13.svg b/test/plugins/removeHiddenElems.13.svg new file mode 100644 index 000000000..3267f9c39 --- /dev/null +++ b/test/plugins/removeHiddenElems.13.svg @@ -0,0 +1,15 @@ +When removing a useless definition, remove references to that definition. + +=== + + + + + + + + + +@@@ + + From 5b1e183df4966ff20803261b7a3cddcb7d1f9624 Mon Sep 17 00:00:00 2001 From: Seth Falco Date: Sun, 26 Nov 2023 15:06:35 +0000 Subject: [PATCH 2/5] chore: increment version to v3.0.5 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bc9eee83e..782e001a3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "packageManager": "yarn@2.4.3", "name": "svgo", - "version": "3.0.4", + "version": "3.0.5", "description": "Nodejs-based tool for optimizing SVG vector graphics files", "license": "MIT", "keywords": [ From 0d7858a217d7a83991d9b674392e13f615680f8a Mon Sep 17 00:00:00 2001 From: Seth Falco Date: Sun, 26 Nov 2023 15:07:13 +0000 Subject: [PATCH 3/5] refactor(removeAttrs): clean up string#join --- plugins/removeAttrs.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/removeAttrs.js b/plugins/removeAttrs.js index c289d8cac..dc1224bd3 100644 --- a/plugins/removeAttrs.js +++ b/plugins/removeAttrs.js @@ -104,13 +104,11 @@ exports.fn = (root, params) => { enter: (node) => { for (let pattern of attrs) { // if no element separators (:), assume it's attribute name, and apply to all elements *regardless of value* - if (pattern.includes(elemSeparator) === false) { - pattern = ['.*', elemSeparator, pattern, elemSeparator, '.*'].join( - '' - ); + if (!pattern.includes(elemSeparator)) { + pattern = ['.*', pattern, '.*'].join(elemSeparator); // if only 1 separator, assume it's element and attribute name, and apply regardless of attribute value } else if (pattern.split(elemSeparator).length < 3) { - pattern = [pattern, elemSeparator, '.*'].join(''); + pattern = [pattern, '.*'].join(elemSeparator); } // create regexps for element, attribute name, and attribute value From 92a8f4d9d06641fee4ff4c22de580bdad5fcef5e Mon Sep 17 00:00:00 2001 From: Seth Falco Date: Sun, 26 Nov 2023 15:07:43 +0000 Subject: [PATCH 4/5] chore: clean up doctypes --- lib/xast.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/xast.js b/lib/xast.js index 595c907f5..98ef5c17a 100644 --- a/lib/xast.js +++ b/lib/xast.js @@ -77,7 +77,8 @@ const visit = (node, visitor, parentNode) => { exports.visit = visit; /** - * @type {(node: XastChild, parentNode: XastParent) => void} + * @param {XastChild} node + * @param {XastParent} parentNode */ const detachNodeFromParent = (node, parentNode) => { // avoid splice to not break for loops From 5199521e76ec5467e730b9bcb2ef28c7f2cf017f Mon Sep 17 00:00:00 2001 From: Seth Falco Date: Sun, 26 Nov 2023 15:07:54 +0000 Subject: [PATCH 5/5] docs: fix typo in reuse-paths --- docs/03-plugins/reuse-paths.mdx | 2 +- lib/xast.js | 4 ++-- plugins/removeHiddenElems.js | 27 +++++++++++++++------------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/docs/03-plugins/reuse-paths.mdx b/docs/03-plugins/reuse-paths.mdx index f787b8ad0..930d04853 100644 --- a/docs/03-plugins/reuse-paths.mdx +++ b/docs/03-plugins/reuse-paths.mdx @@ -12,7 +12,7 @@ If the path contains other attributes, such as `style` or `transform`, they will :::tip -If you only need SVG 2 or inline HTML compatibility, it's recommended to include the [Remove Xlink](/docs/plugins/remove-xlink/) plugin towards the end of your pipeline to convert references to `xlink:href` to the SVG 2 `href` attribute. +If you only need SVG 2 or inline HTML compatibility, it's recommended to include the [Remove XLink](/docs/plugins/remove-xlink/) plugin towards the end of your pipeline to convert references to `xlink:href` to the SVG 2 `href` attribute. ::: diff --git a/lib/xast.js b/lib/xast.js index 98ef5c17a..7f14e2a87 100644 --- a/lib/xast.js +++ b/lib/xast.js @@ -77,8 +77,8 @@ const visit = (node, visitor, parentNode) => { exports.visit = visit; /** - * @param {XastChild} node - * @param {XastParent} parentNode + * @param {XastChild} node + * @param {XastParent} parentNode */ const detachNodeFromParent = (node, parentNode) => { // avoid splice to not break for loops diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index c09117000..ad58324d8 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -69,20 +69,20 @@ exports.fn = (root, params) => { /** * IDs for removed hidden definitions. - * + * * @type {Set} */ const removedDefIds = new Set(); /** - * @param {XastChild} node - * @param {XastParent} parentNode + * @param {XastChild} node + * @param {XastParent} parentNode */ function removeElement(node, parentNode) { if ( node.type === 'element' && node.attributes.id != null && - parentNode.type === 'element' && + parentNode.type === 'element' && parentNode.name === 'defs' ) { removedDefIds.add(node.attributes.id); @@ -352,24 +352,25 @@ exports.fn = (root, params) => { }, exit: (node, parentNode) => { - if ( - node.name === 'defs' && - node.children.length === 0 - ) { + if (node.name === 'defs' && node.children.length === 0) { removeElement(node, parentNode); return; } - + if (node.name === 'use') { const referencesRemovedDef = Object.entries(node.attributes).some( ([attrKey, attrValue]) => (attrKey === 'href' || attrKey.endsWith(':href')) && - removedDefIds.has(attrValue.slice(attrValue.indexOf('#') + 1).trim()) + removedDefIds.has( + attrValue.slice(attrValue.indexOf('#') + 1).trim() + ) ); if (referencesRemovedDef) { detachNodeFromParent(node, parentNode); } + + return; } if (node.name === 'svg' && parentNode.type === 'root') { @@ -378,9 +379,11 @@ exports.fn = (root, params) => { nonRenderedParent, ] of nonRenderedNodes.entries()) { const selector = referencesProps - .map((attr) => `[${attr}="url(#${nonRenderedNode.attributes.id})"]`) + .map( + (attr) => `[${attr}="url(#${nonRenderedNode.attributes.id})"]` + ) .join(','); - + const element = querySelector(root, selector); if (element == null) { detachNodeFromParent(node, nonRenderedParent);