svg / svgo

⚙️ Node.js tool for optimizing SVG files
https://svgo.dev/
MIT License
20.92k stars 1.38k forks source link

Option to generate unique IDs? #674

Closed pluma closed 7 years ago

pluma commented 7 years ago

This is a feature request.

There's a plugin for minifying IDs but this is basically useless when inlining SVGs in HTML because it is guaranteed to cause problems (i.e. when inlining at least two SVGs that contain IDs you're guaranteed to have a conflict on ID "a").

It would be very useful if SVGO allowed prefixing IDs with a configurable key. This would allow tooling like react-svg-loader to generate a unique prefix and pass it to SVGO to guarantee the IDs don't break when the SVGs are inlined.

A problem is that the IDs can be referenced with xlink:href attributes and url() values but this is already addressed by the cleanupIDs plugin.

GreLI commented 7 years ago

Actually, cleanupIDs has prefix option (works only with minifying).

Kraxxis commented 7 years ago

I, too, need this feature.

the cleanupIDs prefix option is not a solution to this problem. Consider two svg files both of which have a defined id "id1". When using the two separate svgs inline, these two ids will conflict, since they exist in the same document. Using the prefix option only moves this problem from having two conflicting "id1" ids to having two conflicting "prefix_id1" ids.

Emasoft commented 7 years ago

The prefix should be the name of the file. For example:

IconOfACar.svg -> iconofacar_id1 IconOfABeer.svg -> iconofabeer_id1

pluma commented 7 years ago

Being able to supply the prefix should suffice. Seems more versatile than having SVGO pick one automagically.

Emasoft commented 7 years ago

I don't think that is versatile for batch processing. I usually process thousands of svg files with a script at every site update, and I surely don't want to manually enter a prefix for each one.

strarsis commented 7 years ago

Similar issue?: https://github.com/svg/svgo/issues/673

OwenMelbz commented 7 years ago

Is there any movement on this?

As much as it sounds a dick move, I think you need to go with a general consensus here, this is an issue that many users are having when using inlined SVGs when passed through SVGO - for example we even have this issue using Sketch SVGO Compressor plugin, we end up having 10 SVGs with #a that causes masking issues so have to manually go through the file and change them, which is not feasible for a fully automated system.

As @Emasoft has suggested, I think the only sensible solution would be to slugify the filename and use that as the id prefix, turn this on as a default, which means every user that has an automated workflow or has tools that then use svgo will be unaffected, unless they pass in an extra option to disable it something like --disable-auto-id-prefix

@strarsis this does seem like similar issue yeah, I think the issue as a whole is just around "naming conflicts as the minification process has no concept of other files"

strarsis commented 7 years ago

PR for a plugin that does this - a bit tweaking is necessary though: https://github.com/svg/svgo/pull/700

GreLI commented 7 years ago

@OwenMelbz you can disable minifying IDs in cleanupIDs with the following config:

plugins:
  - cleanupIDs:
      minify: false

So your files will have unique IDs as long as they had it already.

Editing a bunch of files has nothing with optimization, so it lays out of SVGO's scope.

OwenMelbz commented 7 years ago

@GreLI You've completely missed the point - which clearly several other people have agreed there is an issue.

Many automated systems rely on svgo Many of these systems do not allow the user to modify settings.

e.g Sketch has a official plugin, SVGO Compressor, which uses svgo. The end user cannot modify the plugin or config. Thus your reply is void.

Its extremely narrow minded and selfish to disregard an issue simply because you don't want to address it, rather than helping the global community come up with a solution to a community acknowledged issue.

Maybe if we state the problem again you will understand.


SVGO when cleaning up IDs (a feature that is still desired) does not have a concept of "other svgs" - which means if you minified 100 svgs, they would all end up having the id of a which means they're next to useless to be embedded into webpages.

The solution to help developers/designers/content writers/PEOPLE is by using the privileged stance of svgos positioning in the development world to implement a feature that solves this issue for users who are using their system for some reason. And there have been plenty of suggestions on ways to solve this.

Remember when Facebook - a social media platform, decided to use its privileged position to help family members involved in world crises to mark theirselves as safe? Yes its not directly their problem to solve, but they could propose a solution because it was for the benefit of its users.

This is why people come to you guys seeking solutions, because you're in a position to help out. Similarly to the cheesy line from spider-man - with great power comes great responsibility.

Maybe thinking of it in a different non-dismissive Trump-like manor will urge you not to slap a "closed" on a real life issue.

GreLI commented 7 years ago

Many of these systems do not allow the user to modify settings.

Unfortunately, nobody here can't fix their obvious flaws. What if different users want totally opposite? Configuration is what makes SVGO flexible. Ask these systems authors to add an option to set a configuration. They get paid for this, unlike me or other OSS maintainers.

The “consensus“ here has been set just in your mind. No statistics or clear numbers has been provided to verify your claims. So please stop your nonsense word pouring. Nobody here has to do something for you. Don't make me block conversation.

OwenMelbz commented 7 years ago

if your gonna act like an arrogant cunt and not help the community out then block it. 👌🏼 Peace out dickhead

GreLI commented 7 years ago

As you wish.

walnutpedia commented 7 years ago

@pluma @Kraxxis @Emasoft @OwenMelbz I have encountered this problem recently. I use react-svg-loader to import svg files directly. the react-svg-loader use svgo to optimize svg files. the problem is Adobe Illustrator will export svg files with same ID, and no matter how SVGO minify the ID there will be collision. I searched for a solution and ended up with this issue.

I think @GreLI 's right about the usage and purpose of SVGO. But asking for a feature like this is reasonable. SVGO provides a fairly easy way to manipulate svg code. it just seems very natural to do this kind of stuff with SVGO.

I ended up with writing a node script to process all the svg files using svgo. the tricky part is, every time it process a file, it will use a new SVGO instance and use a config with unique prefix. something look like this:

  let filePath = path.join(folderPath, file)
  let prefix = generatePrefix(file)
  let svgo = new SVGO({
    plugins: [{cleanupIDs: {
      prefix: prefix
    }}]
  })

  svgo.optimize(fs.readFileSync(filePath, 'utf8'), svg => {
    result = svg.data
  })

the generatePrefix function will generate a string use file's name. after running this script every svg file would have unique IDs based on it's file name. it's not a perfect solution but for now it's the best way to do it. maybe later I'll write a webpack loader or send a PR to react-svg-loader. hope it helps.

jt3k commented 7 years ago

The gulp-svgmin have solution for thus problem. Solution right in in readme -> Per-file-options

10xjs commented 7 years ago

I've found a solution for anyone is using webpack and svgo-loader or react-svg-loader that will pass a unique deterministic ID prefix to svgo for each file. This should generate the same IDs across different build environments.

// webpack.config.js
const hash = require('string-hash');
const {relative} = require('path');

const context = __dirname;

module.exports = {
  module: {
    rules: [
      {
        test: /\.svg$/,
        use: ({resource}) => ({
          loader: 'svgo-loader',
          options: {
            plugins: [
              {cleanupIDs: {
                prefix: `svg${hash(relative(context, resource))}`,
              }},
            ],
          },
        }),
      },
    ],
  },
};
ming-codes commented 7 years ago

I found a workaround to this problem.

{
  cleanupIDs: {
    prefix: {
      toString() {
        this.counter = this.counter || 0;

        return `id-${this.counter++}`;
      }
    }
  }
}

JavaScript objects invokes toString when converting from object to string for concatenation. The function can be used to generate unique ids.

tristanisme commented 7 years ago

Hi @GreLI . It's sad to see the abuse you got on this issue 😞

Editing a bunch of files has nothing with optimization, so it lays out of SVGO's scope.

Wouldn't it be within SVGO's scope to not break things while optimising?

CH-RhyMoore commented 6 years ago

I agree, this shouldn't require workarounds from every consumer and client utilizing SVGO.

Out of the box, SVGO's CLI is able to optimize multiple files at once, but the files are effectively broken when they come out due to colliding ids.

If nothing else, it'd cut down on the duplicate issues about it. 😉

mattkime commented 6 years ago

I was curious if anyone can provide an example of an id conflict. I'm very interested in using SVGO but want to make sure that I'm not wading into problems.

o-alexandrov commented 6 years ago

If anyone happens to wonder how to fix broken IDs, go to the following comment: https://github.com/svg/svgo/issues/913#issuecomment-373416266

jt3k commented 6 years ago

2 @mattkime Problems with identical id arise when two inline svg-elements are used on one page. As a rule id are used to describe the gradients, so the same gradient is applied to two different svg-elements.

chrisfinch commented 6 years ago

Anyone who encounters this whilst using react-svg-loader can find my working fix here: https://github.com/boopathi/react-svg-loader/issues/218#issuecomment-402684870

SilverFox70 commented 6 years ago

If anyone is using svg-react-loader, I have created a fix here: https://github.com/SilverFox70/svg-react-loader and it is simple as adding a uniqueIdPrefix: true.

magicspon commented 6 years ago

if anyone is still looking for a solution... this works in gulp:

.pipe(
    svgmin(file => {
        const prefix = path.basename(file.relative, path.extname(file.relative))
        return {
            plugins: [
                {
                    prefixIds: {
                        prefix
                    }
                },
                {
                    cleanupIDs: {
                        prefix: `${prefix}-`,
                        minify: true
                    }
                }
            ]
        }
    })
)
elderapo commented 5 years ago

I wrote a small library replaces id and id references inside SVG elements and manages them.

import * as React from 'react'
import { SVGUniqueID } from 'react-svg-unique-id'

export const SVG1 = () => (
  <SVGUniqueID>
    <svg width="100%" height="100%" viewBox="0 0 60 64">
      <defs>
        <linearGradient id="prefix__bga" /> // prefix__bga => ___SVG_ID__0__0___
      </defs>
      <g fillRule="nonzero" fill="none">
        <use xlinkHref="#prefix__bga" /> // #prefix__bga => #___SVG_ID__0__0___
        <path fill="url(#prefix__bga)" /> // url(#prefix__bga) => "url(#___SVG_ID__0__0___)
      </g>
    </svg>
  </SVGUniqueID>
)

export const SVG2 = () => (
  <SVGUniqueID>
    <svg width="100%" height="100%" viewBox="0 0 60 64">
      <defs>
        <linearGradient id="prefix__bga" /> // prefix__bga => ___SVG_ID__1__0___
      </defs>
      <g fillRule="nonzero" fill="none">
        <use xlinkHref="#prefix__bga" /> // #prefix__bga => #___SVG_ID__1__0___
        <path fill="url(#prefix__bga)" /> // url(#prefix__bga) => url(#___SVG_ID__1__0___)
      </g>
    </svg>
  </SVGUniqueID>
)
joebanks10 commented 5 years ago

The workarounds don't appear to fix the problem of importing the same SVG file multiple times (e.g. a logo that appears in the header and footer). Any ideas on how to workaround that issue?

kopax commented 5 years ago

How can I configure the workaround without ejecting my create react app ?

mesqueeb commented 5 years ago

Hi guys! I'm using this extension via a vue application!

Was this feature ever implemented natively?

I often receive icon sets from designers, and each icon they exported have the same def id's, even though they're different icons... This is a problem when using the svg's inline...

So I need a way to make each icon def id unique.

strarsis commented 5 years ago

@mesqueeb: So I read through the different old issues concerning the prefixIds plugin. Currently it uses (should use) the filename of the SVG file to be optimized as the prefix. Apparently this seem to always work correctly, some users report ID collisions.

You should be able to achieve this by just enabling the prefixIds plugin, if this results in colliding IDs, I want to know so I can fix the plugin.

JoshuaSoileau commented 5 years ago

@ming-codes that solution worked perfectly, thanks a ton!

bradryanbice commented 5 years ago

@mesqueeb: So I read through the different old issues concerning the prefixIds plugin. Currently it uses (should use) the filename of the SVG file to be optimized as the prefix. Apparently this seem to always work correctly, some users report ID collisions.

You should be able to achieve this by just enabling the prefixIds plugin, if this results in colliding IDs, I want to know so I can fix the plugin.

FYI setting prefixIds: true just adds prefix__ to all of my ids, and not the filename, unless I'm missing something else I need to set up.

strarsis commented 5 years ago

.@bradryanbice: Well, prefixIds should get the file name passed by svgo, but apparently it isn't in your setup. Are you using svgo directly (CLI) or something like gulp or webpack?

bradryanbice commented 5 years ago

Using gulp, specifically gulp-svgmin utilizing prefixIds.

Here's the entire gulp task:

gulp.task('svgmin', () => {
  return gulp
    .src('content/interface/*.svg')
    .pipe(
      svgmin({
        plugins: [
          {
            removeAttrs: {
              attrs: "(fill|stroke)"
            }
          },
          {removeTitle: false},
          {removeDesc: false},
          {cleanupIDs: false},
          {
            prefixIds: {
              delim: "_"
            }
          }
        ],
        js2svg: {
          pretty: true
        }
      })
    )
    .pipe(gulp.dest('apps/icons/svg'));
});
strarsis commented 5 years ago

@bradryanbice: There is a svgo branch for testing with gulp, see this issue that seems to be very similar to yours: https://github.com/svg/svgo/issues/1148#issuecomment-527972219 So there is an issue with either gulp, gulp-svgmin or svgo (specifically the svgo release).

siwalikm commented 4 years ago

I found a workaround to this problem.

{
  cleanupIDs: {
    prefix: {
      toString() {
        this.counter = this.counter || 0;

        return `id-${this.counter++}`;
      }
    }
  }
}

JavaScript objects invokes toString when converting from object to string for concatenation. The function can be used to generate unique ids.

Thanks @ming-codes for the smart workaround. 🎉 Adding to this solution, trying something like this in my config to get unique IDs every time than starting counter from 0.

{
  cleanupIDs: {
    prefix: {
      toString() {
        return `${Math.random().toString(36).substr(2, 9)}`;
      }
    }
  }
}
angelicapabonp commented 4 years ago

I found a workaround to this problem.

{
  cleanupIDs: {
    prefix: {
      toString() {
        this.counter = this.counter || 0;

        return `id-${this.counter++}`;
      }
    }
  }
}

JavaScript objects invokes toString when converting from object to string for concatenation. The function can be used to generate unique ids.

Thanks @ming-codes for the smart workaround. 🎉 Adding to this solution, trying something like this in my config to get unique IDs every time than starting counter from 0.

{
  cleanupIDs: {
    prefix: {
      toString() {
        return `${Math.random().toString(36).substr(2, 9)}`;
      }
    }
  }
}

@ming-codes and @siwalikm both answers show a warning related (I think) to the first and last prefixId: This warning was from @siwalikm 's solution:

 Warning: Prop `id` did not match. Server: "y4lr0revm-a" Client: "7o5f5taia-a"

Or this: (it depends on which page I am on my app, and the svgs files it is rendering):

Warning: Prop `fill` did not match. Server: "url(#f45eh9v2c-a)" Client: "url(#yh8ul1tsn-a)"

But I tried @ming-codes solutions and I got the same.

Has anyone some validation or workaround?

Thnks

siwalikm commented 4 years ago

@angelicapabonp I think your warning is not related to SVGs or svgo package and something related to your framework/app.

angelicapabonp commented 4 years ago

@siwalikm I am using ReactJs and NextJs, and I followed the NextJs-babel-svg configuration and added your code to get unique Ids, and then I got those warnings

laleksiunas commented 3 years ago

I wrote a small library to resolve issues when the same SVG is imported in the same page multiple times. Maybe it will work in your case too.

mosesoak commented 2 years ago

I wrote a small library to resolve issues when the same SVG is imported in the same page multiple times.

This library solves the problem, thanks so much @laleksiunas !

I submitted a feature request to have the babel plugin uniquify top-level SVG nodes as well, which would reduce the need to use the hook in most of our cases, when used in combination with SVGO's prefixIds built-in plugin option.

For others stuck on this problem I'd steer you away from trying to solve it at any SVGO config or custom plugin level since SVGO extends Webpack's processing of each SVG asset, not individual per-page import cases, so you're going to need something like @laleksiunas 's inline-svg-unique-id Babel plugin. https://github.com/laleksiunas/inline-svg-unique-id

mosesoak commented 2 years ago

And just a side note, how strange to learn that multiple SVGs in web pages are so problematic, both in their internal reference collisions, such as clip paths, and also as a general a11y/ Accessibility problem, where every element on a page is expected to have a unique id: https://www.w3.org/WAI/standards-guidelines/act/rules/id-value-unique-3ea0c8/.

Considering this is such a general need that addresses so many developers across all frameworks, it seems like it should be addressed at a more fundamental level. Either by a great, widely-used library like SVGO if that's feasible, or some other way. Just sort of flabbergasted each time I discover one of these built-in problems with using the DOM for modern websites.

Does anyone know of any W3C-level proposals or solutions emerging in this problem space? If so please reply here, thanks!

strarsis commented 2 years ago

@mosesoak: You can use svgo for inline styles and prefixing selectors. Except for media queries, this should cover basically all possible style collisions.

First I thought CSS Cascade Layers could help, but those don't isolate but only influence the order of cascading.

Shadow DOM is mentioned as a solution that allows full isolation of SVG DOMs in the same parent document.

mosesoak commented 2 years ago

@strarsis Thanks for the reply. Yes, we did solve almost everything with SVGO alone -- except the need to uniquify the ids of two of the same instances of a single SVG on a page, e.g. checkboxes. (This need is largely an artificial constraint where the page renders fine, but it reduces our a11y score due to the rule I linked to.)

VityaSchel commented 2 years ago

How to fix problem with xlink:href? cleanupId seems to be ignoring this

strarsis commented 2 years ago

@VityaSchel: With xlink:href you mean referenced symbols? There is a plugin in the making (see PR https://github.com/svg/svgo/pull/1279). I sadly don't have the time right now to refactor it with the new transformer - but it should work - you can try it out if you want.

TrySound commented 2 years ago

Hm, is this plugin similar to yours? https://github.com/svg/svgo/pull/1655

o-alexandrov commented 2 years ago

Please let us know what would be the direction here? Are you expecting a PR to modify cleanupIDs as was proposed by different devs above? The simplest way that also takes optimization into account that was mentioned in this comment is only incrementing the counter:

cleanupIDs: {
  prefix: {
    toString() {
      this.counter = this.counter || 0
      return this.counter++
    },
  },
},
maxarias-io commented 1 year ago

How are you guys achieving that ☝️ with the new plug-in format? I can't get the counter to work since I can't override toString.

kettanaito commented 1 year ago

Hey, folks. I've had a similar problem where I needed to ensure a unique id to have unique gradient references when loading multiple SVGs on the page.

That's what worked for me:

// Generate a unique ID per the entire SVG.
const id = Math.random().toString(32).slice(2)

const thumbnailSvg = svgo.optimize(rawThumbnailSvg, {
  path: thumbnailSvgPath,
  plugins: [
    {
      name: 'prefixIds',
      params: {
        // Use that ID here.
        // Do NOT generate the ID itself in the "prefix" function
        // because that will result in each ID being unique,
        // breaking the gradient references within a single SVG.
        prefix: id,
      },
    },
  ],
})

Hope this helps.