maslianok / react-resize-detector

A Cross-Browser, Event-based, Element Resize Detection for React
http://maslianok.github.io/react-resize-detector/
MIT License
1.25k stars 91 forks source link

Allow passing in 'ref' to the useResizeDetector hook #120

Closed niieani closed 3 years ago

niieani commented 3 years ago

Hi @maslianok!

I'm faced with a conundrum: I need to access the ref inside the onResize callback. But... I don't have the ref yet when I'm defining the onResize, because it is the hook creates it 😕.

const useRegistry = (blockId: string) => {
  const onResize = useCallback(() => {
    resizeObservable.next({blockId, ref})
//                                  ^^^ oops, no ref yet!
  }, [blockId])
  const {ref} = useResizeDetector({onResize})
//       ^^^ too late
  return {ref}
}

A solution would be to allow passing in your own ref instead of creating it inside the hook. This way the ref could also be reused by other custom hooks that are used before useResizeDetector is called, like so:

const useRegistry = (blockId: string) => {
  const ref = useRef()
  const onResize = useCallback(() => {
    resizeObservable.next({blockId, ref})
  }, [blockId, ref])
  useResizeDetector({onResize, ref})
  return {ref}
}

I'm willing to make a PR if this solution is fine by you. Thanks 🙇.

maslianok commented 3 years ago

Yes, I think this is a great idea 👍

Some random thoughts:

Will you work on it? Let me know if you need any additional help

maslianok commented 3 years ago

Added in v6.3.0

import { useResizeDetector } from 'react-resize-detector';

const CustomComponent = () => {
  const targetRef = userRef();
  const { width, height } = useResizeDetector({ targetRef });
  return <div ref={targetRef}>{`${width}x${height}`}</div>;
};
niieani commented 3 years ago

Oops, sorry I didn't have the time to get back to you! Thanks for the quick add @maslianok! 🙇

niieani commented 3 years ago

Looking at the change here 👀: https://github.com/maslianok/react-resize-detector/blob/662238b7f488542f0fd0662180db7fbe34178cbd/src/useResizeDetector.ts#L29

This might potentially cause issues, since it's a conditional use of the useRef hook. Might be safer to always allocate the ref in the hook, and simply not use it when passed in.

maslianok commented 3 years ago

Should we rewrite it to

const ref = useRef(null);

if (targetRef) {
  ref.current = targetRef.current;
}

?

niieani commented 3 years ago

Interesting, I haven't thought of syncing them. Perhaps it might be simpler/safer to do this instead:

const localRef = useRef(null);
const ref = targetRef ?? localRef;

The localRef will always be allocated, but if targetRef is passed in, it will be used instead.

maslianok commented 3 years ago

Sold :)

tbh, I don't see any difference between these approaches. Just 2 lines instead of the 1-line condition.

Can you make a PR?

maslianok commented 3 years ago

Thanks, merged!

I'll deploy changes in the next version