fix: useId instead of random number for tooltip ID

[mcmcgrath13]

Summary

Related Issues or PRs

Alternate approach to #2562, which does not change the API of the Tooltip component. Use React's useId to generate a unique identifier instead of a random number.

This is the expected way to generate IDs, so plays nicely with server side rendering frameworks and is easy to mock for unit or snapshot tests.

Example of jest config for mocking

// Make sure the auto-generated IDs are stable for snapshot testing
jest.mock("react", () => ({
  ...jest.requireActual("react"),
  useId: () => "r:id",
}));

How To Test

This is a pure bugfix/refactor. It makes no change to API or UI

[mcmcgrath13]

@crwallace the README says you are an active maintainer - would you be able to approve running the workflow for the tests?

[mcmcgrath13]

@brandonlenz I see you committed to the repo recently - any chance you could approve running the test workflows?

[mdmower-csnw]

@brandonlenz @mcmcgrath13 - genId looks like it disobeys two rules of hooks: hooks should be top-level or in custom hooks, and hooks should not be called conditionally.

[mcmcgrath13]

any ideas on how can we get around the conditional here given the polyfill need? We can just rename the helper useGenId for the custom hook part

[mcmcgrath13]

started a follow on PR at #3096

[mcmcgrath13]

I don't see any warnings in the console on the hook rules with this PR as is 🤔

[mdmower-csnw]

@mcmcgrath13 - While this project has plugin eslint-plugin-react-hooks installed, none of its rules are enabled. If you add this to the eslint config, you'll see a linting error in function genId.

diff --git a/.eslintrc b/.eslintrc
index 85f1c95..70d76cf 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -65,6 +65,7 @@
     "plugin:security/recommended-legacy",
     "plugin:storybook/recommended",
     "plugin:vitest/recommended",
+    "plugin:react-hooks/recommended",
     "prettier"
   ],
   "rules": {

And if you change the name of the function from genId to useId, you'll see the other hook issue I mentioned.

I'm not all that familiar with the issue this PR is solving, but maybe the below is a more accurate polyfill for useId?

const useId = () => {
  const id1 = React.useId?.()
  const [id2] = useState(`${Math.floor(Math.random() * 900000) + 100000}`)
  return id1 ?? id2
}

[mcmcgrath13]

aha - thanks for the tip!

[mcmcgrath13]

I had a similar approach on the follow on, but I like that yours allows getting rid of the useRef entirely, so I've adopted that on the follow on PR