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

@@ -81,14 +81,12 @@ const computeOwnStyle = (node, stylesheet) => {
   const importantStyles = new Map();
 
   // collect attributes
-  if (node.attrs) {
-    for (const { name, value } of Object.values(node.attrs)) {
+  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);
     }
   }
-  }
 
   // collect matching rules
   for (const { selectors, declarations, dynamic } of stylesheet) {
@@ -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) {

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(' ');

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();
 };

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;
       }
     }
 

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;
     }
   }
 

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 () {