Svgr: Making ids unique across all instances of the same svg

[mayorandrew]

🚀 Feature Proposal

Add prefixId prop to the generated React Component in order to manually prefix ids on a per-instance basis at the runtime. Like this:

import Icon from 'my.svg';

...

<Icon prefixId="icon-1" />
<Icon prefixId="icon-2" />

Motivation

Currently SVGR uses SVGO during conversion in order to prefix ids to make them unique on a per-svg basis. This is a great feature to deal with conflicts across multiple svgs.
But when the same svg is inserted twice on the page, the conflicts still occur.
Ids are prefixed at the compile time, so they are equal in both instances of the same svg which leads to conflicts.

I have experienced some weird behaviour with that.
In the example below, I hide first svg and the seconds breaks because it refers the mask by id, and it resolves to the mask included in the hidden first svg, because of the id conflict.

It seems like this issue pops up from time to time in this repo:
https://github.com/smooth-code/svgr/issues/150#issuecomment-490041335
https://github.com/smooth-code/svgr/issues/164#issue-357763613

Example

https://codesandbox.io/s/svgr-ids-xseez

Here I am using create-react-app-based codesandbox, because I am not sure if it is possible to have custom-configured webpack/parcel in CS to include SVGR there.
CRA does disables SVGO in the SVGR though, but as I mentioned above, SVGO does not fix the issue because its runs at a compile time, leaving conflicting ids if svg is instantiated twice.

Pitch

This feature will make svgr more robust and extend the range of possible use-cases.
It should not break any backwards-compatibility because when prop is not set, the ids will not be altered.

[gregberge]

Hello @mayorandrew, I think it is a good idea to try to address this problem. The good fix would be to generate a unique ID at each new rendering, you wouldn't have to specify the "prefixId", it would be unique by Component instance.

I don't have enough time to work on it but if someone want to give a try by implementing it, he is welcome.

[mayorandrew]

Hi @neoziro, thanks for the response!
I though about proposing generation of unique ids, but I have decided that it may be better to leave that up to user. My reasoning is that someone may rely upon having predictable ids in the svg. Maybe in css or jquery-like scripts, etc. So if ids are unique by default, it will be a breaking change. But if we add something like a uniqueIds prop, it is almost the same as adding the prefixIds prop.
Given that, in my opinion most use-cases would simply require having unique ids.

I see two options here:

• Add uniqueIds as the boolean prop for most use-cases and prefixIds as the escape hatch
• Make ids unique by default and add prefixIds prop which can accept an empty string to disable that, or non-empty string to have any other predictable prefix

Would like to hear your opinion on that!

[gregberge]

Hello @mayorandrew, yeah I think you are right. Any idea to implement uniqueIds?

[chaffeqa]

I'd love to see if there is a way to fix this as well... a similar problem we have run into is 2 fold. Code best explains it:

import * as React from "react";

let cnt = 0;

class OneMillionSvg extends React.PureComponent<any> {

  public id = "";

  public render() {
    // optionally pass in an id if you know exactly that there are x of these on a page for example
    const {disabled, id, ...rest} = this.props;
    // We have to increment the id so that multiple instances of this SVG on the page at a time don't collide (which produces rendering issues in some browsers like chrome)
    if (!this.id) {
      cnt += 1;
      this.id = `OneMillionSvg--a${id || cnt}`;
    }
    return (
      <svg viewBox="0 0 43 20" xmlns="http://www.w3.org/2000/svg" {...rest}>
        <defs>
          <linearGradient x1="19.692%" y1="43.665%" y2="57.716%" id={this.id}>
            <stop stopColor="#14B9B3" offset="0%"/>
            <stop stopColor="#007F4E" offset="100%"/>
          </linearGradient>
        </defs>
        <g fill="none" fillRule="evenodd">
          <rect fill={disabled ? "#DFE2E6" : `url(#${this.id})`} width="43" height="20" rx="2"/>
          {/* tslint:disable-next-line */}
          <path d="M9.45 14.188v-.965c-1.082-.107-1.868-.52-2.45-1.093l.93-1.422c.367.382.888.721 1.52.86v-1.179C8.4 10.145 7.226 9.7 7.226 8.193c0-1.072.786-2.048 2.226-2.218v-.997h1.133v1.008c.817.116 1.532.435 2.114.944l-.96 1.369a3.067 3.067 0 0 0-1.154-.616v1.03c1.062.254 2.277.7 2.277 2.175 0 1.273-.806 2.165-2.277 2.335v.965H9.451zm1.56-2.969c0-.18-.192-.295-.483-.38v.837c.301-.076.482-.238.482-.457zM8.876 7.94c0 .18.161.29.432.371v-.804c-.301.05-.432.211-.432.432zm11.55 7.218h-2.685v-6.9l-1.748 1.744-1.53-1.59 3.637-3.55h2.325v10.296zm14.205 0h-2.684V8.35l-2.622 6.807H28.14L25.503 8.35v6.807h-2.684V4.862h3.73l2.184 5.68 2.17-5.68h3.729v10.295z" fill={disabled ? "#959EA6" : "#FFF"} fillRule="nonzero"/>
        </g>
      </svg>
    );
  }
}

export default OneMillionSvg;

The alternative is to do what SVGO does, which is do away with <def>s and instead just make multiple copies of something like <linearGradient>. The problem with this is that is a huge perf issue with some SVG's that reuse a gradient in many paths. One of our SVGs processed by SVGO turned from 200 LOC to 1500 LOC because of that (example below illustrating):

// ...
  {/* tslint:disable-next-line */}
  <path d="M58.32 91.58c15.37-2.43 16.26-1.38 14.81 10.9-3.44.67-6.89 1.37-10.34 2.08.71-5.59.79-5.8-5.56-4.79-6.08.98-6.45 1.64-8.18 14.07-1.73 12.44-1.52 12.96 4.71 11.98 6.16-.96 6.47-1.16 7.18-6.76 3.5-.74 7-1.45 10.49-2.14-1.58 13.47-2.78 14.66-18.76 17.1-15.65 2.48-16.97 1.01-14.09-18.36 2.87-19.3 4.7-21.59 19.74-24.08" fill="url(#SVGID_12_)"/>
  {/* tslint:disable-next-line */}
  <linearGradient id="SVGID_13_" gradientUnits="userSpaceOnUse" x1="97.317" y1="70.258" x2="97.317" y2="123.656">
    <stop offset="0" stopColor="#fff"/>
    <stop offset=".26" stopColor="#f9fbfc"/>
    <stop offset=".6" stopColor="#e9eef2"/>
    <stop offset=".99" stopColor="#cedae3"/>
    <stop offset="1" stopColor="#cdd9e2"/>
  </linearGradient>
  {/* tslint:disable-next-line */}
  <path d="M99.55 86.01c15.48-1.62 16.95.27 15.46 19.56-1.49 19.37-3.33 21.25-19.44 22.87-16.28 1.75-17.78.22-15.64-19.15C82.06 90 83.91 87.76 99.55 86.01m-3.21 34.24c7.02-.72 7.3-1.31 8.38-13.75 1.08-12.43.91-13.02-5.94-12.3-6.91.75-7.26 1.4-8.51 13.83-1.24 12.45-1.01 12.98 6.07 12.22" fill="url(#SVGID_13_)"/>
  {/* tslint:disable-next-line */}
  <linearGradient id="SVGID_14_" gradientUnits="userSpaceOnUse" x1="137.825" y1="67.299" x2="137.825" y2="120.873">
    <stop offset="0" stopColor="#fff"/>
    <stop offset=".26" stopColor="#f9fbfc"/>
    <stop offset=".6" stopColor="#e9eef2"/>
    <stop offset=".99" stopColor="#cedae3"/>
    <stop offset="1" stopColor="#cdd9e2"/>
  </linearGradient>
  {/* tslint:disable-next-line */}
  <path d="M141.33 83.25c-.13 2.49-.26 4.99-.4 7.48-1.24.14-2.48.29-3.72.43-.49 8.6-.98 17.21-1.46 25.81 1.26.01 2.53.02 3.79.04 2.15-.11 4.3-.21 6.45-.3l.99-7.75c2.55-.1 5.1-.2 7.65-.28-.2 5.07-.4 10.14-.59 15.21-10.99.34-22 .9-33.02 1.7.17-2.49.35-4.99.52-7.48 1.28-.17 2.55-.34 3.83-.5.57-8.6 1.14-17.21 1.72-25.81l-3.7.03c.17-2.49.35-4.99.52-7.48 5.81-.43 11.62-.8 17.42-1.1" fill="url(#SVGID_14_)"/>
  <linearGradient id="SVGID_15_" gradientUnits="userSpaceOnUse" x1="178.815" y1="66.678" x2="178.815" y2="119.092">
    <stop offset="0" stopColor="#fff"/>
    <stop offset=".26" stopColor="#f9fbfc"/>
    <stop offset=".6" stopColor="#e9eef2"/>
    <stop offset=".99" stopColor="#cedae3"/>
    <stop offset="1" stopColor="#cdd9e2"/>
  </linearGradient>
// ...

I think the solution for both situations would have to be fairly involved... something with a Context that has a "hidden" SVG node with all the current <def>s... kinda like a styled-components approach if that makes sense.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[prajeshm07]

Hi @gregberge, any update on this feature?

[gregberge]

Hello @prajeshm07, I am sorry but it won't happen. You should avoid using "ids" in SVG. Using plain shape is a better thing. See all icons from popular libraries like Font awesome, they do not have any id.