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

Virtual Elements inside a function body without Memoization will trigger an infinite loop #391

Open
conoremclaughlin opened this issue Oct 29, 2020 · 5 comments

Comments

@conoremclaughlin
Copy link

Reproduction demo

https://codesandbox.io/s/react-popper-v2x-issue-template-forked-vx46f?file=/src/index.js
Sandbox will never finish loading because of the infinite loop

Steps to reproduce the problem

If you are using a virtual element and place it inside a functional component without the use of useMemo or some other caching mechanism, it causes an infinite loop.

  1. Place virtualReference in the functional component
  2. Mount component

What is the expected behavior?

Two potential solutions:

  1. Update the docs to note that the virtual element needs to be memoized or cached outside the render body to avoid others tripping over this.
  2. If possible, don't continue re-rendering and updating if the positional values are the same or follow the render cycle of the parent component.

What went wrong?

Infinite loop with virtual elements specified inside a functional component body.

Any other comments?

Popper.js and React are great! Bringing them together is extra awesome. Thanks for all the hard work :)

Packages versions

  • Popper.js: 2.5.3
  • react-popper: 2.2.3
@FezVrasta
Copy link
Member

A PR to improve the docs would be great.

@conoremclaughlin
Copy link
Author

I can potentially take this on once I have a working React-idiomatic example. I'm a bit concerned about this line from the React docs:

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

https://reactjs.org/docs/hooks-reference.html

One additional re-calculation pass should be fine, but I haven't dived deep enough into the react-popper code to see what is triggering the infinite update loop.

@FezVrasta
Copy link
Member

Fixing the root cause would be great, but Popper relies on re-renders to work on the most recent DOM properties (stylings, positioning, etc) so it's often required to trigger a re-render before we can compute the new position.

@cmdcolin
Copy link

cmdcolin commented Sep 3, 2021

Here is a working codesandbox with the useMemo with a follow-the-mouse just in case anyone is interested. https://codesandbox.io/s/react-popper-v2-x-issue-template-forked-kyoc9?file=/src/index.js

The thing about relying on the useMemo is interesting but hopefully this should be ok...

@conoremclaughlin
Copy link
Author

As a follow up:

We currently DO use useMemo in production and we haven't had any issues since this was opened almost a year ago now.

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

No branches or pull requests

3 participants