search

jacobmischka / gatsby-plugin-react-svg

Adds svg-react-loader to gatsby webpack config
https://www.npmjs.com/package/gatsby-plugin-react-svg
MIT License
70 stars 21 forks source link

Include and exclude are not working correctly in all cases #17

Open niksajanjic opened 5 years ago

niksajanjic commented 5 years ago

I see that whatever gets passed as include or exclude to svg-react-loader will be added to url-loader too, just other way around, include --> exclude, exclude --> include.

That is not correct in all cases. For instance, all my images are held in the same directory. If I put that directory as include, it will be excluded for url-loader. That can be fixed by passing regex and checking for .svg extension. But, that is pointless, because svg-react-loader has test option for that extension, and passing regex with the same extension would be redundant, but it is important for url-loader to work. This gets even worse if I need to exclude node_modules (for any reason) because they will be included for url-loader.

I would like to be able to override those options. For instance, I would like to be able to pass urlLoaderInclude and urlLoaderExclude options and override default behavior as it is now.

jacobmischka commented 5 years ago

Reasonable request. Though for your particular use case, if you don't include either include or exclude then svg-react-loader will just do all svgs and url-loader will do all other images. Is that not what you want anyway?

niksajanjic commented 5 years ago

Yeah, sure, that is how it would work. In my case I'm using regex as a solution, just to be sure it will be applied only to images folder (I'm being extra safe, I have one more folder of images for marketing site).

I just reported this thinking forward. In some specific cases, it could be good to have control over what gets included/excluded for url-loader too. Even now we could just update webpack config after this plugin runs in our gatsby-node, it just seems easier to allow it with this lib, that's all.

jacobmischka commented 5 years ago

Agreed, I just wanted to point out that option in the meantime until this gets implemented. I'll try to take a look soon!