mergeProps should also merge refs

[AndrewPrifer]

🙋 Feature Request

🤔 Expected Behavior

mergeProps should merge refs, if both prop objects contain them. The react-merge-refs NPM package provides an example of how this can be achieved.

😯 Current Behavior

The current implementation doesn't consider refs.

💁 Possible Solution

Using a function ref, similarly to how react-merge-refs does it.

🔦 Context

An increasingly common pattern, and one I use myself often, is, rather then providing hooks with pre-made refs, letting the hooks create the refs they need and return them. This is nice, since the logic that creates the ref can be included in the hook, and when multiple refs are needed, they can be merged together with react-merge-refs.

Specifically for react-aria, it'd help provide a simpler, and more consistent API for hooks like useButton, since refs could be treated just like any other prop (which is also how upcoming React versions are going to treat them conceptually) and returned from the hook instead of letting the user of the library manage them.

💻 Examples

const ref = useRef(null);
const { buttonProps } = useButton(props, ref);

could become

const { buttonProps } = useButton(props);

and if other hooks need a ref to the same component, with this change, mergeProps could take care of that, just like any other prop conflict, like className:

const { buttonProps } = useButton(props);
const { someOtherButtonProps } = useSomeOtherHook();

const finalProps = mergeProps(buttonProps, someOtherButtonProps);

[ktabors]

This sounds like an architectural suggestion that we can adopt going forward. There is some concern about the complexity of merging class and function refs, so we need to do investigation. Since you have interest in this, do you have additional information or thoughts on how we could accomplish this?

[AndrewPrifer]

@ktabors I don't think there is much more to it than this:

function mergeRefs(refs) {
  return value => {
    refs.forEach(ref => {
      if (typeof ref === 'function') {
        ref(value)
      } else if (ref != null) {
        ref.current = value
      }
    })
  }
}

This implementation covers all currently supported refs, and would scale to any future ref implementations as well, as long as function refs are supported.

[ktabors]

Thanks for the info, we are still investigating.