React-popper: Maintain children ref

Created on 11 Jul 2018  路  8Comments  路  Source: popperjs/react-popper

Hello :)

I'm writing a simple tooltip that should be able to use any child as a reference component. I don't want to render any extra DOM nodes around my reference components. To do this, I am cloning the children and adding a ref tag.

Simplified example:

<Reference>
    {({ ref }) => React.cloneElement(children, { ref })}
</Reference>

However, it's possible the child already has a ref, and I don't want to override it. So I am doing something like this:

<Reference>
    {({ ref }) => React.cloneElement(children, {
        ref: (node) => {
            ref(node);
            children.ref(node);
        },
    })}
</Reference>

Unfortunately, react-popper doesn't seem to like this. Here is a simplified example reproducing the error:

Reproduction demo

https://codesandbox.io/s/7yv72zmw51

Steps to reproduce the problem

Replace

<Reference>
  {({ ref }) => (
    <div ref={ref} style={referenceStyle}>
      Reference element
    </div>
  )}
</Reference>

with

<Reference>
  {({ ref }) => (
    // -- Change is on the next line --
    <div ref={node => ref(node)} style={referenceStyle}>
      Reference element
    </div>
  )}
</Reference>

What is the expected behavior?

Ref is set and everything continues to work.

What went wrong?

Getting a "Maximum update depth exceeded" error.

Packages versions

[email protected]

Is this indeed an issue or is this the expected behaviour? Please let me know if I can provide any more information. Happy to help 馃憤

Most helpful comment

I too have been bitten by this. In my case, I'm trying to upgrade from 0.9.5 to 1.0.0. In previous versions I was able to use an arrow function, and I agree that using an arrow function for a ref feels very common.

Try to define you combined ref setter outsie of your component

Unfortunately, this doesn't seem to help in my case; I tried both moving it out to a public class field as well as to a free-floating function, but both continue to cause the infinite loop. In my case, I'm trying to combine react-popper's ref with my own, passed in as another prop.

Here's my "ideal" code (currently non-working):

const PopButtonStateless: React.SFC<PopButtonStatelessProps> ({ button, children, isOpen, onClick, setButtonRef, setPopperRef }) => (
  <Manager>
    <Reference>{({ ref: setReferenceRef }) => (
      button({
        popButtonProps: {
          onClick, ref: r => {
            setButtonRef(r);
            setReferenceRef(r); // this causes infinite re-render
          }
        },
      }))}</Reference>
    {isOpen && <PopButtonPopper setPopperRef={setPopperRef}>{children}</PopButtonPopper>}
  </Manager >
);

I'll keep poking around for a solution, but I'm excited to see the progress in #197 which might help nip the problem in the bud.

All 8 comments

The problem I think is that you are defining a new function on every render, so it forces an update that goes into a loop. Try to define you combined ref setter outsie of your component.

Thanks for the quick response. That's ultimately what I ended up doing.

I guess it just seems a bit odd that it behaves this way since using an arrow function for a ref typically doesn't cause a loop :). I imagine it is just a result of the unique behaviour of the Reference component.

I'm open to suggestions to avoid this loop!

I too have been bitten by this. In my case, I'm trying to upgrade from 0.9.5 to 1.0.0. In previous versions I was able to use an arrow function, and I agree that using an arrow function for a ref feels very common.

Try to define you combined ref setter outsie of your component

Unfortunately, this doesn't seem to help in my case; I tried both moving it out to a public class field as well as to a free-floating function, but both continue to cause the infinite loop. In my case, I'm trying to combine react-popper's ref with my own, passed in as another prop.

Here's my "ideal" code (currently non-working):

const PopButtonStateless: React.SFC<PopButtonStatelessProps> ({ button, children, isOpen, onClick, setButtonRef, setPopperRef }) => (
  <Manager>
    <Reference>{({ ref: setReferenceRef }) => (
      button({
        popButtonProps: {
          onClick, ref: r => {
            setButtonRef(r);
            setReferenceRef(r); // this causes infinite re-render
          }
        },
      }))}</Reference>
    {isOpen && <PopButtonPopper setPopperRef={setPopperRef}>{children}</PopButtonPopper>}
  </Manager >
);

I'll keep poking around for a solution, but I'm excited to see the progress in #197 which might help nip the problem in the bud.

I ran into this too, pulling the ref prop value as a class method, or completely outside of it made no difference in my case, same as @shepmaster. Invoking ref as a method with the node seems to be the problem.

My workaround was to wrap another div inside and attach my own ref to that instead.

In my case I call the ref manually in the callback, as I pass it a DOM node which is not in the subtree of the Reference component. This has worked fine in previous versions of the module.

I looked into the code and found out that getReferenceRef always updates the state, regardless of the reference actually changing. Is this intended? Could there be a check if the refs are unequal or would it break anything?

Code Sample:

<Reference>
  {({ ref }) => {
    // Is always called with the same node, but still triggers an
    // infinite update stack
    ref(myNodeFromSomewhereElse);
    return null;
  }}
</Reference>

Hi,
I have faced the same issue too, and seems like there is only one workaround (suggested by @ebemunk) - add one more div and attach ref to it, .
However this solution looks like a HACK, it adds a surplus node in the DOM for each popper.
I have also tried to add reference change check inside Manager.getReferenceRef (suggested by @janizde), however it did not help.

Great comment here: https://github.com/FezVrasta/react-popper/issues/140#issuecomment-397031037.

Define the function to handle your refs externally and memoize it.

Was this page helpful?
0 / 5 - 0 ratings