From 35b7356ff0c0f03e6ec5947e8a2eb30e126d11b9 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Thu, 12 Aug 2021 13:05:09 +0300 Subject: [PATCH] Pass parent node to visitor (#1517) Wrapping each node with JSAPI class and passing parent to it is not reliable. Parent may be changed but the reference will stay. Here I wasn't able to detach comment from parent node for some reason. Explicit parent node inferred while ast traverse is easier to debug and work with. Eventually we will not need to wrap each node with JSAPI class. --- lib/xast.js | 15 +++++++-------- lib/xast.test.js | 4 ++-- plugins/mergePaths.js | 2 +- plugins/mergeStyles.js | 6 +++--- plugins/removeComments.js | 21 ++++++++++++--------- plugins/removeHiddenElems.js | 36 ++++++++++++++++++------------------ 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/lib/xast.js b/lib/xast.js index 9800e2ac..a9ecf928 100644 --- a/lib/xast.js +++ b/lib/xast.js @@ -52,34 +52,33 @@ const traverse = (node, fn) => { }; exports.traverse = traverse; -const visit = (node, visitor) => { +const visit = (node, visitor, parentNode = null) => { const callbacks = visitor[node.type]; if (callbacks && callbacks.enter) { - callbacks.enter(node); + callbacks.enter(node, parentNode); } // 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(child, visitor, node); } } // visit element children if still attached to parent if (node.type === 'element') { - if (node.parentNode.children.includes(node)) { + if (parentNode.children.includes(node)) { for (const child of node.children) { - visit(child, visitor); + visit(child, visitor, node); } } } if (callbacks && callbacks.exit) { - callbacks.exit(node); + callbacks.exit(node, parentNode); } }; exports.visit = visit; -const detachNodeFromParent = (node) => { - const parentNode = node.parentNode; +const detachNodeFromParent = (node, parentNode) => { // avoid splice to not break for loops parentNode.children = parentNode.children.filter((child) => child !== node); }; diff --git a/lib/xast.test.js b/lib/xast.test.js index 0008b936..80d60007 100644 --- a/lib/xast.test.js +++ b/lib/xast.test.js @@ -95,10 +95,10 @@ describe('xast', () => { const entered = []; visit(root, { element: { - enter: (node) => { + enter: (node, parentNode) => { entered.push(node.name); if (node.name === 'g') { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); } }, }, diff --git a/plugins/mergePaths.js b/plugins/mergePaths.js index cd5863e1..65083699 100644 --- a/plugins/mergePaths.js +++ b/plugins/mergePaths.js @@ -88,7 +88,7 @@ exports.fn = (root, params) => { floatPrecision, noSpaceAfterFlags, }); - detachNodeFromParent(child); + detachNodeFromParent(child, node); continue; } diff --git a/plugins/mergeStyles.js b/plugins/mergeStyles.js index c57a5029..b5eaccaa 100644 --- a/plugins/mergeStyles.js +++ b/plugins/mergeStyles.js @@ -17,7 +17,7 @@ exports.fn = () => { let collectedStyles = ''; let styleContentType = 'text'; - const enterElement = (node) => { + const enterElement = (node, parentNode) => { // collect style elements if (node.name !== 'style') { return; @@ -51,7 +51,7 @@ exports.fn = () => { // remove empty style elements if (css.trim().length === 0) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -67,7 +67,7 @@ exports.fn = () => { if (firstStyleElement == null) { firstStyleElement = node; } else { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); firstStyleElement.children = [ new JSAPI( { type: styleContentType, value: collectedStyles }, diff --git a/plugins/removeComments.js b/plugins/removeComments.js index 191618f7..d7632239 100644 --- a/plugins/removeComments.js +++ b/plugins/removeComments.js @@ -1,9 +1,9 @@ 'use strict'; -exports.type = 'perItem'; +const { detachNodeFromParent } = require('../lib/xast.js'); +exports.type = 'visitor'; exports.active = true; - exports.description = 'removes comments'; /** @@ -13,13 +13,16 @@ exports.description = 'removes comments'; * * - * @param {Object} item current iteration item - * @return {Boolean} if false, item will be filtered out - * * @author Kir Belevich */ -exports.fn = function (item) { - if (item.type === 'comment' && item.value.charAt(0) !== '!') { - return false; - } +exports.fn = () => { + return { + comment: { + enter: (node, parentNode) => { + if (node.value.charAt(0) !== '!') { + detachNodeFromParent(node, parentNode); + } + }, + }, + }; }; diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 8d7be78c..e5f55c6f 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -53,7 +53,7 @@ exports.fn = (root, params) => { return { element: { - enter: (node) => { + enter: (node, parentNode) => { // Removes hidden elements // https://www.w3schools.com/cssref/pr_class_visibility.asp const computedStyle = computeStyle(stylesheet, node); @@ -65,7 +65,7 @@ exports.fn = (root, params) => { // keep if any descendant enables visibility querySelector(node, '[visibility=visible]') == null ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -82,7 +82,7 @@ exports.fn = (root, params) => { // markers with display: none still rendered node.name !== 'marker' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -97,7 +97,7 @@ exports.fn = (root, params) => { // transparent element inside clipPath still affect clipped elements closestByName(node, 'clipPath') == null ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -113,7 +113,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.r === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -129,7 +129,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.rx === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -145,7 +145,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.ry === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -161,7 +161,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.width === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -178,7 +178,7 @@ exports.fn = (root, params) => { node.children.length === 0 && node.attributes.height === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -193,7 +193,7 @@ exports.fn = (root, params) => { node.name === 'pattern' && node.attributes.width === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -208,7 +208,7 @@ exports.fn = (root, params) => { node.name === 'pattern' && node.attributes.height === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -223,7 +223,7 @@ exports.fn = (root, params) => { node.name === 'image' && node.attributes.width === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -238,7 +238,7 @@ exports.fn = (root, params) => { node.name === 'image' && node.attributes.height === '0' ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -249,12 +249,12 @@ exports.fn = (root, params) => { // if (pathEmptyD && node.name === 'path') { if (node.attributes.d == null) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } const pathData = parsePathData(node.attributes.d); if (pathData.length === 0) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } // keep single point paths for markers @@ -263,7 +263,7 @@ exports.fn = (root, params) => { computedStyle['marker-start'] == null && computedStyle['marker-end'] == null ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } return; @@ -279,7 +279,7 @@ exports.fn = (root, params) => { node.name === 'polyline' && node.attributes.points == null ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } @@ -293,7 +293,7 @@ exports.fn = (root, params) => { node.name === 'polygon' && node.attributes.points == null ) { - detachNodeFromParent(node); + detachNodeFromParent(node, parentNode); return; } },