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

SVG duplicate ids on one page #57

Open
nghiepdev opened this issue Jan 15, 2019 · 25 comments · Fixed by exogen/babel-plugin-inline-react-svg#1
Open

SVG duplicate ids on one page #57

nghiepdev opened this issue Jan 15, 2019 · 25 comments · Fixed by exogen/babel-plugin-inline-react-svg#1

Comments

@nghiepdev
Copy link

Hi all,

The current v1.0.1 duplicate ids on one page.

I think the problem with svgo 0.7.2

@ljharb
Copy link
Collaborator

ljharb commented Jan 15, 2019

Can you elaborate? What's the content of your svg?

@nghiepdev
Copy link
Author

@ljharb

2 examples my icon:

<polygon id="path-1" will convert to <polygon id="a" in both icons

<?xml version="1.0" encoding="UTF-8"?>
<svg width="22px" height="18px" viewBox="0 0 22 18" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <!-- Generator: Sketch 52.5 (67469) - http://www.bohemiancoding.com/sketch -->
    <title>Group 3 Copy 5</title>
    <desc>Created with Sketch.</desc>
    <defs>
        <polygon id="path-1" points="0 0 21.1304348 0 21.1304348 18 0 18"></polygon>
    </defs>
    <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
        <g id="Home" transform="translate(-1218.000000, -1853.000000)">
            <g id="Group-26" transform="translate(321.000000, 1282.000000)">
                <g id="Group-3-Copy-5" transform="translate(897.000000, 571.000000)">
                    <mask id="mask-2" fill="white">
                        <use xlink:href="#path-1"></use>
                    </mask>
                    <g id="Clip-2"></g>
                    <path d="M18.1312581,8.50780063 L10.5652313,15.8740807 L2.99898159,8.50780063 C1.41720897,6.96804996 1.41720897,4.4625642 2.99898159,2.92194546 C3.76534775,2.17626775 4.78449438,1.76588459 5.86783921,1.76588459 C6.95140695,1.76588459 7.97010775,2.17626775 8.73602809,2.92194546 L9.92414084,4.07909141 C10.2645251,4.41004557 10.8663834,4.41004557 11.2067676,4.07909141 L12.3942116,2.92194546 C13.1603549,2.17626775 14.1792786,1.76588459 15.2630693,1.76588459 C16.346637,1.76588459 17.3651149,2.17626775 18.1312581,2.92194546 C19.7132537,4.4625642 19.7132537,6.96804996 18.1312581,8.50780063 M19.4134391,1.67365147 C18.3049054,0.594198355 16.8310214,0 15.2628463,0 C13.6946713,0 12.2205644,0.594198355 11.1115849,1.67365147 L10.5652313,2.20556533 L10.0188778,1.67365147 C8.91012116,0.594198355 7.43579138,0 5.86783921,0 C4.29988704,0 2.82533435,0.594198355 1.71680063,1.67365147 C-0.572266877,3.90222083 -0.572266877,7.52839334 1.71680063,9.75609462 L9.91879099,17.7415302 C10.0527602,17.8723928 10.2197201,17.9526898 10.394259,17.9837236 C10.4526615,17.9947915 10.5115099,18 10.5705812,18 C10.802631,18 11.0349037,17.9138434 11.2116717,17.7415302 L19.413662,9.75609462 C21.7027295,7.52839334 21.7027295,3.90222083 19.4134391,1.67365147" id="Fill-1" fill="#E82D67" mask="url(#mask-2)"></path>
                </g>
            </g>
        </g>
    </g>
</svg>
<?xml version="1.0" encoding="UTF-8"?>
<svg width="43px" height="22px" viewBox="0 0 43 22" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <!-- Generator: Sketch 52.5 (67469) - http://www.bohemiancoding.com/sketch -->
    <title>Group 10 Copy 2</title>
    <desc>Created with Sketch.</desc>
    <defs>
        <polygon id="path-1" points="0 0.00285020243 42.9615377 0.00285020243 42.9615377 21.8218534 0 21.8218534"></polygon>
    </defs>
    <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
        <g id="Home" transform="translate(-1199.000000, -1541.000000)">
            <g id="Group-26" transform="translate(321.000000, 1282.000000)">
                <g id="Group-10-Copy-2" transform="translate(878.000000, 259.000000)">
                    <g id="Group-3" transform="translate(0.000000, 0.086219)">
                        <mask id="mask-2" fill="white">
                            <use xlink:href="#path-1"></use>
                        </mask>
                        <g id="Clip-2"></g>
                        <path d="M42.9614486,2.09497895 L42.9614486,1.44317328 C42.9614486,0.647699595 42.3165903,0.00284129555 41.5211166,0.00284129555 L1.44032308,0.00284129555 C0.644849393,0.00284129555 -8.90688259e-06,0.647699595 -8.90688259e-06,1.44317328 L-8.90688259e-06,20.3815215 C-8.90688259e-06,21.1769951 0.644849393,21.8218534 1.44032308,21.8218534 L41.5212057,21.8218534 C42.3166794,21.8218534 42.9615377,21.1769951 42.9615377,20.3815215 L42.9615377,19.7297158 C42.9615377,19.1704526 42.6337644,18.6587522 42.1199263,18.4378615 C39.2043474,17.184485 37.1623555,14.2872543 37.1623555,10.9123474 C37.1623555,7.53744049 39.2043474,4.64020972 42.1199263,3.38674413 C42.6337644,3.16594251 42.9614486,2.65424211 42.9614486,2.09497895" id="Fill-1" fill="#FDCB31" mask="url(#mask-2)"></path>
                    </g>
                    <path d="M8.89475142,16.2096713 L4.91889717,6.69881296 C4.83125344,6.4886996 4.76115628,6.27849717 4.76115628,6.05074818 C4.76115628,5.29758219 5.35667045,4.71961457 6.10983644,4.71961457 C6.81045182,4.71961457 7.26577166,5.12247287 7.47606316,5.66534737 L10.5411887,13.5995984 L13.6412291,5.57761457 C13.8164275,5.13975223 14.289383,4.71943644 14.9373587,4.71943644 C15.6730672,4.71943644 16.2686704,5.27985749 16.2686704,6.01556599 C16.2686704,6.22567935 16.1984842,6.45342834 16.1284761,6.61108016 L12.1176178,16.209404 C11.8373182,16.8750154 11.3293587,17.2776955 10.5937393,17.2776955 L10.4185409,17.2776955 C9.68301053,17.2779628 9.17505101,16.8751045 8.89475142,16.2096713" id="Fill-4" fill="#FFFFFF"></path>
                    <path d="M18.4587393,6.06817004 C18.4587393,5.31500405 19.0543425,4.71957895 19.8073304,4.71957895 C20.5604964,4.71957895 21.1560996,5.31500405 21.1560996,6.06817004 L21.1560996,15.8416032 C21.1560996,16.5947692 20.5604964,17.1903725 19.8073304,17.1903725 C19.0543425,17.1903725 18.4587393,16.5947692 18.4587393,15.8416032 L18.4587393,6.06817004 Z" id="Fill-6" fill="#FFFFFF"></path>
                    <path d="M29.0558008,11.0075352 C30.40457,11.0075352 31.1927401,10.2019077 31.1927401,9.15089555 L31.1927401,9.1158915 C31.1927401,7.9073166 30.3519304,7.25934089 29.0032502,7.25934089 L26.9189506,7.25934089 L26.9189506,11.0075352 L29.0558008,11.0075352 Z M24.2215903,6.17341377 C24.2215903,5.42024777 24.8171935,4.82464453 25.5703595,4.82464453 L29.2308211,4.82464453 C32.1560194,4.82464453 33.9250154,6.55863644 33.9250154,9.06334089 L33.9250154,9.09825587 C33.9250154,11.9357215 31.718068,13.4069603 28.9682462,13.4069603 L26.9189506,13.4069603 L26.9189506,15.8415676 C26.9189506,16.5947336 26.3233474,17.1904259 25.5703595,17.1904259 C24.8171935,17.1904259 24.2215903,16.5947336 24.2215903,15.8415676 L24.2215903,6.17341377 Z" id="Fill-8" fill="#FFFFFF"></path>
                </g>
            </g>
        </g>
    </g>
</svg>

@ljharb
Copy link
Collaborator

ljharb commented Jan 15, 2019

See #46; you may want to reconsider having IDs in your svgs. Why are they there?

@florianmutter
Copy link

As far as I know you need ids for stuff like gradients on paths. This plugin does cleanup the ids by default. So is the only way to handle situations like this to disable the cleanupIds open?

"plugins": [
    [
        "inline-react-svg",
        {
            "svgo": {
                "plugins": [
                    {
                        "cleanupIDs": {
                            "minify": false
                        }
                    }
                ]

            }
        }
    ]
]

I think it would be great if this plugin or svgo could randomize the ids.

@ljharb
Copy link
Collaborator

ljharb commented Jan 24, 2019

In the example the id “Fill-6”, eg, doesn’t seem to be referenced anywhere else - which suggests that either it’s styled from outside (meaning it couldn’t be randomized) or that it’s unused (meaning it could be deleted). What am i missing?

@florianmutter
Copy link

I did't look at the example. Here is one where (as far as I know) the id is needed:

<svg height="150" width="400">
  <defs>
    <linearGradient id="grad1" x1="0%" y1="0%" x2="100%" y2="0%">
      <stop offset="0%" style="stop-color:rgb(255,255,0);stop-opacity:1" />
      <stop offset="50%" style="stop-color:rgb(255,0,255);stop-opacity:1" />
      <stop offset="100%" style="stop-color:rgb(255,0,0);stop-opacity:1" />
    </linearGradient>
  </defs>
  <ellipse cx="200" cy="70" rx="85" ry="55" fill="url(#grad1)" />
</svg>

@wouter-willems
Copy link

I also get the same issues. Had to turn the minification off. Is this going to be picked up?

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2019

a PR is welcome.

@mikefowler
Copy link

mikefowler commented Feb 27, 2019

@ljharb if we upgrade to the next major version of svgo, these types of issues should go away. For example, as of [email protected], the following is possible:

// babel.config.js
const path = require('path');

const iconPrefixes = [];

module.exports = (api) => {
  api.cache(true);

  const presets = [/* ... */];

  const plugins = [
    // ...
    [
      'inline-react-svg',
      {
        svgo: {
          plugins: [
            {
              prefixIds: (node, extra) => {
                // Generate the ID prefix from the file's basename
                const prefix = path.basename(extra.path);

                // Keep track of prefixes for the cleanupIDs plugin
                iconPrefixes.push(prefix);
                
                return prefix;
                },
            },
            {
              cleanupIDs: {
                // Minify IDs, but preserve their prefixes
                preservePrefixes: iconPrefixes,
              },
            },
          ],
        },
      },
    ],
  ];

  return {
    presets,
    plugins,
  };
};

It seems like the only breaking change in v1+ was requiring Node v4+.

/cc @nghiepit @wouter-willems @florianmutter

@ljharb
Copy link
Collaborator

ljharb commented Feb 27, 2019

@mikefowler thanks! that would certainly be semver-major for this plugin, but seems easy enough to adapt to. A PR would be welcome :-)

@mikefowler
Copy link

@ljharb you bet, working on it!

@mikefowler
Copy link

mikefowler commented Feb 27, 2019

After digging in a bit, I found this comment to have accurately summed up the issue with upgrading svgo (namely, .optimize is no longer synchronous).

@ljharb, given that you have previously stated that we are not interested in using a package that makes Promises synchronous, it seems we truly are blocked on upgrading svgo and providing the functionality I mentioned above. Bummer.

The maintainer of SVGR opened an issue over a year ago about re-implementing a sync mode for svgo and there has been no response from the maintainers.

For those who stumble onto this issue, babel-plugin-inline-svg is using synchronized-promise and a newer version of svgo, allowing for the functionality above.

@ljharb
Copy link
Collaborator

ljharb commented Feb 27, 2019

@mikefowler ah, I'd forgotten about that :-/ unless babel 7 added a way for transforms to run asynchronously, i don't think we really have much of an option.

I suppose we could fork svgo; another (unfortunate) option would be to synchronously invoke a separate process that provides the results.

@Trainbird
Copy link

I just had a look into Babel 7 and it looks as if there was an async way of transforming now. See the compat note in the Babel Docs for infos.

Don't actually know, if that means that visitors also can be async though. Can't check it, because I'm not at the code right now. Also quite busy, hence the terrible response time 🙈

@ghost
Copy link

ghost commented Mar 22, 2019

@Trainbird @ljharb I came across this issue as the js-yaml package inside svgo has a vulnerability.

I was looking into the SVGO Promise breaking change and came across error thrown if the return value would be a promise:

You appear to be using a plugin with an async traversal visitor, which your current version of Babel does not support. If you're using a published plugin, you may need to upgrade your @babel/core version.

Checking the latest core code, this still seems to be unsupported:

https://github.com/babel/babel/blob/master/packages/babel-traverse/src/path/context.js#L32

@nghiepdev
Copy link
Author

Moved to @svgr/webpack instead

@ljharb ljharb reopened this Mar 26, 2019
@msingleton
Copy link

Also just bumped into this issue, importing 3 SVGs on a page and they all render as the same image since the ids get mapped to the same id.

I'll just manually edit the SVG (exported from figma), but just adding a +1 for seeing if there's a way to support this without needing to do that.

@exogen
Copy link

exogen commented Feb 5, 2020

If anyone is interested in testing out a solution to this, I've merged exogen#1 (and a followup hotfix) into master of that fork, and you should be able to install it by specifying that repo as the dependency version in your package.json: https://github.com/exogen/babel-plugin-inline-react-svg.git (it's currently unpublished).

Read that pull request for more info on the how and why.

While there are some workarounds for rendering different SVGs on the same page whose source files have ID collisions (you can just edit the SVG, or use the cleanupIDs option to change them per SVG), that doesn't help the situation of rendering multiple of the same SVG on one page.

For this to be fully solved you need unique IDs per instance, not just per component. So if you render <HeartIcon /><HeartIcon /> they still shouldn't conflict. That's what the patch there does.

@maarekj
Copy link

maarekj commented Apr 23, 2020

Option prefixIds should me modifiable from the props, dynamically instead of statically.

<div>
    <HeartIcon prefixIds="icon-1-" />
    <HeartIcon prefixIds="icon-2-" />
</div>

@beqramo
Copy link

beqramo commented Jul 27, 2020

have you find any solution on this?

@t4t5
Copy link

t4t5 commented Feb 25, 2021

This was driving me nuts, because usually the page would render great except for one SVG causing problems. I decided to write a small utility function just to deal with those cases.

Warning: This code is not efficient at all, and shouldn't be used unless you're desperate but lazy. 😄

import { useRef, useEffect } from "react"

const URL_REGEX = /url\(#([a-z])\)/

export default function prefixSvgIds(file, prefix) {
  const ref = useRef()

  useEffect(() => {
    const descendants = ref.current.querySelectorAll("svg *")
    descendants.forEach((descendant) => {
      for (const attr of descendant.attributes) {
        // Replace things like id="#a"
        if (attr.name === "id") {
          if (attr.value.includes(prefix)) return
          const newId = `${prefix}__${attr.value}`
          descendant.setAttribute(attr.name, newId)
        }

        // Replace things like fill="#a"
        const match = attr.value.match(URL_REGEX)
        if (match) {
          if (attr.value.includes(prefix)) return
          const newId = `url(#${prefix}__${match[1]})`
          descendant.setAttribute(attr.name, newId)
        }
      }
    })
  }, [])

  return <span ref={ref}>{file}</span>
}

Then when rendering your SVG, you replace something like this...

<MyIcon />

...with this:

{prefixSvgIds(<MyIcon />, "my-prefix")}

This will turn all id="a" into id="my-prefix__a", and all url(#a) into url(#my-prefix__a).

@0kzh
Copy link

0kzh commented Jun 15, 2021

This is still an issue and hinders the scalability of this plugin when your site has multiple SVGs - rendering issues are almost guaranteed since ids are transformed into alphabet chars

@laleksiunas
Copy link

I took inspiration from @t4t5 and wrote a small library which does id prefix logic at build-time and generation at runtime. It works well with the same icon repeated multiple times in the same page.

@miketheodorou
Copy link

miketheodorou commented Nov 12, 2021

Hey everyone, I was able to solve this issue using this config in my .babelrc

  "plugins": [
    [
      "inline-react-svg",
      {
        "svgo": {
          "plugins": [
            {
              "name": "preset-default",
              "params": {
                "overrides": {
                  "cleanupIDs": false
                }
              }
            }
          ]
        }
      }
    ]
  ]

@ljharb

This comment was marked as resolved.

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

Successfully merging a pull request may close this issue.