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

we cant use optional refs in useMergeRef #190

Open
47vigen opened this issue Apr 3, 2022 · 5 comments
Open

we cant use optional refs in useMergeRef #190

47vigen opened this issue Apr 3, 2022 · 5 comments

Comments

@47vigen
Copy link

47vigen commented Apr 3, 2022

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
Copy link
Author

47vigen commented Apr 3, 2022

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
Copy link
Owner

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

@47vigen
Copy link
Author

47vigen commented Apr 6, 2022

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

@jaredLunde
Copy link
Owner

That's not all it does

@kkirby
Copy link

kkirby commented May 11, 2022

@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>;
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants