Replace attributes iterators with for/of (svg#1431)

These iterators allows to directly manipulate passed value
which does not let us to get rid from legacy "attrs" field.

Object.entries makes it easier to get an access to both attribute
name and value.

lib/svgo/jsAPI.js

@@ -202,6 +202,7 @@ JSAPI.prototype.hasAttr = function (name, val) {
   if (name == null) {
     return true;
   }
+  // eslint-disable-next-line no-prototype-builtins
   if (this.attributes.hasOwnProperty(name) === false) {
     return false;
   }

plugins/_applyTransforms.js

@@ -25,14 +25,14 @@ const applyTransforms = (elem, pathData, params) => {
   // if there are no 'stroke' attr and references to other objects such as
   // gradiends or clip-path which are also subjects to transform.
   if (
-    !elem.hasAttr('transform') ||
-    !elem.attr('transform').value ||
+    elem.attributes.transform == null ||
+    elem.attributes.transform === '' ||
     // styles are not considered when applying transform
     // can be fixed properly with new style engine
-    elem.hasAttr('style') ||
-    elem.someAttr(
-      (attr) =>
-        referencesProps.includes(attr.name) && attr.value.includes('url(')
+    elem.attributes.style != null ||
+    Object.entries(elem.attributes).some(
+      ([name, value]) =>
+        referencesProps.includes(name) && value.includes('url(')
     )
   ) {
     return;

plugins/cleanupAttrs.js

@@ -28,27 +28,25 @@ var regNewlinesNeedSpace = /(\S)\r?\n(\S)/g,
  */
 exports.fn = function (item, params) {
   if (item.type === 'element') {
-    item.eachAttr(function (attr) {
+    for (const name of Object.keys(item.attributes)) {
       if (params.newlines) {
         // new line which requires a space instead of themselve
-        attr.value = attr.value.replace(
+        item.attributes[name] = item.attributes[name].replace(
           regNewlinesNeedSpace,
-          function (match, p1, p2) {
-            return p1 + ' ' + p2;
-          }
+          (match, p1, p2) => p1 + ' ' + p2
         );
 
         // simple new line
-        attr.value = attr.value.replace(regNewlines, '');
+        item.attributes[name] = item.attributes[name].replace(regNewlines, '');
       }
 
       if (params.trim) {
-        attr.value = attr.value.trim();
+        item.attributes[name] = item.attributes[name].trim();
       }
 
       if (params.spaces) {
-        attr.value = attr.value.replace(regSpaces, ' ');
+        item.attributes[name] = item.attributes[name].replace(regSpaces, ' ');
       }
-    });
+    }
   }
 };

plugins/cleanupIDs.js

@@ -117,8 +117,10 @@ exports.fn = function (data, params) {
    * @return {Array} output items
    */
   function monkeys(items) {
-    for (var i = 0; i < items.children.length && !hasStyleOrScript; i++) {
-      var item = items.children[i];
+    for (const item of items.children) {
+      if (hasStyleOrScript === true) {
+        break;
+      }
 
       // quit if <style> or <script> present ('force' param prevents quitting)
       if (!params.force) {
@@ -144,43 +146,42 @@ exports.fn = function (data, params) {
       }
       // …and don't remove any ID if yes
       if (item.type === 'element') {
-        item.eachAttr(function (attr) {
-          var key, match;
+        for (const [name, value] of Object.entries(item.attributes)) {
+          let key;
+          let match;
 
           // save IDs
-          if (attr.name === 'id') {
-            key = attr.value;
+          if (name === 'id') {
+            key = value;
             if (IDs.has(key)) {
               item.removeAttr('id'); // remove repeated id
             } else {
               IDs.set(key, item);
             }
-            return;
-          }
-          // save references
-          const { local } = parseName(attr.name);
-          if (
-            referencesProps.has(attr.name) &&
-            (match = attr.value.match(regReferencesUrl))
-          ) {
-            key = match[2]; // url() reference
-          } else if (
-            (local === 'href' &&
-              (match = attr.value.match(regReferencesHref))) ||
-            (attr.name === 'begin' &&
-              (match = attr.value.match(regReferencesBegin)))
-          ) {
-            key = match[1]; // href reference
-          }
-          if (key) {
-            var ref = referencesIDs.get(key) || [];
-            ref.push(attr);
-            referencesIDs.set(key, ref);
+          } else {
+            // save references
+            const { local } = parseName(name);
+            if (
+              referencesProps.has(name) &&
+              (match = value.match(regReferencesUrl))
+            ) {
+              key = match[2]; // url() reference
+            } else if (
+              (local === 'href' && (match = value.match(regReferencesHref))) ||
+              (name === 'begin' && (match = value.match(regReferencesBegin)))
+            ) {
+              key = match[1]; // href reference
+            }
+            if (key) {
+              const refs = referencesIDs.get(key) || [];
+              refs.push({ element: item, name, value });
+              referencesIDs.set(key, refs);
+            }
           }
-        });
+        }
       }
       // go deeper
-      if (item.children) {
+      if (item.type === 'root' || item.type === 'element') {
         monkeys(item);
       }
     }
@@ -196,9 +197,7 @@ exports.fn = function (data, params) {
   const idPreserved = (id) =>
     preserveIDs.has(id) || idMatchesPrefix(preserveIDPrefixes, id);
 
-  for (var ref of referencesIDs) {
-    var key = ref[0];
-
+  for (const [key, refs] of referencesIDs) {
     if (IDs.has(key)) {
       // replace referenced IDs with the minified ones
       if (params.minify && !idPreserved(key)) {
@@ -211,13 +210,13 @@ exports.fn = function (data, params) {
 
         IDs.get(key).attr('id').value = currentIDstring;
 
-        for (var attr of ref[1]) {
-          attr.value = attr.value.includes(idValuePrefix)
-            ? attr.value.replace(
+        for (const { element, name, value } of refs) {
+          element.attributes[name] = value.includes(idValuePrefix)
+            ? value.replace(
                 idValuePrefix + key,
                 idValuePrefix + currentIDstring
               )
-            : attr.value.replace(
+            : value.replace(
                 key + idValuePostfix,
                 currentIDstring + idValuePostfix
               );

plugins/cleanupNumericValues.js

@@ -49,13 +49,13 @@ exports.fn = function (item, params) {
         .join(' ');
     }
 
-    item.eachAttr(function (attr) {
+    for (const [name, value] of Object.entries(item.attributes)) {
       // The `version` attribute is a text string and cannot be rounded
-      if (attr.name === 'version') {
-        return;
+      if (name === 'version') {
+        continue;
       }
 
-      var match = attr.value.match(regNumericValues);
+      var match = value.match(regNumericValues);
 
       // if attribute value matches regNumericValues
       if (match) {
@@ -85,8 +85,8 @@ exports.fn = function (item, params) {
           units = '';
         }
 
-        attr.value = num + units;
+        item.attributes[name] = num + units;
       }
-    });
+    }
   }
 };

plugins/collapseGroups.js

@@ -66,25 +66,24 @@ exports.fn = function (item) {
                 !g.hasAttr('transform') &&
                 !inner.hasAttr('transform')))
           ) {
-            g.eachAttr(function (attr) {
-              if (g.children.some(hasAnimatedAttr, attr.name)) return;
+            for (const [name, value] of Object.entries(g.attributes)) {
+              if (g.children.some(hasAnimatedAttr, name)) return;
 
-              if (!inner.hasAttr(attr.name)) {
-                inner.addAttr(attr);
-              } else if (attr.name == 'transform') {
-                inner.attr(attr.name).value =
-                  attr.value + ' ' + inner.attr(attr.name).value;
-              } else if (inner.hasAttr(attr.name, 'inherit')) {
-                inner.attr(attr.name).value = attr.value;
+              if (!inner.hasAttr(name)) {
+                inner.attributes[name] = value;
+              } else if (name == 'transform') {
+                inner.attributes[name] = value + ' ' + inner.attributes[name];
+              } else if (inner.hasAttr(name, 'inherit')) {
+                inner.attributes[name] = value;
               } else if (
-                attrsInheritable.indexOf(attr.name) < 0 &&
-                !inner.hasAttr(attr.name, attr.value)
+                attrsInheritable.includes(name) === false &&
+                !inner.hasAttr(name, value)
               ) {
                 return;
               }
 
-              g.removeAttr(attr.name);
-            });
+              g.removeAttr(name);
+            }
           }
         }
 

plugins/convertColors.js

@@ -51,10 +51,10 @@ var collections = require('./_collections'),
  */
 exports.fn = function (item, params) {
   if (item.type === 'element') {
-    item.eachAttr(function (attr) {
-      if (collections.colorsProps.indexOf(attr.name) > -1) {
-        var val = attr.value,
-          match;
+    for (const [name, value] of Object.entries(item.attributes)) {
+      if (collections.colorsProps.includes(name)) {
+        let val = value;
+        let match;
 
         // Convert colors to currentColor
         if (params.currentColor) {
@@ -99,9 +99,9 @@ exports.fn = function (item, params) {
           }
         }
 
-        attr.value = val;
+        item.attributes[name] = val;
       }
-    });
+    }
   }
 };
 

plugins/moveGroupAttrsToElems.js

@@ -34,26 +34,23 @@ exports.fn = function (item) {
   if (
     item.isElem('g') &&
     item.children.length !== 0 &&
-    item.hasAttr('transform') &&
-    !item.someAttr(function (attr) {
-      return ~referencesProps.indexOf(attr.name) && ~attr.value.indexOf('url(');
-    }) &&
-    item.children.every(function (inner) {
-      return inner.isElem(pathElems) && !inner.hasAttr('id');
-    })
+    item.attributes.transform != null &&
+    Object.entries(item.attributes).some(
+      ([name, value]) =>
+        referencesProps.includes(name) && value.includes('url(')
+    ) === false &&
+    item.children.every(
+      (inner) => inner.isElem(pathElems) && !inner.hasAttr('id')
+    )
   ) {
-    item.children.forEach(function (inner) {
-      var attr = item.attr('transform');
-      if (inner.hasAttr('transform')) {
-        inner.attr('transform').value =
-          attr.value + ' ' + inner.attr('transform').value;
+    for (const inner of item.children) {
+      const value = item.attributes.transform;
+      if (inner.attributes.transform != null) {
+        inner.attributes.transform = value + ' ' + inner.attributes.transform;
       } else {
-        inner.addAttr({
-          name: attr.name,
-          value: attr.value,
-        });
+        inner.attributes.transform = value;
       }
-    });
+    }
 
     item.removeAttr('transform');
   }

plugins/removeAttrs.js

@@ -124,9 +124,7 @@ exports.fn = function (item, params) {
       // matches element
       if (pattern[0].test(item.name)) {
         // loop attributes
-        item.eachAttr(function (attr) {
-          var name = attr.name;
-          var value = attr.value;
+        for (const [name, value] of Object.entries(item.attributes)) {
           var isFillCurrentColor =
             preserveCurrentColor && name == 'fill' && value == 'currentColor';
           var isStrokeCurrentColor =
@@ -136,12 +134,12 @@ exports.fn = function (item, params) {
             // matches attribute name
             if (pattern[1].test(name)) {
               // matches attribute value
-              if (pattern[2].test(attr.value)) {
+              if (pattern[2].test(value)) {
                 item.removeAttr(name);
               }
             }
           }
-        });
+        }
       }
     });
   }

plugins/removeEditorsNSData.js

@@ -36,24 +36,24 @@ exports.fn = function (item, params) {
 
   if (item.type === 'element') {
     if (item.isElem('svg')) {
-      item.eachAttr(function (attr) {
-        const { prefix, local } = parseName(attr.name);
-        if (prefix === 'xmlns' && editorNamespaces.includes(attr.value)) {
+      for (const [name, value] of Object.entries(item.attributes)) {
+        const { prefix, local } = parseName(name);
+        if (prefix === 'xmlns' && editorNamespaces.includes(value)) {
           prefixes.push(local);
 
           // <svg xmlns:sodipodi="">
-          item.removeAttr(attr.name);
+          item.removeAttr(name);
         }
-      });
+      }
     }
 
     // <* sodipodi:*="">
-    item.eachAttr(function (attr) {
-      const { prefix } = parseName(attr.name);
+    for (const name of Object.keys(item.attributes)) {
+      const { prefix } = parseName(name);
       if (prefixes.includes(prefix)) {
-        item.removeAttr(attr.name);
+        item.removeAttr(name);
       }
-    });
+    }
 
     // <sodipodi:*>
     const { prefix } = parseName(item.name);

plugins/removeEmptyAttrs.js

@@ -18,14 +18,14 @@ exports.description = 'removes empty attributes';
  */
 exports.fn = function (item) {
   if (item.type === 'element') {
-    item.eachAttr(function (attr) {
+    for (const [name, value] of Object.entries(item.attributes)) {
       if (
-        attr.value === '' &&
+        value === '' &&
         // empty conditional processing attributes prevents elements from rendering
-        attrsGroups.conditionalProcessing.includes(attr.name) === false
+        attrsGroups.conditionalProcessing.includes(name) === false
       ) {
-        item.removeAttr(attr.name);
+        item.removeAttr(name);
       }
-    });
+    }
   }
 };