[useForkRef] Accept an array of refs

[oliviertassinari]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Summary 💡

Instead of:

https://github.com/mui/material-ui/blob/23697b9be6ded1ed9aba5f5242940ab8a5d4afbd/packages/mui-base/src/ButtonUnstyled/useButton.ts#L162-L165

we could have:

  const handleRef = useForkRef([updateHostElementName, externalRef, focusVisibleRef, buttonRef]);

Examples 🌈

The signature of

• https://github.com/gregberge/react-merge-refs/blob/812fefa4390884a4a2d1bc489d869bd088df8ff5/src/index.tsx#L3
• https://github.com/mantinedev/mantine/blob/d9d3340bb73c5681d74f2994129ce8b69184e3a4/src/mantine-hooks/src/use-merged-ref/use-merged-ref.ts#L6

Motivation 🔦

It's an idea of simplification that I notice while looking at a notification on https://github.com/gregberge/react-merge-refs/issues/5.

[michaldudak]

I've already attempted to do so in #27939.

[oliviertassinari]

@michaldudak It looks like it was a fun conversation. From what I understand, the discussion stopped at

it breaks the rules of hooks

I don't see how this statement is true. For example, the eslint rule says that It can't determine if the use case is valid or not, so its up to you to evaluate the case:

https://github.com/facebook/react/blob/b2748907c3cffb226447417957f2608a7c60c27d/packages/eslint-plugin-react-hooks/tests/ESLintRuleExhaustiveDeps-test.js#L2440-L2444

It seems OK.

[kabernardes]

Hey, can I try this one?!

[kabernardes]

@oliviertassinari, I am looking at this issue and I have a question/suggestion: I used the initial idea of @michaldudak in #27939, which appears to work. But, it will be a little different than what you ask, it would be:

const handleRef = useForkRef(updateHostElementName, externalRef, focusVisibleRef, buttonRef);

When I try to put all the refs in an array, it didn't accept because some can be undefined or null, and typeScript doesn't accept that. Can the solution be like that? I believe it will improve the code the same way you want.

Thanks for your attention!

[michaldudak]

@oliviertassinari if you agree with my points in #27939, we can just merge that one. I think the usage will be even simpler than with an array.

[oliviertassinari]

@michaldudak The first approach you did in #27939 seems correct. From what I understand, the eslint error was about raising the attention that the plugin can't statically determine if it's a correct or not use of the hook API this way.