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

ID collisions with multiple SVG #150

Closed
YannickAWE opened this issue Aug 1, 2018 · 11 comments · Fixed by JetBrains/logos#23
Closed

ID collisions with multiple SVG #150

YannickAWE opened this issue Aug 1, 2018 · 11 comments · Fixed by JetBrains/logos#23

Comments

@YannickAWE
Copy link

YannickAWE commented Aug 1, 2018

With React + Webpack when I load multiple SVG in the same component (SVG is in the dom), ids are the same in multiple SVG :

First SVG result :

<svg width="158" height="185" xmlns:xlink="http://www.w3.org/1999/xlink" class="sc-fYxtnH hSLfWw">
<defs>
<linearGradient x1="0%" y1="100%" y2="0%" id="c">
...

Second SVG result :

<svg width="23" height="23" xmlns:xlink="http://www.w3.org/1999/xlink">
  <defs>
      <path d="M22.733 1-.267-2.09z" id="c"></path>
  </defs>
  <g fill="none" fill-rule="evenodd"> 
      <g>
          <mask id="d" fill="#fff">
              <use xlink:href="#c"></use>
          </mask>
          <path fill="#EA4335" fill-rule="nonzero" mask="url(#d)" d="M-1.07 4.705L8.023 11.5l3.744-3.189 12.838-2.038v-7.318H-1.07z"></path>
      </g>
...

The second SVG is not rendered correctly because of that. When i remove the first SVG everything works fine.

Here is my Webpack config :

{
  test: /\.svg$/,
  use: [{
    loader:'@svgr/webpack',
    options: {
      "svgoConfig": {
        "plugins": [{ "cleanupIDs": true }]
      }
    }
  }]
}

Is there something I forgot ?
Thanks

@gregberge
Copy link
Owner

I met this problem several times. The best solution is to avoid using mask in SVG, if you are using Sketch you can flatten it to create a complete shape instead of using mask. Ids are not scoped this is why there is a problem in multiple SVG.

On SVGR part, we could maybe mitigate the problem by adding something that scope ids to the SVG file.

Imagine a foo.svg:

<svg width="23" height="23" xmlns:xlink="http://www.w3.org/1999/xlink">
  <defs>
      <path d="M22.733 1-.267-2.09z" id="c"></path>
  </defs>
</svg>

That will be transformed into Foo.js:

<svg width="23" height="23">
  <defs>
      <path d="M22.733 1-.267-2.09z" id="foo-1"></path>
  </defs>
</svg>

But the problem remain when we don't have a filename (stdin) or when we have nested directories. So the best solution is to avoid ids in inline SVG.

@gregberge
Copy link
Owner

Seems like SVGO already has this feature see #152.

@jjmax75
Copy link

jjmax75 commented Aug 22, 2018

sorry for jumping in on this issue - to confirm should -

{
  test: /\.svg$/,
  use: [{
    loader:'@svgr/webpack',
    options: {
      "svgoConfig": {
        "plugins": [{ "cleanupIDs": true }]
      }
    }
  }]
}

work?

@jjmax75
Copy link

jjmax75 commented Aug 22, 2018

found this - svg/svgo#674 (comment)

the next one is a fix too

Hope it helps

 {
            test: /\.svg$/,
            use: ({resource}) => ({
                loader: '@svgr/webpack',
                options: {
                  svgoConfig: {
                    plugins: [{
                      "cleanupIDs": {
                        "prefix": `svg${hash(relative(context, resource))}`
                      }
                    }]
                  }
                }
              })
          }

@YannickAWE
Copy link
Author

Thank you @jjmax75 it works

@wbobeirne
Copy link

I ran into this with linear gradients, fortunately some searching around led me here. However, this seems like a pretty disastrous default. I was able to fix mine with a simplified version of the hashing solution:

const hash = require('string-hash');

...

use: ({ resource }) => ({
  loader: '@svgr/webpack',
  options: {
    svgoConfig: {
      plugins: [{
        cleanupIDs: {
          prefix: `svg-${hash(resource)}`,
        },
      }],
    },
  },
}),

Is there a way we could get the default behavior to be more like this hashing solution? Seems like this is a fairly common problem.

@nghiepdev
Copy link

Hi all,
Have anyone maintained this issue?

I have a problem. Don't work with render list component.
Example:

import AvatarSvgIcon from './avatar.svg';

render() {
   return this.prop.users.map(user => <div key={user.id}><h1>{user.name}</h1><AvatarSvgIcon /></div>)
}

@artola
Copy link

artola commented Dec 19, 2019

I met this problem several times. The best solution is to avoid using mask in SVG, if you are using Sketch you can flatten it to create a complete shape instead of using mask. Ids are not scoped this is why there is a problem in multiple SVG.

On SVGR part, we could maybe mitigate the problem by adding something that scope ids to the SVG file.

Imagine a foo.svg:

<svg width="23" height="23" xmlns:xlink="http://www.w3.org/1999/xlink">
  <defs>
      <path d="M22.733 1-.267-2.09z" id="c"></path>
  </defs>
</svg>

That will be transformed into Foo.js:

<svg width="23" height="23">
  <defs>
      <path d="M22.733 1-.267-2.09z" id="foo-1"></path>
  </defs>
</svg>

But the problem remain when we don't have a filename (stdin) or when we have nested directories. So the best solution is to avoid ids in inline SVG.

Even when using the filename, hash or random to get an id, it will be repeated if is included several times in the page.

May be a note in the documentation could be useful, pointing to the Specs.

While this specification defines requirements for class, id, and slot attributes on any element, it makes no claims as to whether using them is conforming or not.

As I understand, there is no restriction to include the same ID several times, but it is not of common sense, and getElementById will match the first one in the subtree as per specs.

@gregberge do you have some recommendation?

@gregberge
Copy link
Owner

I avoid to use ID in inline SVG, it is not reliable. Inline SVG are components, you would have the same problem in React components. Anyway there is a way to solve it. Edit by your own the SVG and use a hook to get a unique ID by component instance (example: https://gist.github.com/sqren/fc897c1629979e669714893df966b1b7).

@VityaSchel
Copy link

found this - svg/svgo#674 (comment)

the next one is a fix too

Hope it helps

 {
            test: /\.svg$/,
            use: ({resource}) => ({
                loader: '@svgr/webpack',
                options: {
                  svgoConfig: {
                    plugins: [{
                      "cleanupIDs": {
                        "prefix": `svg${hash(relative(context, resource))}`
                      }
                    }]
                  }
                }
              })
          }

What is hash? relative? context? How would it fix multiple same inline svgs on one page when first svg takes all ids and next has no gradients and masks?

@VityaSchel
Copy link

found this - svg/svgo#674 (comment)
the next one is a fix too
Hope it helps

 {
            test: /\.svg$/,
            use: ({resource}) => ({
                loader: '@svgr/webpack',
                options: {
                  svgoConfig: {
                    plugins: [{
                      "cleanupIDs": {
                        "prefix": `svg${hash(relative(context, resource))}`
                      }
                    }]
                  }
                }
              })
          }

What is hash? relative? context? How would it fix multiple same inline svgs on one page when first svg takes all ids and next has no gradients and masks?

So my problem is that I import a component that imports svg using @svgr/webpack plugin several times on page, but every time it has same ID even if I add Math.random in cleanupIDs plugin options.

I have no solution for that problem and maybe I need to open a new issue, but for now I'll just import same svg as many times as I need unique ID, and pass imported inline svg component to the rendering component.

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.

7 participants