danielstgt / laravel-mix-svg-vue

A Laravel Mix extension to inline SVG files with Vue.js and automatically optimize them with SVGO
MIT License
38 stars 9 forks source link

Prevent from stripping viewBox #9

Closed fr0tt closed 3 years ago

fr0tt commented 3 years ago

First of all thank you for providing this package !

However lately I find myself experiencing an issue with it.

The first one was when I tried to use npm update && npm run prod causing an Segmentation fault error. So far I have found two ways to resolve this:

a) Downgrade or never update npm. Using npm v6 works just fine. b) Update laravel-mix-svg-vue (from 0.2.7) to v0.3.4

Obvious the latter makes more sense but it causes a new problem, at least for me. Everything works just fine, but all SVGs are stripped from their viewBox, which breaks my UI.

Changing options seems not work either. No matter if simply use

mix.svgVue()

or

mix.svgVue({
    svgPath: 'resources/svg',
    extract: false,
    svgoSettings: [
        { removeTitle: true },
        { removeViewBox: false },
        { removeDimensions: true }
    ]
})

I'm using: npm 7.20.3 laravel-mix 6.0.18 or 6.0.27 vue 2.6.12 node: 15.14.0

danielstgt commented 3 years ago

Hey 👋🏼

I tried to replicate your issue with the specified versions (only npm had a minor different version with 7.20.3). The SVG used for this test was this one:

<svg xmlns="http://www.w3.org/2000/svg" class="h-6 w-6" fill="none" viewBox="0 0 24 24" stroke="currentColor">
    <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z" />
</svg>

Source: heroicons

The desired output had the viewBox attribute included.

Could you share a SVG where you're experiencing the missing viewBox attribute? And it would help if you could show me the generated output of that SVG, maybe the library isn't even picking up that Vue component.

fr0tt commented 3 years ago

Thank you for your response.

Well that's strange. I tried to make sure that this is not only a problem of my own.

I use different SVGs, among others Material Icons. One example would be:

<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
    <path fill="none" d="M0 0h24v24H0V0z"/>
    <path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"/>
</svg>

results in:

<svg xmlns="http://www.w3.org/2000/svg" class="editor-icon">
    <path fill="none" d="M0 0h24v24H0V0z"></path>
    <path d="M9.4 16.6 4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0 4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"></path>
</svg>

If this doesn't help you to reproduce the issue, maybe try to use my code directly:

danielstgt commented 3 years ago

Oh, I messed up with the options for the webpack svgo-loader (actually SVGO itself) when this loader was introduced 🤦🏼‍♂️.

The reason I didn't encounter the removed viewBox attribute issue was because of the missing width and height values in the SVG, which get recognized from SVGO.

However, I fixed the options problem in a new release: https://github.com/danielstgt/laravel-mix-svg-vue/releases/tag/v0.3.5

Upgrading to the newest version should fix the missing viewBox attribute. If you still find any problems, just reopen this issue or let me know.

Thank you for reporting this!

fr0tt commented 3 years ago

Thank you for fixing this so quickly, glad I was of help ! (I suspected something like that after reading Issue#8 but wasn't sure because nobody else was reporting a similar issue.)

While this fixes the example I posted before, it didn't work everywhere. If you download a fresh Material Icon from Google - in this case I used the same to make it more obvious - the SVG is actually a little bit different and the viewBox is still getting removed with v0.3.5.

The new version is:

<svg xmlns="http://www.w3.org/2000/svg" height="24px" viewBox="0 0 24 24" width="24px" fill="#000000">
    <path d="M0 0h24v24H0V0z" fill="none"/>
    <path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"/>
</svg>

which results again in:

<svg xmlns="http://www.w3.org/2000/svg" class="editor-icon">
    <path d="M0 0h24v24H0V0z" fill="none"></path>
    <path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"></path>
</svg>

(I can't reopen an issue if I'm not a contributor of you repo.)

I also noticed another difference with the new svgo-loader but I'm not sure if that's a feature or a bug, so I didn't open another issue :D This:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24">
    <path fill="none" d="M0 0h24v24H0z"/>
    <path d="M12 14v8H4a8 8 0 0 1 8-8zm0-1c-3.315 0-6-2.685-6-6s2.685-6 6-6 6 2.685 6 6-2.685 6-6 6zm2.595 5.812a3.51 3.51 0 0 1 0-1.623l-.992-.573 1-1.732.992.573A3.496 3.496 0 0 1 17 14.645V13.5h2v1.145c.532.158 1.012.44 1.405.812l.992-.573 1 1.732-.992.573a3.51 3.51 0 0 1 0 1.622l.992.573-1 1.732-.992-.573a3.496 3.496 0 0 1-1.405.812V22.5h-2v-1.145a3.496 3.496 0 0 1-1.405-.812l-.992.573-1-1.732.992-.572zM18 17a1 1 0 1 0 0 2 1 1 0 0 0 0-2z" fill="#000"/>
</svg>

results in:

<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
  <path fill="none" d="M0 0h24v24H0z"></path>
  <path d="M12 14v8H4a8 8 0 0 1 8-8zm0-1c-3.315 0-6-2.685-6-6s2.685-6 6-6 6 2.685 6 6-2.685 6-6 6zm2.595 5.812a3.51 3.51 0 0 1 0-1.623l-.992-.573 1-1.732.992.573A3.496 3.496 0 0 1 17 14.645V13.5h2v1.145c.532.158 1.012.44 1.405.812l.992-.573 1 1.732-.992.573a3.51 3.51 0 0 1 0 1.622l.992.573-1 1.732-.992-.573a3.496 3.496 0 0 1-1.405.812V22.5h-2v-1.145a3.496 3.496 0 0 1-1.405-.812l-.992.573-1-1.732.992-.572zM18 17a1 1 0 1 0 0 2 1 1 0 0 0 0-2z" fill="#000"></path>
</svg>

before svgo-loader (e.g. v0.2.7) it resulted in: (the difference is removing the second fill fill="#000")

<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
    <path fill="none" d="M0 0h24v24H0z"></path>
    <path d="M12 14v8H4a8 8 0 018-8zm0-1c-3.315 0-6-2.685-6-6s2.685-6 6-6 6 2.685 6 6-2.685 6-6 6zm2.595 5.812a3.51 3.51 0 010-1.623l-.992-.573 1-1.732.992.573A3.496 3.496 0 0117 14.645V13.5h2v1.145c.532.158 1.012.44 1.405.812l.992-.573 1 1.732-.992.573a3.51 3.51 0 010 1.622l.992.573-1 1.732-.992-.573a3.496 3.496 0 01-1.405.812V22.5h-2v-1.145a3.496 3.496 0 01-1.405-.812l-.992.573-1-1.732.992-.572zM18 17a1 1 0 100 2 1 1 0 000-2z"></path>
</svg>
danielstgt commented 3 years ago

I ran the following steps with the provided SVG icon:

  1. git clone https://github.com/fr0tt/benotes
  2. composer install (php 7.3)
  3. npm install laravel-mix-svg-vue@^0.3
  4. Create a new SVG: resources/svg/material/testing.svg (see below for exact content)
  5. Use the new SVG in Login.vue with <svg-vue class="w-16 block m-auto" icon="material.testing"/>
  6. npm run prod

The content of the testing.svg was copied from your last reply:

<svg xmlns="http://www.w3.org/2000/svg" height="24px" viewBox="0 0 24 24" width="24px" fill="#000000">
    <path d="M0 0h24v24H0V0z" fill="none"/>
    <path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"/>
</svg>

And the generated output was:

<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" class="w-16 block m-auto">
    <path d="M0 0h24v24H0V0z" fill="none"></path>
    <path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"></path>
</svg>

The used SVGO options:

.svgVue({
    svgoSettings: [
        { removeTitle: false },
        { removeViewBox: false },
        { removeDimensions: true },
    ]
})

So that seems to work so far... could you verify that these steps really remove the viewBox attribute? Just for sanity you could try rm -rf node_modules and a reinstall so we can rule out something crazy happening there.

Regarding the fill attribute: There is indeed something wrong, but I found out thats actually an issue with SVGO itself, see https://github.com/svg/svgo/issues/1478. I've subscribed to that issue and will react if this gets fixed there. In the meantime I'll try to find a workaround and setup some automated tests...

fr0tt commented 3 years ago

Using rm -rf node_modules didn't resolve my issue.

Maybe I am going to be wrong twice in a row but bear with me.

If you use on my repo npm install laravel-mix-svg-vue it should install v0.2.7 not v0.3.5 (because my master version still works correctly using the older version of your package). That's way I wrote use npm install laravel-mix-svg-vue@^0.3 to upgrade to the new version v0.3.5 in order to experience any issues.

Regarding svgo#1478: Isn't that an issue that svgo strips a fill it shouldn't ? Because my example shows a fill that is not getting removed but it should, just like in older versions (and my css relies on this clean stripped version in order to change the color with css). (My theory might be that your previous implementation stripped all fill attributes that were not fill="none", but that's just a guess.)

danielstgt commented 3 years ago

You are right, the install command was actually a typo in my command, I used a local version.

I think now I've got it fixed, the absence of the default plugins passed to SVGO are causing the missing viewBox attribute. A new release has been tagged (v0.3.6) which uses the function to merge custom configs with the built-in plugins.

Could you verify that npm i laravel-mix-svg-vue@^0.3 now keeps the viewBox included?

About the fill attribute issue: I'm not sure what exactly causes or caused that, maybe some optimization that I'm not aware of right now. Does the new version with the default plugin list still remove necessary fill attributes?

fr0tt commented 3 years ago

Thank you ! Everything works just fine now (viewBox and fill alike).