ZeeCoder / use-resize-observer

A React hook that allows you to use a ResizeObserver to measure an element's size.
MIT License
651 stars 42 forks source link

The "onResize" callback doesn't work correctly with conditional rendering #37

Closed QmarXX closed 4 years ago

QmarXX commented 4 years ago

I'm using the library with onResize callback because I need to use another reference to DOM element to set width, but I have an issue when the component uses "loading status" so component with associated ref is conditionally rendered.

  return (
    <>
      {displayLoader ? (
        <LoadingBar />
      ) : (
        <Grid ref={measuredBoxRef}>...</Grid>
      )}
    </>
  )

In the library is a dependency for ref in the useEffect hook so when loading state is changed and component is rendered the ResizeObserver doesn't observe this element because the ref is not completely changed (only current value). This problem is described on pages: https://medium.com/@teh_builder/ref-objects-inside-useeffect-hooks-eb7c15198780 https://dev.to/fnky/pitfalls-of-conditional-rendering-and-refs-in-react-29oi

I can't separate the component like it is proposed in second article, so I'm trying to change the library to use useCallback hook.

Perhaps you met with this problem and you have another solution?

I know that issue is specified for my code but ref in dependencies is not recommended practice and even on the React page is an example to measure DOM element with useCallback hook: https://reactjs.org/docs/hooks-faq.html#how-can-i-measure-a-dom-node

ZeeCoder commented 4 years ago

Maybe try something like:

const {width, height} = useResizeObserver({ref: displayLoader ? null : measuredBoxRef});

Then the hook should receive the ref only when it's actually used.

QmarXX commented 4 years ago

After a quick check it seems to work :) Thank you. This is not an intuitive solution but I think it does not have any side effect and does not affect app performance so I will use it.

ZeeCoder commented 4 years ago

Yeah, I might add it to the readme πŸ€”

On Sun, 26 Apr 2020, 21:03 QmarXX, notifications@github.com wrote:

After a quick check it seems to work :) Thank you. This is not an intuitive solution but I think it does not have any side effect and does not affect app performance so I will use it.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ZeeCoder/use-resize-observer/issues/37#issuecomment-619606464, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4CKEX7EPEEXDMHG53TC7TROSAOPANCNFSM4MRHZK3Q .

QmarXX commented 4 years ago

I think it's a good idea. After "onResize" section or in "Troubleshooting" section. I lost all day searching for the cause so if it helps at least one person it will not be in vain πŸ˜ƒ But also think about the recommended solution from React page with useCallback hook πŸ˜‰

ZeeCoder commented 4 years ago

Yeah so the useCallback hook is not even the interesting part there, but rather supporting callback refs, so that the observer hook would receive the new ref as React provides it.

Not sure what it would look like though...

Maybe something like this?

const { width, height, callbackRef } = useResizeObserver<HTMLDivElement>();

return <div ref={callbackRef}></div>

Then whenever a new node is provided, the hook would trigger the same effect it currently does when the ref instance changes.

Could work.

rlgreen91 commented 4 years ago

Well this certainly saved my bacon :) Btw, if you'd be interested in a Typescript version of the debounced hook, just let me know. Figuring out the types of the ref is a bit tricky.

QmarXX commented 4 years ago

@rlgreen91 good for you :)

@ZeeCoder I tried useCallback and in it useEffect from your lib but it is not allowed. Just hook change to callback also doesn't work correctly and I think when hook will be changed another usage methods will stop working.

ZeeCoder commented 4 years ago

@QmarXX I don't quite understand what you mean here.

ZeeCoder commented 4 years ago

Well this certainly saved my bacon :) Btw, if you'd be interested in a Typescript version of the debounced hook, just let me know. Figuring out the types of the ref is a bit tricky.

@rlgreen91 sure, feel free to post it here. I might update the CodeSandbox examples to use TS in time.

ZeeCoder commented 4 years ago

callback refs are now live in the latest release: https://github.com/ZeeCoder/use-resize-observer/releases/tag/v6.2.0-alpha.1

Also see the documentation: https://github.com/ZeeCoder/use-resize-observer#observing-components-mounted-with-a-delay