[1.15.1] new act implementation is causing actual tests to fail

[idrissakhi]

Describe the bug

When updating to new version, all tests having act will fail

Expected behavior

update documentation with new way of using act or declare it as breaking change

Steps to Reproduce

import { PropsWithChildren, ReactElement } from 'react';

import { NavigationContext } from '@react-navigation/native';

import { act } from 'react-test-renderer';

import { fireEvent, navContext, render } from 'test-utils';

import { CloseButton } from 'src/Features/Authentication/EmailSent/SubComponents/CloseButton/CloseButton';
import { routeNames } from 'src/Shared/Constants';

const TestComponent = ({ children }: PropsWithChildren): ReactElement => (
<NavigationContext.Provider value={navContext}>{children}</NavigationContext.Provider>
);

describe('CloseButton component', () => {
it('on press navigates to login screen', () => {
const { getByTestId } = render(, { wrapper: TestComponent });
const button = getByTestId('CloseButton');

act(() => {
  fireEvent.press(button);
});

expect(navContext.navigate).toHaveBeenCalledTimes(1);
expect(navContext.navigate).toHaveBeenLastCalledWith(routeNames.Login);

});
});

report

● CloseButton component › on press navigates to login screen

Trying to detect host component names triggered the following error:

Can't access .root on unmounted test renderer

There seems to be an issue with your configuration that prevents React Native Testing Library from working correctly.
Please check if you are using compatible versions of React Native and React Native Testing Library.

  20 |
  21 |     act(() => {
> 22 |       fireEvent.press(button);
     |                 ^
  23 |     });
  24 |
  25 |     expect(navContext.navigate).toHaveBeenCalledTimes(1);

Screenshots

Versions

"@testing-library/react-native": "11.5.1",

[MuhmdRaouf]

I have the same issue (+1)

[mdjastrzebski]

@idrissakhi, @MuhmdRaouf thanks for raporting this. We will look into that.

BTW could you try fireEvent without wrapping it into act? fireEvent is already automatically wrapping it's operation with act.

@pierrezimmermannbam looking at the reported error it is related to detect host components PR.

[pierrezimmermannbam]

@idrissakhi I am unable to reproduce the issue, what version of react-test-renderer are you using? From which version of react-native-testing-library did you upgrade?

[mdjastrzebski]

There was a similar issue in the legacy Native Testing Library: testing-library/native-testing-library#91

It seems like calling render inside act might be producing this error.

[mdjastrzebski]

Following test, when added to RNTL tests, is able to reproduce this error:

test('Repo', () => {
  render(<View testID="view" />);
  const view = screen.getByTestId('view');
  act(() => {
    fireEvent.press(view);
  });

  expect(1).toBe(1);
});

The root cause here is that fireEvent.press internally needsTextInput name, so calls getHostComponentNames, which triggers TestRenderer.create inside explicit act.

Removing explicit act or triggering any other code that would cache host component names prevents the error.

So following test will pass:

test('Repo', () => {
  render(<View testID="view" />);
  getHostComponentNames();
  const view = screen.getByTestId('view');
  act(() => {
    fireEvent.press(view);
  });

  expect(1).toBe(1);
});

@pierrezimmermannbam maybe we should proactively call getHostComponentNames on render, to trigger filling internalConfig value, wdyt?

[mdjastrzebski]

@idrissakhi @MuhmdRaouf: there should be no need to wrap fireEvent with act, as fireEvent internally uses act. I was able to avoid this error by removing explicit act from the test code.

What is the reason you are using explicit act around fireEvent? Does your code works correctly when you remove explicit act around fireEvent?

[pierrezimmermannbam]

@mdjastrzebski this could be a solution but it would fix an issue that I'm not sure we want to fix because we'd want user not to use act in the first place. The ideal solution for me would be to detect the usage of act when it's not supposed to be used in order to warn the user that he shouldn't use it but I'm not sure if it's doable. Also I'd like to understand why using act in such scenario results in this error.

@idrissakhi @MuhmdRaouf I think for now the best solution to fix your issues would be to use the eslint rule no-unncessary-act from eslint-plugin-testing-library

There are still a lot of users that don't use this plugin yet and maybe we should think of ways to promote it better

[mdjastrzebski]

@pierrezimmermannbam spreading good practices is always a good idea, however I think we should do it by some console warnings with helpful message rather than the current cryptic uncaught exception ;-)

I think we should apply the fix to make the explicit act code to continue work correctly, as it is not a bug to wrap fireEvent in act, just a suboptimal practice.

In a separate PR we could add warning for unnecessary act wrappers and/or update docs to mention it.

[mdjastrzebski]

Also I'd like to understand why using act in such scenario results in this error.

@pierrezimmermannbam This seems to come from Test Renderer package itself. See previously linked old issue from Native Testing Library.

[pierrezimmermannbam]

I think I understand better what is happening. Components are not mounted until we exit act so when we try to read root within an act call after rendering it will fail, if we wrap RNTL's render with act we get the same error so this is probably the intended behavior for act because we shouldn't be able to interact with the root before exiting the act call.

What's unclear to me is why act was designed to support nesting, maybe we should ask what purpose it serves because if it threw an error then we wouldn't have this issue and it would prevent users from wrapping RNTL's utils with act calls

@mdjastrzebski In the meantime I think the solution you proposed is good, I opened a pr to fix this #1304

[mdjastrzebski]

Published v11.5.2 which fixes the issue. Nevertheless, we recommend to remove explicit act wrappers around fireEvent and other RNTL functions.

[idrissakhi]

@mdjastrzebski agree, but in the same time we should not block without declaring a breaking change. Thanks for the new version