gilbarbara / react-inlinesvg

An SVG loader component for ReactJS
https://codesandbox.io/s/github/gilbarbara/react-inlinesvg/tree/main/demo
MIT License
1.27k stars 100 forks source link

SVG content is reparsed even when it doesn't change #167

Closed leeoniya closed 3 years ago

leeoniya commented 3 years ago

hi @gilbarbara,

first, thanks for this library :)

I noticed several areas where performance can be improved and would like to see what you think.

  1. i'm trying to pre-cache svg contents by path so that 0 fetch requests happen on initial load for statically-included icons. i was able to do this by exporting the global cacheStore, and pre-populating it with 'loaded' status cache objects and svg contents. i was hoping you can add this export to react-inlinesvg, so i can do this without a fork. https://github.com/gilbarbara/react-inlinesvg/blob/eefce45bfdb3709ff0a036a859e75702b92021b7/src/index.tsx#L14
  2. i noticed that the svg sting is re-parsed into a DOM node, even when it does not change. is it possible to re-use the existing dom node? or alternatively, it would be cheaper to cache the parsed node in cacheStore, and then use cachedNode.clone(deep) instead of DOMParser(): https://developer.mozilla.org/en-US/docs/Web/API/Node/cloneNode
  3. some flow and regular expressions can be optimized, several examples here: https://github.com/gilbarbara/react-inlinesvg/blob/eefce45bfdb3709ff0a036a859e75702b92021b7/src/index.tsx#L235-L247

would you consider adding these or would you accept a PR?

thanks! leon

gilbarbara commented 3 years ago

Hello @leeoniya

  1. I'll export the cacheStore in 2.3.0
  2. Did you have problems with this or it's just an optimization urge? Because the DOMParse is quite an inexpensive operation at this level. I did think about this when I was implementing it and decided against caching as it would require a lot of code and I was already over my budget.
  3. Based on perf tests I've found indexOf is way faster than the rest.

And the difference between startsWith and regex is insignificant.

E.g.: https://www.measurethat.net/Benchmarks/Show/4797/1/js-regex-vs-startswith-vs-indexof#latest_results_block

leeoniya commented 3 years ago

thank you :pray: