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

Option to generate unique IDs? #674

Closed
pluma opened this issue Mar 2, 2017 · 48 comments
Closed

Option to generate unique IDs? #674

pluma opened this issue Mar 2, 2017 · 48 comments

Comments

@pluma
Copy link

pluma commented Mar 2, 2017

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
Copy link
Member

GreLI commented Mar 2, 2017

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

@Kraxxis
Copy link

Kraxxis commented Mar 10, 2017

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
Copy link

Emasoft commented Mar 10, 2017

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

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

@pluma
Copy link
Author

pluma commented Mar 10, 2017

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

@Emasoft
Copy link

Emasoft commented Mar 10, 2017

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
Copy link
Contributor

strarsis commented Mar 20, 2017

Similar issue?: #673

@OwenMelbz
Copy link

OwenMelbz commented Apr 5, 2017

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
Copy link
Contributor

strarsis commented Apr 5, 2017

PR for a plugin that does this - a bit tweaking is necessary though: #700

@GreLI
Copy link
Member

GreLI commented Apr 5, 2017

@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.

@GreLI GreLI closed this as completed Apr 5, 2017
@OwenMelbz
Copy link

OwenMelbz commented Apr 5, 2017

@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
Copy link
Member

GreLI commented Apr 5, 2017

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

This comment has been minimized.

@GreLI
Copy link
Member

GreLI commented Apr 5, 2017

As you wish.

@walnutpedia
Copy link

walnutpedia commented Jul 26, 2017

@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
Copy link

jt3k commented Jul 27, 2017

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

@10xjs
Copy link

10xjs commented Aug 23, 2017

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
Copy link

ming-codes commented Sep 12, 2017

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
Copy link

tristanisme commented Oct 11, 2017

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
Copy link

CH-RhyMoore commented Feb 28, 2018

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
Copy link

mattkime commented May 12, 2018

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
Copy link

o-alexandrov commented Jun 10, 2018

If anyone happens to wonder how to fix broken IDs, go to the following comment:
#913 (comment)

@jt3k
Copy link

jt3k commented Jun 13, 2018

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
Copy link

chrisfinch commented Jul 5, 2018

Anyone who encounters this whilst using react-svg-loader can find my working fix here: boopathi/react-svg-loader#218 (comment)

@SilverFox70
Copy link

SilverFox70 commented Aug 21, 2018

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.

@JoshuaSoileau
Copy link

JoshuaSoileau commented Jun 3, 2019

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

@bradryanbice
Copy link

bradryanbice commented Oct 10, 2019

@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
Copy link
Contributor

strarsis commented Oct 10, 2019

.@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
Copy link

bradryanbice commented Oct 10, 2019

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
Copy link
Contributor

strarsis commented Oct 10, 2019

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

@siwalikm
Copy link

siwalikm commented Jan 6, 2020

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
Copy link

angelicapabonp commented May 1, 2020

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
Copy link

siwalikm commented May 2, 2020

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

@angelicapabonp
Copy link

angelicapabonp commented May 4, 2020

@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
Copy link

laleksiunas commented Sep 25, 2021

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
Copy link

mosesoak commented Feb 15, 2022

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
Copy link

mosesoak commented Feb 15, 2022

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
Copy link
Contributor

strarsis commented Feb 15, 2022

@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
Copy link

mosesoak commented Feb 16, 2022

@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
Copy link

VityaSchel commented Mar 7, 2022

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

@strarsis
Copy link
Contributor

strarsis commented Mar 7, 2022

@VityaSchel: With xlink:href you mean referenced symbols?
There is a plugin in the making (see PR #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
Copy link
Member

TrySound commented Mar 7, 2022

Hm, is this plugin similar to yours?
#1655

@o-alexandrov
Copy link

o-alexandrov commented May 14, 2022

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++
    },
  },
},

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

No branches or pull requests