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

<Icon /> component uses custom logic for getting unique ids, that doesn't produce equal results on server and client (in next.js) #3705

Closed
sebastianvitterso opened this issue Dec 12, 2024 · 0 comments · Fixed by #3706
Labels
🐛 bug Something isn't working

Comments

@sebastianvitterso
Copy link
Contributor

Describe the bug

The Icon component uses custom logic for getting unique ids to match aria-described-by tags to a <title>-tag with an id. This custom id logic doesn't produce equal results on server and client, since it's based on Math.random().

To track down the bug, let's trek through the source code of the Icon component:
Assumptions:

import { external_link } from '@equinor/eds-icons'
// the component usage
<Icon data={external_link} title="Open in new tab"/>

The title tag is rendered on Icon.tsx:150:

{title && <title id={titleId}>{title}</title>}

Let's look for titleId on Icon.tsx:138:

titleId = `${icon.prefix}-${icon.name}-${count}`

Now, count comes from the call to findIcon on Icon.tsx:108:

const { icon, count } = findIcon(name, data, size)

findIcon, when the data prop isn't empty, returns count from customIcon, as we see on Icon.tsx:57:

let { icon, count } = data ? customIcon(data) : get(name)

customIcon simply returns a random number as its count value, which is where the problem comes from:
Icon.tsx:50-53

const customIcon = (icon: IconData) => ({
  icon,
  count: Math.floor(Math.random() * 1000),
})

Proposed solution

Use React's useId hook (or implement it yourselves), as that is stable between server-side and client-side rendering, allowing us to simply do the following in the Icon component:

const id = useId()
const { icon } = findIcon(name, data, size)

// ...

if (title) {
  titleId = `${icon.prefix}-${icon.name}-${id}`
// ...

Now I do realize that you have to support react < 18, so I propose you build a stable useId replacement in that case.

How about testing something like the following (based on your current @equinor/eds-utils hooks/useId.ts):

let counter = 0

export const useId = (idOverride: string, type?: string): string => {
  const [defaultId, setDefaultId] = useState(idOverride)
  const id = idOverride || defaultId

  useEffect(() => {
    if (defaultId == null) {
      setDefaultId(`eds-${type ? type + `-` : ''}${counter++}`)
    }
  }, [defaultId, type])
  return id
}

Steps to reproduce the bug

  1. Build a next.js app
  2. Have an icon in the app, give it an IconData object as the icon prop and a string for the title attribute.
  3. See error

The bug results in a warning/error message from next.js in dev mode:

screenshot

Expected behavior

No error message, as the id's should be equal on server and client.

Specifications

  • Version: "@equinor/eds-core-react": "^0.42.5",
  • Browser: Chrome Version 131.0.6778.109 (Official Build) (arm64)
  • OS: MacOS Sequoia 15.1.1 (24B91)

Additional context

None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant