-
Notifications
You must be signed in to change notification settings - Fork 214
svg-url-loader
works incorrectly when load images not from CSS files.
#272
Comments
Note: webpack-chain supports issuer, it just doesn't have a shorthand method for it: neutrino.config.rule(name).set('issuer', ...) Also, feel free to PR webpack-chain and add issuer to Rule extensions. |
Ok, I will investigate it. |
This is affecting us badly. @constgen did you get this to work with Does it make sense to revert to |
@timkelty you did the original PR for switching to svg-url-loader. What is your opinion here? |
Hmm - yeah I was under the impression Sounds like maybe it is a good idea to revert back to |
affects: neutrino-preset-tux See upstream issue: neutrinojs/neutrino#272 ISSUES CLOSED: #144
Started working on it. Currently have an issue with separating config.module
.rule('svg')
.set('issuer', /\.(js|jsx|ts|tsx|html|vue)$/)
.test(/\.svg(\?v=\d+\.\d+\.\d+)?$/)
.use('url')
.loader(svgUrlLoader)
.options(merge({ limit }, options.svg || {}))
.tap(opts => merge(opts, { noquotes: true }));
config.module
.rule('style-svg')
.set('issuer', /\.(css|less|sass|scss)$/)
.test(/\.svg(\?v=\d+\.\d+\.\d+)?$/)
.use('url')
.loader(svgUrlLoader)
.options(merge({ limit }, options.svg || {}))
.tap(opts => merge(opts, { noquotes: false })); Only the latest rule is applied correctly. Changing order of rules affects the result. This is not expected to be. It may take several days to fix. Have one more idea with |
In addition to the use-case of importing SVGs directly in a project's JS, this issue also affects HTML assets loaded by It seems really broken for consumers of Given that |
The new test suite is available on the same link https://mega.nz/#!KZFlCD6B!ziIF6WMlEArcD8RPJChIsteJLIQfDgzKmlTxXyUlSxI . You need to link the Neutrino locally and switch to There are bad and good news.
So in the test suite you can see that I used the copy of the image in the CSS to avoid this problem: |
@constgen would this problem go away if we used file/url-loader instead? |
Yes, it would. But we need to provide an alternative for those who uses SVG markup for inlined isons. |
Not sure what you mean, can you elaborate? |
So I've spent some more time digging into this... I've come to the conclusion that I've created a reduced STR at webpack-contrib/css-loader/issues/615 and once/if that's fixed, the whole quoting and Given this, I don't think we should add any workarounds in Neutrino for now, and should instead switch back to Also something I forgot to mention in my last comment -- even when using |
@edmorley wow, great analysis, thank you so much! Luckily, I decided I revert back to using url-loader for SVGs right before I released Neutrino v7, so we should be safe there right now. Closing, and we can add the svg-url-loader back when other packages get updated. |
Thanks to all, and sorry for causing this headache initially! |
@timkelty no worries at all, it's a great learning experience for all of us! |
Can we merge my changes in v6? At least they will resolve some problems. |
The issue:
https://mega.nz/#!KZFlCD6B!ziIF6WMlEArcD8RPJChIsteJLIQfDgzKmlTxXyUlSxI
This is a simple project that demonstrates the problem with
svg-url-loader
.It works as expected when SVG images are referenced from CSS code. But when load image with
import
and inject it in HTML with interpolation or by reference in JSX it works incorrectly. It is caused by wrapping quotes around resulting string"..."
. Need to strip this quotes every time when you load SVG in JSIt is expected that
svg-url-loader
will work the same way asimage-loader
The solution:
Webpack loader config has an
"issuer"
property. There is a good example of usage in the Less loader https://webpack.js.org/loaders/less-loader/#non-less-imports . We can pass different config tosvg-url-loader
dependeing on a dependency parent.svg-url-loader
supports"noquotes"
option that disables that quotes.But
webpack-chain
doesn't support"issuer"
property, that's why I propose to add it. @eliperelmanAlso there is a chance that we will have to contribute to
svg-url-loader
and add an additional option.The text was updated successfully, but these errors were encountered: