Skip to content

UserEvent.type causes act warning with react-hook-form with mode: 'onChange' #457

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

Closed
DamianPereira opened this issue Sep 30, 2020 · 7 comments

Comments

@DamianPereira
Copy link

DamianPereira commented Sep 30, 2020

  • @testing-library/user-event version: 12.1.6
  • Testing Framework and version: react testing library 16.13.1 with jest 26.1.0
  • DOM Environment: I'm using ts-jest to run the tests, which I think also defaults to jsdom

Relevant code or config
Form:

const SomeForm = () => {
  const { register } = useForm({
    mode: "onChange",
  });

  return <input name="name" ref={register({ required: true })} />;
};

Form spec:

describe("SomeForm input name", () => {
  it("can be changed", async () => {
    render(<SomeForm />);
    const nameInput = screen.getByRole("textbox", "name");
    await userEvent.type(nameInput, "John");
    // await fireEvent.change(nameInput, { target: { value: 'John' } })
    expect(nameInput).toHaveValue('John');
  });
});

What you did:

I tried using react-hook-form with validation mode onChange, which will trigger a validation whenever an input changes.

What happened:

The test works as expected, but an act warning appears. When using fireEvent.change, the warning does not show up. I can wrap the userEvent.type in an act(), but I feel like that hides the problem. I understand correctly the onChange event from the input happens asynchronously so userEvent.type ends before it gets called.

How can I expect validations to have been called? Does userEvent.type call onChange callbacks synchronously? Is there any way to await for all changes triggered by the input change? Maybe adding an extra empty validation function that I can mock and wait for it?

If this issue can't be handled in userEvent maybe I should take it to react-hook-form.

Reproduction repository:

I have an example repository here: https://codesandbox.io/s/usereventtype-issue-example-g0g8h
It is currently failing because of an unrelated codesandbox issue, but you can download the zip, and run npm i and npx jest src/SomeForm.spec.js

@ph-fritsche
Copy link
Member

ph-fritsche commented Oct 1, 2020

Yes, from a short look at the code of react-hook-form:
It attaches event handlers directly on the DOM element which handle events and trigger some rerendering of the component asynchronously.

The lack of warning when using fireEvent directly might be a result of the test being completed before that rerendering causing the warning occurs.

As the promise for the async operations being performed is not returned to you, I don't see a "clean" way to ensure all actions are performed.
You can just try to wait either for some time or - preferably - until some expected change happens.

@DamianPereira
Copy link
Author

You can just try to wait either for some time or - preferably - until some expected change happens.

The problem is that there's nothing I can wait for outside, I think the onChange validation is correctly wrapped in act, but the problem comes when the reference changes (I'm not sure why this happens). The full warning with trace is:

console.error
Warning: An update to SomeForm 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 SomeForm

      at printWarning (node_modules/react-dom/cjs/react-dom.development.js:88:30)
      at error (node_modules/react-dom/cjs/react-dom.development.js:60:5)
      at warnIfNotCurrentlyActingUpdatesInDEV (node_modules/react-dom/cjs/react-dom.development.js:23284:7)
      at dispatchAction (node_modules/react-dom/cjs/react-dom.development.js:15656:9)
      at node_modules/react-hook-form/src/useForm.ts:165:7
      at node_modules/react-hook-form/src/useForm.ts:213:9
      at HTMLInputElement.handleChangeRef.current (node_modules/react-hook-form/src/useForm.ts:574:11)

If you see where it comes from is a function inside react-hook-forms that does handleChangeRef. If I understand correctly userEvent.click or userEvent.type wrap the event listener callbacks in act right? So if a button sets state inside onClick callback, there will be no warning since this was expected. But it seems userEvent.type is not taking this handleChangeRef into account, is there any way to count it?

@ph-fritsche
Copy link
Member

I would not be too worried about hidden sideeffects as everything happening in that act call should eventually happen in the browser too.

But the test will be dependent on how far down the event loop the eventual rerendering is pushed.
If someone refactors some code used by the change handler you might again end up before that crucial call.

If you really want to test that some ref callback is called and you can leverage some forwarded ref with that library, you could try something like:

let resolveChange
const changeIsMade = new Promise((res) => { resolveChange = res })

render(<MyComponent changeRef={resolveChange}/>)

act(async () => {
  userEvent.type(input, 'text')
  await changeIsMade
})

If the library does not allow you to pass down a ref, you could wait for the expected change like:

act(() => {
  userEvent.type(input, 'text')
  waitFor(() => expect(input).toHaveValue('text'))
})

@DamianPereira
Copy link
Author

DamianPereira commented Oct 2, 2020

It seems like in the react-hook-forms code the ref gets an event listener like this, with ref being a useRef that points to the input:

    ref.addEventListener(
      shouldAttachChangeEvent ? EVENTS.CHANGE : EVENTS.INPUT,
      handleChange,
    );

The function in handleChange is the one that is causing the warning. But this does not happen with onChange like in the following code:

const SomeForm = () => {
  const [value, setValue] = useState("");
  return (
    <input
      name="name"
      aria-label="name"
      value={value}
      onChange={(e) => setValue(e.target.value)}
    />
  );
};

So it looks like userEvent is wrapping input onChange in act, but it is not wrapping onChange events added to the input's ref, could that be happening?

@ph-fritsche
Copy link
Member

ph-fritsche commented Oct 2, 2020

The onChange handler you passed as a prop triggers the state/hook change synchronously. That's why the hook change happens inside the act wrapper.
The handleChange triggers the state/hook change asynchronously. It is put at the end of the event loop and processed after the current call with act is finished.

@DamianPereira
Copy link
Author

DamianPereira commented Oct 2, 2020

Makes sense, then there's nothing that userEvent can do right? The only alternative to stop the warning in this case is to wrap it in act.

Since the event triggers asynchronously, and it is internal to react-hook-form, there's no way to test it. Maybe the correct thing would be to add a "onValidationFinished" callback to react-hook-form, that is called after the validation promise is returned.Then I could await a jest mock callback for that.

If you feel the solution lies outside of userEvent, or that wrapping in act is a good enough solution feel free to close the ticket. Thanks a lot for your help!

@IanVS
Copy link

IanVS commented May 27, 2021

I'm getting these warnings as well with react-hook-form, and I'm not quite clear if there's a solution. If I try to wrap my userEvent.type calls in an act, and when I try to waitFor the value to be correct, I get a warning about overlapping act calls.

In my case, I'm using a Controller similar to what is shown here: https://codesandbox.io/s/react-hook-form-parse-and-format-textarea-zbgog?file=/src/index.tsx:666-805

Edit: I was able to solve this by following this pattern:

const myInput = getByRole('textbox');
userEvent.type(myInput, 'value');
await waitFor(() => expect(myInput).toHaveValue('value'));

I had a problem trying to waitFor something that was already true, so if you get act warnings you don't expect, make sure that your expect actually fails before your userEvent.

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

No branches or pull requests

3 participants