Unique ids for all icon elements referenced through url(#someId)

[VitGottwald]

All elements that use id attribute in icons now generate a unique identifier for every mount of the component. It fixes #4.

[VitGottwald]

My apologies for the huge diffs. I had to convert the bodies of the components from expressions to code block so that I could use useUniqueId. I used prettier to format it.

[VitGottwald]

These are the changes I made:

• created useUniqueId.tsx custom hook to generate new id on every mount
• updated all icons that define elements with id="someId" and use url(#someId) to refer to it - appended _${id} in all such places
• written tests to
  • make sure all elements with the id attribute id="someId" have different ids
  • validate that two renders of the same icon generate unique ids

[MichaelDeBoey]

Hi @baremetalfreak!

Thanks for this great improvement!

Just left some small remarks 🙂

[MichaelDeBoey]

You can make this an implicit return

[MichaelDeBoey]

Also: shouldn’t counter be the dependency of useMemo? 🤔

[VitGottwald]

Not really. Hooks are executed during render and their dependencies should come from properties and state. There is no way for react to know when to re-evaluate a hook if something outside of its control has changed.

This hook is essentially a poor man's hack to facebook/react#1137 . I have not found a cleaner way to do it.

Do you want me to add some comment into the code to explain it?

[VitGottwald]

What we want is a unique identifier for every "instance" of a component. That is achieved by the useMemo's second argument [] which says, execute the function and return new, incremented, value on every mount. And a mount is when a new "instance" is created.

[VitGottwald]

Does that make sense @MichaelDeBoey ?

[MichaelDeBoey]

Maybe adding a little comment here would make this a bit clearer indeed. 🙂

[VitGottwald]

Done https://github.com/codesandbox/codesandbox-templates/pull/5/files#diff-7daac9e6713c1145ad0340ef5927b3cdR3

[MichaelDeBoey]

LGTM

[VitGottwald]

Is there something you would like me to do with this PR before merging it?

[Saeris]

Hey @baremetalfreak, thanks for the PR! Sorry it took me so long to get around to looking at it, but it looks great to me! I'll merge this and publish a new version this evening. Love that you added tests as well!