refactor(svgImport): Remove storage + cssRules, gradientDefs, clipPat…

…hDefs globals (#9030)
fabricjs · Jul 1, 2023 · 951e760 · 951e760
1 parent 42b8259
commit 951e760
Show file tree

Hide file tree

Showing 17 changed files with 192 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## [next]
 
+- refactor(svImport): remove the css/gradient/clipPath global definitions [#9030](https://github.com/fabricjs/fabric.js/pull/9030)
 - fix(): tweaks to type getter [#9022](https://github.com/fabricjs/fabric.js/pull/9022)
 - ci() Refactor GHA actions for caching and reuse [#9029](https://github.com/fabricjs/fabric.js/pull/9029)
 - ci(): install dev deps types [#9039](https://github.com/fabricjs/fabric.js/pull/9039)

diff --git a/src/parser/constants.ts b/src/parser/constants.ts
@@ -1,17 +1,6 @@
-//@ts-nocheck
 import { getSvgRegex } from './getSvgRegex';
 import { LEFT, TOP } from '../constants';
 
-export const cssRules = {};
-export const gradientDefs = {};
-export const clipPaths = {};
-
-export const storage = {
-  cssRules,
-  gradientDefs,
-  clipPaths,
-};
-
 export const reNum = String.raw`(?:[-+]?(?:\d*\.\d+|\d+\.?)(?:[eE][-+]?\d+)?)`;
 
 export const svgNS = 'http://www.w3.org/2000/svg';

diff --git a/src/parser/elements_parser.ts b/src/parser/elements_parser.ts
@@ -8,11 +8,12 @@ import {
   multiplyTransformMatrices,
   qrDecompose,
 } from '../util/misc/matrix';
-import { storage } from './constants';
 import { removeTransformMatrixForSvgParsing } from '../util/transform_matrix_removal';
 import type { FabricObject } from '../shapes/Object/FabricObject';
 import { Point } from '../Point';
 import { CENTER } from '../constants';
+import { getGradientDefs } from './getGradientDefs';
+import { getCSSRules } from './getCSSRules';
 
 const findTag = (el: HTMLElement) =>
   classRegistry.getSVGClass(el.tagName.toLowerCase().replace('svg:', ''));
@@ -22,22 +23,24 @@ const ElementsParser = function (
   options,
   reviver,
   parsingOptions,
-  doc
+  doc,
+  clipPaths
 ) {
   this.elements = elements;
   this.options = options;
   this.reviver = reviver;
-  this.svgUid = (options && options.svgUid) || 0;
   this.parsingOptions = parsingOptions;
   this.regexUrl = /^url\(['"]?#([^'"]+)['"]?\)/g;
   this.doc = doc;
+  this.clipPaths = clipPaths;
+  this.gradientDefs = getGradientDefs(doc);
+  this.cssRules = getCSSRules(doc);
 };
 
 (function (proto) {
   proto.parse = function (): Promise<FabricObject[]> {
     return Promise.all(
       this.elements.map((element: HTMLElement, i) => {
-        element.setAttribute('svgUid', this.svgUid);
         return this.createObject(element);
       })
     );
@@ -46,7 +49,7 @@ const ElementsParser = function (
   proto.createObject = async function (el: HTMLElement): Promise<FabricObject> {
     const klass = findTag(el);
     if (klass) {
-      const obj = await klass.fromElement(el, this.options);
+      const obj = await klass.fromElement(el, this.options, this.cssRules);
       let _options;
       this.resolveGradient(obj, el, 'fill');
       this.resolveGradient(obj, el, 'stroke');
@@ -61,7 +64,7 @@ const ElementsParser = function (
     return null;
   };
 
-  proto.extractPropertyDefinition = function (obj, property, storageType) {
+  proto.extractPropertyDefinition = function (obj, property, storage) {
     const value = obj[property],
       regex = this.regexUrl;
     if (!regex.test(value)) {
@@ -71,14 +74,14 @@ const ElementsParser = function (
     const id = regex.exec(value)[1];
     regex.lastIndex = 0;
     // @todo fix this
-    return storage[storageType][this.svgUid][id];
+    return storage[id];
   };
 
   proto.resolveGradient = function (obj, el, property) {
     const gradientDef = this.extractPropertyDefinition(
       obj,
       property,
-      'gradientDefs'
+      this.gradientDefs
     );
     if (gradientDef) {
       const opacityAttr = el.getAttribute(property + '-opacity');
@@ -94,7 +97,7 @@ const ElementsParser = function (
     const clipPathElements = this.extractPropertyDefinition(
       obj,
       'clipPath',
-      'clipPaths'
+      this.clipPaths
     );
     if (clipPathElements) {
       const objTransformInv = invertTransform(obj.calcTransformMatrix());
@@ -111,7 +114,7 @@ const ElementsParser = function (
       const container = await Promise.all(
         clipPathElements.map((clipPathElement) => {
           return findTag(clipPathElement)
-            .fromElement(clipPathElement, this.options)
+            .fromElement(clipPathElement, this.options, this.cssRules)
             .then((enlivedClippath) => {
               removeTransformMatrixForSvgParsing(enlivedClippath);
               enlivedClippath.fillRule = enlivedClippath.clipRule;

diff --git a/src/parser/getCSSRules.ts b/src/parser/getCSSRules.ts
@@ -1,14 +1,15 @@
+import type { CSSRules } from './typedefs';
+
 /**
  * Returns CSS rules for a given SVG document
- * @param {SVGDocument} doc SVG document to parse
+ * @param {HTMLElement} doc SVG document to parse
  * @return {Object} CSS rules of this document
  */
-export function getCSSRules(doc: Document) {
+export function getCSSRules(doc: HTMLElement) {
   const styles = doc.getElementsByTagName('style');
   let i;
   let len;
-  const allRules: Record<string, Record<string, string>> = {};
-  let rules;
+  const allRules: CSSRules = {};
 
   // very crude parsing of style contents
   for (i = 0, len = styles.length; i < len; i++) {
@@ -23,39 +24,40 @@ export function getCSSRules(doc: Document) {
     }
     // recovers all the rule in this form `body { style code... }`
     // rules = styleContents.match(/[^{]*\{[\s\S]*?\}/g);
-    rules = styleContents.split('}');
-    // remove empty rules.
-    rules = rules.filter(function (rule) {
-      return rule.trim();
-    });
-    // at this point we have hopefully an array of rules `body { style code... `
-    // eslint-disable-next-line no-loop-func
-    rules.forEach(function (rule) {
-      const match = rule.split('{'),
-        ruleObj: Record<string, string> = {},
-        declaration = match[1].trim(),
-        propertyValuePairs = declaration.split(';').filter(function (pair) {
-          return pair.trim();
-        });
+    styleContents
+      .split('}')
+      // remove empty rules.
+      .filter(function (rule) {
+        return rule.trim();
+      })
+      // at this point we have hopefully an array of rules `body { style code... `
+      // eslint-disable-next-line no-loop-func
+      .forEach(function (rule) {
+        const match = rule.split('{'),
+          ruleObj: Record<string, string> = {},
+          declaration = match[1].trim(),
+          propertyValuePairs = declaration.split(';').filter(function (pair) {
+            return pair.trim();
+          });
 
-      for (i = 0, len = propertyValuePairs.length; i < len; i++) {
-        const pair = propertyValuePairs[i].split(':'),
-          property = pair[0].trim(),
-          value = pair[1].trim();
-        ruleObj[property] = value;
-      }
-      rule = match[0].trim();
-      rule.split(',').forEach((_rule) => {
-        _rule = _rule.replace(/^svg/i, '').trim();
-        if (_rule === '') {
-          return;
+        for (i = 0, len = propertyValuePairs.length; i < len; i++) {
+          const pair = propertyValuePairs[i].split(':'),
+            property = pair[0].trim(),
+            value = pair[1].trim();
+          ruleObj[property] = value;
         }
-        allRules[_rule] = {
-          ...(allRules[_rule] || {}),
-          ...ruleObj,
-        };
+        rule = match[0].trim();
+        rule.split(',').forEach((_rule) => {
+          _rule = _rule.replace(/^svg/i, '').trim();
+          if (_rule === '') {
+            return;
+          }
+          allRules[_rule] = {
+            ...(allRules[_rule] || {}),
+            ...ruleObj,
+          };
+        });
       });
-    });
   }
   return allRules;
 }
diff --git a/src/parser/getGlobalStylesForElement.ts b/src/parser/getGlobalStylesForElement.ts
@@ -1,18 +1,21 @@
-//@ts-nocheck
-import { cssRules } from './constants';
 import { elementMatchesRule } from './elementMatchesRule';
+import type { CSSRules } from './typedefs';
 
 /**
  * @private
  */
 
-export function getGlobalStylesForElement(element, svgUid) {
-  const styles = {};
-  for (const rule in cssRules[svgUid]) {
+export function getGlobalStylesForElement(
+  element: HTMLElement,
+  cssRules: CSSRules = {}
+) {
+  let styles: Record<string, string> = {};
+  for (const rule in cssRules) {
     if (elementMatchesRule(element, rule.split(' '))) {
-      for (const property in cssRules[svgUid][rule]) {
-        styles[property] = cssRules[svgUid][rule][property];
-      }
+      styles = {
+        ...styles,
+        ...cssRules[rule],
+      };
     }
   }
   return styles;

diff --git a/src/parser/parseAttributes.ts b/src/parser/parseAttributes.ts
@@ -1,4 +1,3 @@
-//@ts-nocheck
 import { DEFAULT_SVG_FONT_SIZE } from '../constants';
 import { parseUnit } from '../util/misc/svgParsing';
 import { cPath, fSize, svgValidParentsRegEx } from './constants';
@@ -8,6 +7,7 @@ import { normalizeValue } from './normalizeValue';
 import { parseFontDeclaration } from './parseFontDeclaration';
 import { parseStyleAttribute } from './parseStyleAttribute';
 import { setStrokeFillOpacity } from './setStrokeFillOpacity';
+import type { CSSRules } from './typedefs';
 
 /**
  * Returns an object of attributes' name/value, given element and an array of attribute names;
@@ -17,59 +17,58 @@ import { setStrokeFillOpacity } from './setStrokeFillOpacity';
  * @return {Object} object containing parsed attributes' names/values
  */
 export function parseAttributes(
-  element: SVGElement | HTMLElement,
+  element: HTMLElement | null,
   attributes: string[],
-  svgUid?: string
+  cssRules?: CSSRules
 ): Record<string, any> {
   if (!element) {
     return {};
   }
 
   let value,
-    parentAttributes = {},
+    parentAttributes: Record<string, string> = {},
     fontSize,
-    parentFontSize;
+    parentFontSize = DEFAULT_SVG_FONT_SIZE;
 
-  if (typeof svgUid === 'undefined') {
-    svgUid = element.getAttribute('svgUid');
-  }
   // if there's a parent container (`g` or `a` or `symbol` node), parse its attributes recursively upwards
   if (
     element.parentNode &&
     svgValidParentsRegEx.test(element.parentNode.nodeName)
   ) {
-    parentAttributes = parseAttributes(element.parentNode, attributes, svgUid);
+    parentAttributes = parseAttributes(
+      element.parentElement,
+      attributes,
+      cssRules
+    );
+    if (parentAttributes.fontSize) {
+      fontSize = parentFontSize = parseUnit(parentAttributes.fontSize);
+    }
   }
 
-  let ownAttributes = attributes.reduce(function (memo, attr) {
-    value = element.getAttribute(attr);
-    if (value) {
-      // eslint-disable-line
-      memo[attr] = value;
-    }
-    return memo;
-  }, {});
-  // add values parsed from style, which take precedence over attributes
-  // (see: http://www.w3.org/TR/SVG/styling.html#UsingPresentationAttributes)
-  const cssAttrs = Object.assign(
-    getGlobalStylesForElement(element, svgUid),
-    parseStyleAttribute(element)
-  );
-  ownAttributes = Object.assign(ownAttributes, cssAttrs);
-  if (cssAttrs[cPath]) {
-    element.setAttribute(cPath, cssAttrs[cPath]);
+  const ownAttributes: Record<string, string> = {
+    ...attributes.reduce<Record<string, string>>((memo, attr) => {
+      value = element.getAttribute(attr);
+      if (value) {
+        memo[attr] = value;
+      }
+      return memo;
+    }, {}),
+    // add values parsed from style, which take precedence over attributes
+    // (see: http://www.w3.org/TR/SVG/styling.html#UsingPresentationAttributes)
+    ...getGlobalStylesForElement(element, cssRules),
+    ...parseStyleAttribute(element),
+  };
+
+  if (ownAttributes[cPath]) {
+    element.setAttribute(cPath, ownAttributes[cPath]);
   }
-  fontSize = parentFontSize =
-    parentAttributes.fontSize || DEFAULT_SVG_FONT_SIZE;
   if (ownAttributes[fSize]) {
     // looks like the minimum should be 9px when dealing with ems. this is what looks like in browsers.
-    ownAttributes[fSize] = fontSize = parseUnit(
-      ownAttributes[fSize],
-      parentFontSize
-    );
+    fontSize = parseUnit(ownAttributes[fSize], parentFontSize);
+    ownAttributes[fSize] = `${fontSize}`;
   }
 
-  const normalizedStyle = {};
+  const normalizedStyle: Record<string, string> = {};
   for (const attr in ownAttributes) {
     const normalizedAttr = normalizeAttr(attr);
     const normalizedValue = normalizeValue(