React-popper: Act warning in tests

[nstepien]

Reproduction demo

import React, { useState } from 'react';
import { render, act } from '@testing-library/react';
import { usePopper } from 'react-popper';

function MyComponent() {
  const [reference, setReference] = useState();
  const [popper, setPopper] = useState();
  const { styles, attributes } = usePopper(reference, popper);

  return (
    <>
      <div ref={setReference} />
      <div ref={setPopper} style={styles.popper} {...attributes.popper} />
    </>
  );
}

test('act warning', () => {
  render(<MyComponent />);
});

test('no act warning with a poor workaround', async () => {
  render(<MyComponent />);
  await act(() => Promise.resolve());
});

Steps to reproduce the problem

1. Run test using usePopper
2. 
   
   
   
   
   

What is the expected behavior?

No act warning, or a way to address it without feeling like a cheap workaround.

What went wrong?

  console.error
    Warning: An update to MyComponent inside a test was not wrapped in act(...).

    When testing, code that causes React state updates should be wrapped into act(...):

    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */

    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
        in MyComponent

Any other comments?

This is probably coming from the use of useEffect (useIsomorphicLayoutEffect) + state update.
https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning

Packages versions

• Popper.js: 2.4.2
• react-popper: 2.2.3

[FezVrasta]

Take a look at the react-popper tests, they work just fine

[amanmahajan7]

It is possible that the underlying issue may have not been fixed. react-popper is using the following ways to fix the warnings

https://github.com/popperjs/react-popper/blob/master/src/Popper.test.js#L55

await waitFor(() => {});

https://github.com/popperjs/react-popper/blob/master/src/usePopper.test.js

await act(async () => {
   await rerender({ referenceElement, popperElement });
});

Some of them are unnecessary as per testing-library best practices
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#passing-an-empty-callback-to-waitfor
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#wrapping-things-in-act-unnecessarily

From what I suspect it is happening because of the following reason
https://github.com/popperjs/react-popper/blob/master/src/usePopper.js#L60

We provide modifiers to the createPopper function and it may be calling fn hence setting the state before the component is destroyed.

https://github.com/popperjs/popper-core/blob/master/src/index.js#L282

I also noticed the warnings are fixed if we manually unmount the component which further validates this theory.

Thank you for looking into this

[AndrewOCC]

I'm also encountering this same issue running tests with the Render Props version of popper – we're on React 16.8.x so we don't have access to async act() to mitigate the issue.

I've included a basic example test below that raises the error:

import React, { useState } from 'react';
import { render } from '@testing-library/react';
import { Manager, Popper, Reference } from 'react-popper';
function BasicPopper () {
  return(
    <Manager>
      <Reference>{({ ref }) => <div ref={ref}>Reference element</div>}</Reference>
        <Popper>
          {({ ref, style }) => (
            <div ref={ref} style={style}>
              foo bar
            </div>
          )}
        </Popper>
    </Manager>
  )
};
test('act warning', () => {
  render(<BasicPopper />);
});

[FezVrasta]

PRs to improve this are welcome

[ismay]

From what I suspect it is happening because of the following reason
https://github.com/popperjs/react-popper/blob/master/src/usePopper.js#L60

We provide modifiers to the createPopper function and it may be calling fn hence setting the state before the component is destroyed.

@amanmahajan7 I suspect that this is the case as well. I'm dealing with a similar issue with a popper that's also supplied with modifiers.

@FezVrasta I think this issue should be reopened. It's not resolved yet and an open issue will catch the attention of potential contributors more than a closed one.