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

Repeated values in class names #1133

Closed
ilariaventurini opened this issue Jul 24, 2019 · 11 comments
Closed

Repeated values in class names #1133

ilariaventurini opened this issue Jul 24, 2019 · 11 comments

Comments

@ilariaventurini
Copy link

ilariaventurini commented Jul 24, 2019

Use --multipass option and {"prefixIds":true} creates css class names with repeated values.
For example run svgo hello3.svg -o - --pretty --config='{"plugins":[{"prefixIds":true}]}' --multipass using the svg file hello3.svg:

<svg xmlns="http://www.w3.org/2000/svg" id="Livello_1" data-name="Livello 1" viewBox="0 0 300 170">
  <style>
    .cls-1{fill:#d9e7f2;}.cls-2{fill:#1191ae;}
  </style>
  <rect width="265.1" height="1" x="32.9" y="117.95" class="cls-1"/>
  <rect width="265.1" height="1" x="32.9" y="90.08" class="cls-1"/>
  <rect width="265.1" height="1" x="32.9" y="61.68" class="cls-1"/>
  <rect width="265.1" height="1" x="32.9" y="33.8" class="cls-1"/>
  <rect width="266.03" height="1" x="32.4" y="5.4" class="cls-1"/>
  <rect width="15.12" height="27.88" x="54.66" y="118.5" class="cls-2"/>
  <rect width="15.12" height="39.97" x="89.49" y="106.4" class="cls-2"/>
  <rect width="15.12" height="80.47" x="124.33" y="65.91" class="cls-2"/>
  <rect width="15.12" height="98.35" x="159.16" y="48.02" class="cls-2"/>
  <rect width="15.12" height="83.1" x="193.99" y="63.28" class="cls-2"/>
  <rect width="15.12" height="108.87" x="228.83" y="37.51" class="cls-2"/>
  <rect width="15.12" height="30.5" x="263.66" y="115.87" class="cls-2"/>
  <rect width="266.03" height="1" x="32.4" y="145.83" class="cls-1"/>
</svg>

Creates this svg:

<svg xmlns="http://www.w3.org/2000/svg" id="hello3_svg__hello3_svg__hello3_svg__Livello_1" data-name="Livello 1" viewBox="0 0 300 170">
    <style>
        .hello3_svg__hello3_svg__hello3_svg__cls-1{fill:#d9e7f2}
    </style>
    <path class="hello3_svg__hello3_svg__hello3_svg__cls-1" d="M32.9 117.95H298v1H32.9zm0-27.87H298v1H32.9zm0-28.4H298v1H32.9zm0-27.88H298v1H32.9zm-.5-28.4h266.03v1H32.4z"/>
    <path d="M54.66 118.5h15.12v27.88H54.66zm34.83-12.1h15.12v39.97H89.49zm34.84-40.49h15.12v80.47h-15.12zm34.83-17.89h15.12v98.35h-15.12zm34.83 15.26h15.12v83.1h-15.12zm34.84-25.77h15.12v108.87h-15.12zm34.83 78.36h15.12v30.5h-15.12z" fill="#1191ae"/>
    <path class="hello3_svg__hello3_svg__hello3_svg__cls-1" d="M32.4 145.83h266.03v1H32.4z"/>
</svg>

As you can see, class names contains the name file duplicated different times (hello3_svg__hello3_svg__hello3_svg__cls-1).

This PR should solve the problem.

@kylemh
Copy link

kylemh commented Mar 5, 2020

#1227

Copied this PR, but removed the extra changes

TrySound added a commit that referenced this issue Feb 18, 2021
Ref
  #1330
  #1148
  #1133
  #1227
Took tests from #1177
TrySound added a commit that referenced this issue Feb 18, 2021
@TrySound
Copy link
Member

Fixed in 2.0.1

@kylemh
Copy link

kylemh commented Feb 21, 2021

@TrySound was excited to come back to the fold, but I just tried using [email protected] and the following config:

{
  "plugins": [
    {
      "addAttributesToSVGElement": {
        "attributes": [{ "aria-hidden": "true" }]
      }
    },
    {
      "prefixIds": {
        "delim": "_"
      }
    },
    {
      "cleanupIDs": { "minify": true }
    },
    { "cleanupListOfValues": true },
    { "convertColors": true },
    { "convertStyleToAttrs": true },
    { "convertTransform": true },
    { "mergePaths": true },
    { "minifyStyles": true },
    { "moveElemesAttrsToGroup": true },
    {
      "removeAttrs": true,
      "params": { "attrs": ["fill-rule", "fillRule"] }
    },
    { "removeComments": true },
    { "removeDesc": true, "params": { "removeAny": true } },
    { "removeDimensions": true },
    { "removeDoctype": true },
    { "removeEditorsNSData": true },
    { "removeEmptyAttrs": true },
    { "removeEmptyContainers": true },
    { "removeEmptyText": true },
    { "removeNonInheritableGroupAttrs": true },
    { "removeTitle": true },
    { "removeUnknownsAndDefaults": true },
    { "removeUnusedNS": true },
    { "removeUselessDefs": true },
    { "removeUselessStrokeAndFill": true },
    { "removeXMLProcInst": true },
    { "sortAttrs": true }
  ],
  "floatPrecision": 3,
  "multipass": true
}

still leads to a class being duplicated upon an SVG

For example:

Run yarn svgo --pretty --config=svgo.config.format.json **/GoogleDrive.svg

<svg xmlns="http://www.w3.org/2000/svg" aria-hidden="true" viewBox="0 0 32 32">
    <path fill="currentColor" d="M8 26.573l4-6.667h16l-4 6.667H8z" class="GoogleDrive_svg_path1"/>
    <path fill="currentColor" d="M20 19.906h8L20 6.573h-8l8 13.333z" class="GoogleDrive_svg_path2"/>
    <path fill="currentColor" d="M4 19.906l4 6.667 8-13.333-4-6.667-8 13.333z" class="GoogleDrive_svg_path3"/>
</svg>

yields

<path fill="currentColor" d="M8 26.573l4-6.667h16l-4 6.667H8z" class="GoogleDrive_svg_GoogleDrive_svg_GoogleDrive_svg_path1"/>
    <path fill="currentColor" d="M20 19.906h8L20 6.573h-8l8 13.333z" class="GoogleDrive_svg_GoogleDrive_svg_GoogleDrive_svg_path2"/>
    <path fill="currentColor" d="M4 19.906l4 6.667 8-13.333-4-6.667-8 13.333z" class="GoogleDrive_svg_GoogleDrive_svg_GoogleDrive_svg_path3"/>

@TrySound
Copy link
Member

Hi! Configuration is changed in v2. See https://github.com/svg/svgo/releases/tag/v2.0.0

const { extendDefaultPlugins } = require('svgo');
module.exports = {
  "plugins": extendDefaultPlugins([
    {
      name: "addAttributesToSVGElement",
      params: {
        "attributes": [{ "aria-hidden": "true" }]
      }
    },
    {
      name: "prefixIds",
      params: {
        "delim": "_"
      }
    },
    {
      name: "cleanupIDs", params: { "minify": true }
    },
    "cleanupListOfValues",
    "convertColors",
    "convertStyleToAttrs",
    "convertTransform",
    "mergePaths",
    "minifyStyles",
    "moveElemesAttrsToGroup",
    {
      name: "removeAttrs",
      params: { attrs: ["fill-rule", "fillRule"] }
    },
    "removeComments",
    { name: "removeDesc", params: { removeAny: true } },
    "removeDimensions",
    "removeDoctype",
    "removeEditorsNSData",
    "removeEmptyAttrs",
    "removeEmptyContainers",
    "removeEmptyText",
    "removeNonInheritableGroupAttrs",
    "removeTitle",
    "removeUnknownsAndDefaults",
    "removeUnusedNS",
    "removeUselessDefs",
    "removeUselessStrokeAndFill",
    "removeXMLProcInst",
    "sortAttrs",
  ]),
  "floatPrecision": 3,
  "multipass": true
}

@kylemh
Copy link

kylemh commented Feb 21, 2021

Great catch! Adjusted accordingly, but ran into #1358

@kylemh
Copy link

kylemh commented Feb 21, 2021

Circling back... I forced the version of SVGO to v2 via resolutions key in package.json to confirm SVGO CLI behavior.

Unfortunately, I'm still seeing classnames continuing to be edited on each SVGO CLI command.

Proof there's only SVGOv2:

Screen Shot 2021-02-21 at 12 00 11 PM

Beginning:

Screen Shot 2021-02-21 at 12 00 22 PM

After 1st yarn svgo --pretty --config=svgo.config.format.js ./svgs/GoogleDrive.svg

Screen Shot 2021-02-21 at 12 00 33 PM

After 2nd yarn svgo --pretty --config=svgo.config.format.js ./svgs/GoogleDrive.svg

Screen Shot 2021-02-21 at 12 00 38 PM

@TrySound
Copy link
Member

TrySound commented Feb 21, 2021

I see. Well, this is expected behaviour. If you want to have better optimized svg use --multipass flag. And in any case run svgo only once per svg.

@kylemh
Copy link

kylemh commented Feb 21, 2021

Is there anything against merging a PR like #1135 which prevents prepending class name if it's already added?

@TrySound
Copy link
Member

It may be unreliable in case if prefix matches actual id.

@kylemh
Copy link

kylemh commented Feb 21, 2021

What about checking for that situation?

I'm surprised classes get prefixed when prefixIds I think would only be for the value of the id attribute?

Sorry if my persistence is frustrating... I'm more curious than anything. I use classnames to bypass path merging on a per-file basis and - sometimes - I run SVGO against all my files. It seems like the classname attribute is the only one that returns differently on each run even if they've already been optimized.

It seems my only recourse for now is to never run SVGO against all files.

@TrySound
Copy link
Member

I agree naming is not very reliable. Though there is an option to disable or id or classnames prefixing

    delim: '__',
    prefixIds: true,
    prefixClassNames: true,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants