Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@parcel/transformer-svgo removes viewBox attribute #4314

Closed
surma opened this issue Mar 11, 2020 · 10 comments
Closed

@parcel/transformer-svgo removes viewBox attribute #4314

surma opened this issue Mar 11, 2020 · 10 comments
Labels
✖️ Non-Parcel bug Bugs related to dependencies or plugins 🔌 Plugins

Comments

@surma
Copy link

surma commented Mar 11, 2020

🐛 bug report

@parcel/transformer-svgo removes viewBox attribute. This will drastically affect the visual of the resulting SVG.

🎛 Configuration (.babelrc, package.json, cli command)

Please see repro repo

🤔 Expected Behavior

The SVG should be minified and still look the same than the original.

😯 Current Behavior

The viewBox attribute is removed, creating a different visual from the original.

💁 Possible Solution

🔦 Context

Normal web development.

💻 Code Sample

Please see repro repo

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-alpha.3.2
Node v12.8.0
npm/Yarn 6.12.1
Operating System MacOS 10.15.3 (19D76)
@mischnic
Copy link
Member

mischnic commented Mar 11, 2020

It seems to do this by default (tested with https://jakearchibald.github.io/svgomg/),
you can add a .svgorc with

{ "plugins": [{ "removeViewBox": false }] }

The removeViewBox description says "remove viewBox attribute when possible", so it seems to be a bit too aggressive.

(And pasting the two files into the issue would be enough as well, you don't have to create a repo everytime if you want 😄 )

@surma
Copy link
Author

surma commented Mar 11, 2020

Interesting. I could have sworn I tested with svgomg. @jakearchibald what do you think? Should removing viewBox be the default?

(And pasting the two files into the issue would be enough as well, you don't have to create a repo everytime if you want 😄 )

Believe it or not, this is currently easier for me :D

@mischnic
Copy link
Member

That plugin should only run when width and height are set via an attribute and the viewbox has the form 0 0 ${widthAttribute} ${heightAttribute}: https://github.com/svg/svgo/blob/07ca9764f71fb946adc23f4ea9f19070d335305d/plugins/removeViewBox.js#L31-L32
https://github.com/svg/svgo/blob/07ca9764f71fb946adc23f4ea9f19070d335305d/plugins/removeViewBox.js#L40-L41

Not sure why the plugin is actually removing viewBox in this case.

@mischnic mischnic added ✖️ Non-Parcel bug Bugs related to dependencies or plugins and removed 🐛 Bug labels Mar 11, 2020
@surma
Copy link
Author

surma commented Mar 30, 2020

Well, this is embarrassing: the removeViewBox plugin does the correct thing in SVGO, in that it only removes the viewBox attribute when it’s safe to do so.

The reason why my repro was misbehaving is because it says viewPort instead of viewBox 🤦‍♂ So all is well, no configuration needed.

@surma surma closed this as completed Mar 30, 2020
@mischnic
Copy link
Member

😄
I didn't see that either...

@micrology
Copy link

I'm having this issue too, and the width and height are not the same as the width and height in the viewport. The original inline svg is:

<svg
									xmlns="http://www.w3.org/2000/svg"
									width="24"
									height="24"
									fill="currentColor"
									class="bi bi-plus-lg"
									viewBox="0 0 16 16"
								>
									<path
										d="M8 0a1 1 0 0 1 1 1v6h6a1 1 0 1 1 0 2H9v6a1 1 0 1 1-2 0V9H1a1 1 0 0 1 0-2h6V1a1 1 0 0 1 1-1z"
									/>
								</svg>

The result from parcel 2.0.0-nightly.679 using parcel build is:

<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="currentColor" class="bi bi-plus-lg"><path d="M8 0a1 1 0 011 1v6h6a1 1 0 110 2H9v6a1 1 0 11-2 0V9H1a1 1 0 010-2h6V1a1 1 0 011-1z"></path></svg>

Note the removal of the viewBox, despite the width and height (24) not being the same as the viewBox width and height (16).

@micrology
Copy link

The suggestion of adding a .svgorc with { "plugins": [{ "removeViewBox": false }] } doesn't solve this. Can I disable parcel's use of SVGO? If so, how would I do this?

@mischnic
Copy link
Member

I assume that you aren't using SVGO yourself then (you have to specify a parcelrc config for that at the moment), but this happens via htmlnano (and your SVG is inside an HTML file).

So adding a .htmlnanorc with

{ "minifySvg": { "plugins": [{ "removeViewBox": false }] } }

or outright

{ "minifySvg": false }

should work

@micrology
Copy link

micrology commented May 26, 2021 via email

@charIeszhao
Copy link

charIeszhao commented Jun 22, 2022

I came across with this today, and none of these solutions mentioned above worked.
For those who have the same issue, try this:

Create a svgo.config.js file in the project root.

module.exports = {
  plugins: [
    {
      name: 'preset-default',
      params: {
        overrides: {
          cleanupIDs: false, // Disable the ID minification, for some reasons the minified ID causes problems in my project
          removeViewBox: false, // Keep viewbox attribute
        },
      },
    },
  ],
};

Re-build the project and it should work now

Update (Nov 24, 2022):
For those who switched to ESM in their projects and found none of the svgo.config.cjs or mjs working, try creating a .svgorc.json file instead... Like this.
We came across this issue again after switching to ESM and this was the only way that worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✖️ Non-Parcel bug Bugs related to dependencies or plugins 🔌 Plugins
Projects
None yet
Development

No branches or pull requests

5 participants