icon with mask may cause memory leak on ssr [nodejs]

[nkrtarm]

import React from 'react'
import FontAwesomeIcon from '@fortawesome/react-fontawesome'
const Test = () => (
  <div>
    <FontAwesomeIcon icon="star" mask="circle" />
  </div>
)
export default Test

when i add fontawesome icon with mask props, variable name "_uniqueId" in file (@fortawesome/fontawesome/index.js) keep increasing on server side everytime the component was rendered through ReactDOM.renderToString(appComponent)

@fortawesome/fontawesome/index.js

what i got from server rendered <svg aria-hidden="true" data-prefix="fas" data-icon="star" class="svg-inline--fa fa-star fa-w-16 " role="img" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><defs><clipPath id="clip-12"><path fill="currentColor" d="M256 8C119 8 8 119 8 256s111 248 248 248 248-111 248-248S393 8 256 8z"></path></clipPath><mask x="0" y="0" width="100%" height="100%" id="mask-11" maskUnits="userSpaceOnUse" maskContentUnits="userSpaceOnUse"><rect x="0" y="0" width="100%" height="100%" fill="white"></rect><g transform="translate(256 256)"><g transform="translate(0, 0) scale(1, 1) rotate(0 0 0)"><path fill="black" d="M259.3 17.8L194 150.2 47.9 171.5c-26.2 3.8-36.7 36.1-17.7 54.6l105.7 103-25 145.5c-4.5 26.3 23.2 46 46.4 33.7L288 439.6l130.7 68.7c23.2 12.2 50.9-7.4 46.4-33.7l-25-145.5 105.7-103c19-18.5 8.5-50.8-17.7-54.6L382 150.2 316.7 17.8c-11.7-23.6-45.6-23.9-57.4 0z" transform="translate(-288 -256)"></path></g></g></mask></defs><rect fill="currentColor" clip-path="url(#clip-12)" mask="url(#mask-11)" x="0" y="0" width="100%" height="100%"></rect></svg>

clipPath id keep increasing First <clipPath id="clip-1">

Second <clipPath id="clip-2">

Third <clipPath id="clip-3"> .... Eighth ... Twentieth ...

[robmadole]

Would something like a uuid work better? And when you say memory leak are you concerned with the number exceeding the upper limit of the integer?

[imarjanovic]

I'm running into this issue as well. I don't see the issue as being about a memory leak, so much as about the inevitable disconnect between the server rendered output and the client rendered output, which makes isomorphic/universal rendering fail. A uuid would probably do well to resolve the issue.

[robmadole]

Well the UUID is not going to prevent the mismatch on the server and client. We'd have to have some kind of hashing algorithm that would be idempotent. That's going to be really difficult to do. I'll have to think about this one! PRs are welcome.

[imarjanovic]

Hmm yes, it's a difficult one. We could generate a hash ID using the iconName, mask, and any transform or style props but obviously this leaves open the possibility of having conflicting IDs for users with multiple of the same icon on a page, which introduces further issues.

Perhaps, as an alternative, we could allow users to specify a maskId (or even maskKey, to follow React convention to an extent) prop on the FontAwesomeIcon, defaulting to the nextUniqueId if not set. Most users will never have to worry about this situation and those who do would take on the responsibility to ensure it is unique and idempotent, so it seems like a practical solution to me. Happy to submit a PR for this.

[robmadole]

@imarjanovic I like maskKey. That echoes React's key. I briefly thought about even using the key prop but React says that's unique only within a group of siblings so we could potentially hit a conflict.

So this is going to take some work in the @fortawesome/fontawesome-svg-core library. That hadn't occurred to me when I said "PRs are welcome". The core lib is not currently open source so you'd only be able to do a part of the work here and I don't want to commit our team to this until I can make sure we can get it done in a timely fashion. So please hold off. I'll get this on our work list so it can be prioritized.

[robmadole]

Add support for specifying maskKey (this is an internal tracker, ignore)

[imarjanovic]

Hey Rob! Just wondering if you folks have made any progress here or if you have any sort of timelines for when a fix for this will be available.

[robmadole]

@imarjanovic we have a PR up in our build system to switch this to a unique ID but no work has been done for specifying the mask key using the maskKey prop.

[imarotte]

Has there been any progress on this?

[justlevine]

Any progress on this?

[tarik-besic]

Any progress on this?