jaredLunde/react-hook

we cant use optional refs in useMergeRef

47vigen opened this issue · 5 comments

47vigen commented

I think it's better we can use optional ref, sometimes you want to get some optional ref from props so you cant use the current useMergeRef for this!

so change the refs argument to optional and use a filter before "for".

47vigen commented

I create a pull request but I set a bad commit message for that so it doesn't link to this issue

pull request

jaredLunde commented

Why should you be able to do this? You can't conditionally use hooks.

47vigen commented

Please note at my pull request that nothing special has changed. I just use a filter for empty refs.

jaredLunde commented

That's not all it does

kkirby commented

@47vigen Was there some use case that you were trying to solve with this? The code in its current form is exactly the same as what you have provided, except yours is more verbose and allows for undefined as a ref.

Breaking it down:

function useMergedRef<T>(...refs: (React.Ref<T> | undefined | null)[]): React.RefCallback<T> {

The addition of | null is not required since React.Ref<T> resolves to RefCallback<T> | RefObject<T> | null.

const _refs = refs.filter(Boolean)

This is unnecessary since the code already checks for null and undefined values:

else if (ref && typeof ref === 'object')

The condition ref && returns true if the value is not null or undefined.

If your use case is that you want to account for undefined values, you can simply update the call of useMergeRef:

const C = React.forwardRef(function C(
  props: {
    innerRef?: React.Ref<HTMLDivElement>;
  },
  ref: React.ForwardedRef<HTMLDivElement>
) {
  const myRef = React.useRef<HTMLDivElement>();
  const merged = useMergedRef(ref, myRef, props.innerRef ?? null);
  return <div ref={merged}>Hello world.</div>;
});
  • ❤️1