From 3e1bfd528db8fc2f945843f39b1d50b331993893 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 10 Mar 2021 23:11:09 +0300 Subject: [PATCH] Convert attributes to xast with proxy fallback There is a lot of attributes manipulation which is hard to remove at once. In this diff I added `attributes` object and wrapped it as proxy for `attrs` field. --- lib/style.js | 19 ++++++----- lib/svgo/css-class-list.js | 33 +------------------- lib/svgo/css-style-declaration.js | 34 +------------------- lib/svgo/svg2js.js | 52 ++++++++++++++++++++++++++++--- plugins/moveElemsAttrsToGroup.js | 26 +++++++++------- test/svg2js/_index.js | 4 +-- 6 files changed, 74 insertions(+), 94 deletions(-) diff --git a/lib/style.js b/lib/style.js index 68a74248..c737cf6a 100644 --- a/lib/style.js +++ b/lib/style.js @@ -81,12 +81,10 @@ const computeOwnStyle = (node, stylesheet) => { const importantStyles = new Map(); // collect attributes - if (node.attrs) { - for (const { name, value } of Object.values(node.attrs)) { - if (attrsGroups.presentation.includes(name)) { - computedStyle[name] = { type: 'static', inherited: false, value }; - importantStyles.set(name, false); - } + for (const [name, value] of Object.entries(node.attributes)) { + if (attrsGroups.presentation.includes(name)) { + computedStyle[name] = { type: 'static', inherited: false, value }; + importantStyles.set(name, false); } } @@ -146,11 +144,12 @@ const computeStyle = (node) => { const stylesheet = []; for (const styleNode of styleNodes) { const dynamic = - styleNode.hasAttr('media') && styleNode.attr('media').value !== 'all'; + styleNode.attributes.media != null && + styleNode.attributes.media !== 'all'; if ( - styleNode.hasAttr('type') === false || - styleNode.attr('type').value === '' || - styleNode.attr('type').value === 'text/css' + styleNode.attributes.type == null || + styleNode.attributes.type === '' || + styleNode.attributes.type === 'text/css' ) { const children = styleNode.content || []; for (const child of children) { diff --git a/lib/svgo/css-class-list.js b/lib/svgo/css-class-list.js index 7cf38b5a..08d94167 100644 --- a/lib/svgo/css-class-list.js +++ b/lib/svgo/css-class-list.js @@ -3,7 +3,6 @@ var CSSClassList = function (node) { this.parentNode = node; this.classNames = new Set(); - this.classAttr = null; //this.classValue = null; }; @@ -31,32 +30,13 @@ CSSClassList.prototype.clone = function (parentNode) { }; CSSClassList.prototype.hasClass = function () { - this.classAttr = { - // empty class attr - name: 'class', - value: null, - }; - - this.addClassHandler(); -}; - -// attr.class - -CSSClassList.prototype.addClassHandler = function () { - Object.defineProperty(this.parentNode.attrs, 'class', { - get: this.getClassAttr.bind(this), - set: this.setClassAttr.bind(this), - enumerable: true, - configurable: true, - }); - this.addClassValueHandler(); }; // attr.class.value CSSClassList.prototype.addClassValueHandler = function () { - Object.defineProperty(this.classAttr, 'value', { + Object.defineProperty(this.parentNode.attributes, 'class', { get: this.getClassValue.bind(this), set: this.setClassValue.bind(this), enumerable: true, @@ -64,17 +44,6 @@ CSSClassList.prototype.addClassValueHandler = function () { }); }; -CSSClassList.prototype.getClassAttr = function () { - return this.classAttr; -}; - -CSSClassList.prototype.setClassAttr = function (newClassAttr) { - this.setClassValue(newClassAttr.value); // must before applying value handler! - - this.classAttr = newClassAttr; - this.addClassValueHandler(); -}; - CSSClassList.prototype.getClassValue = function () { var arrClassNames = Array.from(this.classNames); return arrClassNames.join(' '); diff --git a/lib/svgo/css-style-declaration.js b/lib/svgo/css-style-declaration.js index 3bffcf9c..cf516942 100644 --- a/lib/svgo/css-style-declaration.js +++ b/lib/svgo/css-style-declaration.js @@ -9,7 +9,6 @@ var CSSStyleDeclaration = function (node) { this.properties = new Map(); this.hasSynced = false; - this.styleAttr = null; this.styleValue = null; this.parseError = false; @@ -39,32 +38,13 @@ CSSStyleDeclaration.prototype.clone = function (parentNode) { }; CSSStyleDeclaration.prototype.hasStyle = function () { - this.addStyleHandler(); -}; - -// attr.style - -CSSStyleDeclaration.prototype.addStyleHandler = function () { - this.styleAttr = { - // empty style attr - name: 'style', - value: null, - }; - - Object.defineProperty(this.parentNode.attrs, 'style', { - get: this.getStyleAttr.bind(this), - set: this.setStyleAttr.bind(this), - enumerable: true, - configurable: true, - }); - this.addStyleValueHandler(); }; // attr.style.value CSSStyleDeclaration.prototype.addStyleValueHandler = function () { - Object.defineProperty(this.styleAttr, 'value', { + Object.defineProperty(this.parentNode.attributes, 'style', { get: this.getStyleValue.bind(this), set: this.setStyleValue.bind(this), enumerable: true, @@ -72,18 +52,6 @@ CSSStyleDeclaration.prototype.addStyleValueHandler = function () { }); }; -CSSStyleDeclaration.prototype.getStyleAttr = function () { - return this.styleAttr; -}; - -CSSStyleDeclaration.prototype.setStyleAttr = function (newStyleAttr) { - this.setStyleValue(newStyleAttr.value); // must before applying value handler! - - this.styleAttr = newStyleAttr; - this.addStyleValueHandler(); - this.hasSynced = false; // raw css changed -}; - CSSStyleDeclaration.prototype.getStyleValue = function () { return this.getCssText(); }; diff --git a/lib/svgo/svg2js.js b/lib/svgo/svg2js.js index d3eb3211..0adefb65 100644 --- a/lib/svgo/svg2js.js +++ b/lib/svgo/svg2js.js @@ -78,10 +78,55 @@ module.exports = function (data) { }; sax.onopentag = function (data) { + const attrsHandler = { + get: (attributes, name) => { + if (attributes.hasOwnProperty(name)) { + return new Proxy( + { + name, + value: attributes[name], + }, + { + get: (target, property) => { + if (property === 'name') { + return name; + } + if (property === 'value') { + return attributes[name]; + } + }, + set: (target, property, value) => { + if (property === 'value') { + attributes[name] = value; + return true; + } + }, + } + ); + } + }, + set: (attributes, name, attr) => { + attributes[name] = attr.value; + return true; + }, + }; + var element = { type: 'element', name: data.name, - attrs: {}, + attributes: {}, + // temporary attrs polyfill + // TODO remove after migration + get attrs() { + return new Proxy(element.attributes, attrsHandler); + }, + set attrs(value) { + const newAttributes = {}; + for (const attr of Object.values(value)) { + newAttributes[attr.name] = attr.value; + } + element.attributes = newAttributes; + }, content: [], }; @@ -100,10 +145,7 @@ module.exports = function (data) { element.style.hasStyle(); } - element.attrs[name] = { - name: name, - value: attr.value, - }; + element.attributes[name] = attr.value; } } diff --git a/plugins/moveElemsAttrsToGroup.js b/plugins/moveElemsAttrsToGroup.js index f6addb99..4a6c799d 100644 --- a/plugins/moveElemsAttrsToGroup.js +++ b/plugins/moveElemsAttrsToGroup.js @@ -43,9 +43,12 @@ exports.fn = function (item) { // don't mess with possible styles (hack until CSS parsing is implemented) if (inner.hasAttr('class')) return false; if (!Object.keys(intersection).length) { - intersection = inner.attrs; + intersection = inner.attributes; } else { - intersection = intersectInheritableAttrs(intersection, inner.attrs); + intersection = intersectInheritableAttrs( + intersection, + inner.attributes + ); if (!intersection) return false; } @@ -59,22 +62,22 @@ exports.fn = function (item) { if (intersected) { item.content.forEach(function (g) { - for (const [name, attr] of Object.entries(intersection)) { + for (const [name, value] of Object.entries(intersection)) { if ((!allPath && !hasClip) || name !== 'transform') { g.removeAttr(name); if (name === 'transform') { if (!hasTransform) { if (item.hasAttr('transform')) { - item.attr('transform').value += ' ' + attr.value; + item.attr('transform').value += ' ' + value; } else { - item.addAttr(attr); + item.addAttr({ name, value }); } hasTransform = true; } } else { - item.addAttr(attr); + item.addAttr({ name, value }); } } } @@ -94,15 +97,14 @@ exports.fn = function (item) { function intersectInheritableAttrs(a, b) { var c = {}; - for (const [n, attr] of Object.entries(a)) { + for (const [name, value] of Object.entries(a)) { if ( // eslint-disable-next-line no-prototype-builtins - b.hasOwnProperty(n) && - inheritableAttrs.indexOf(n) > -1 && - attr.name === b[n].name && - attr.value === b[n].value + b.hasOwnProperty(name) && + inheritableAttrs.includes(name) && + value === b[name] ) { - c[n] = attr; + c[name] = value; } } diff --git a/test/svg2js/_index.js b/test/svg2js/_index.js index 0b5287dc..14563989 100644 --- a/test/svg2js/_index.js +++ b/test/svg2js/_index.js @@ -324,11 +324,11 @@ describe('svg2js', function () { it('svg.content[0].eachAttr(function() {}) should be true', function () { expect( root.content[3].content[0].eachAttr(function (attr) { - attr.test = 1; + attr.value = '1'; }) ).to.be.true; - expect(root.content[3].content[0].attr('type').test).to.equal(1); + expect(root.content[3].content[0].attr('type').value).to.equal('1'); }); it('svg.content[1].eachAttr(function() {}) should be false', function () {