-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(): Remove storage + cssRules, gradientDefs, clipPathDefs globals #9030
Conversation
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks bad but i really just removed the var rules
and chained 3 array methods of which we didn't need the return value
} | ||
|
||
const normalizedStyle = {}; | ||
const normalizedStyle: Record<string, string> = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i m not sure what happened here, i will rely on tests.
Build Stats
|
No tests failing, super shady. |
i m sure it require more tests |
Motivation
I wanted to remove the 3 globals cited in the title.
Description
gradientDefs, clipPathDefs and Cssrules were global because with the old code we couldn't await anything.
We assigned every SVG parsing an id ( svguid ), and then we would keep a registry called storage, where we had 3 properties:
clippaths, gradients, and css rules.
In each of those 3 properties we had an object of which a property assigned to svguid would contain the data for our parsin sessions.
Now this data is local to the parser class.
The parser injects the css rules in the fromElement function that then pushes it in the parseAttributes function.
ClipPaths and gradientDefs are consume in the parser so they didn't need much work.
The old code was just bad, was one of the first things i did for fabricJS in 2014 and i just extended existing things without really diving deep in how/why.
Changes
fromElement has now 3 arguments.
Gist
In Action