search

jhamlet / svg-react-loader

Webpack SVG to React Component Loader
MIT License
559 stars 82 forks source link

Add support for a custom SVG root element #44

Closed chrisronline closed 7 years ago

chrisronline commented 7 years ago

This PR adds the option to configure an optional root element(s) to replace whichever root element exists in the svg file.

This is helpful if the source svg files aren't standardized or contain unnecessary pieces of data (like if they were exported from Sketch). Specifying a root (like path) allows you to consume only the necessary parts of the svg file and wrap them in a standard <svg> template.

I also added an optimization around fetching the template file - it's constantly refetched from disk for each svg so I added a variable cache to improve that performance.

I added tests for the root changes too.

chrisronline commented 7 years ago

@jhamlet

jhamlet commented 7 years ago

@chrisronline

Thanks for wanting to contribute to svg-react-loader

If I read it correctly, the root change you want to add is to take one of the nested elements in the SVG and lift that up to be the root, correct?

However...

I've tried to follow the webpack philosophy with svg-react-loader. It does one thing, and only one thing. That way configuration and setup are fairly easy, and straight forward.

It sounds like your wanting to "lift" an element up to be root is the realm of another loader. svg-lift-loader perhaps?

In addition, I've stopped supporting svg-react-loader@<=0.4.0 -- version 0.4.0 is in beta right now, just waiting for me to finish my test suite.

What is nice, with the changes to how svg-react-loader@next parses SVGs and then generates output, you can actually use svg-react-loader@next to accomplish what you want to do. Take a look at the new Object Tree API and the Filters API for inspiration.

Again thanks for wanting to contribute. I believe there is merit in what you want to accomplish, I just don't think it belongs in svg-react-loader, but I don't see any problem with you using the capabilities of the next version of it to accomplish your goals. I'd actually be interested in a loader like that.

Thanks,

;-j