callstack / react-native-testing-library

🦉 Simple and complete React Native testing utilities that encourage good testing practices.
https://callstack.github.io/react-native-testing-library/
MIT License
3.09k stars 273 forks source link

Error thrown - Warning: You called act(async () => ...) without await. #379

Closed jpmonette closed 2 years ago

jpmonette commented 4 years ago

Ask your Question

I have a simple test that seems to generate the right snapshot at the end of execution, but throws me a console.error during the execution.

Here are the steps to get to the expected component state:

  1. Load the component
  2. Wait for the useEffect asynchronous logic to be executed so that the SegmentedControlIOS is exposed (testID = recordTypePicker)
  3. Set the selectedSegmentIndex to "1"
  4. Wait for the component to re-render and all the async logic to be executed
  5. Assert that the rendered component includes newSObjectLayout testID

The test

import * as React from 'react';
import { fireEvent, render, waitFor } from 'react-native-testing-library';
import { NewSObjectContainer } from '../NewSObject';

describe('NewSObjectContainer', () => {
  const setup = () => {
    const route = { params: { sobject: 'Account' } };
    const navigation = { setOptions: jest.fn() };

    const container = render(<NewSObjectContainer route={route} navigation={navigation} />);
    return { container };
  };

  it('should render a NewSObjectContainer - with page layout exposed', async () => {
    const { container } = setup();

    await waitFor(() => expect(container.getByTestId('recordTypePicker')).toBeTruthy());

    const input = container.getByTestId('recordTypePicker');
    fireEvent(input, 'onChange', { nativeEvent: { selectedSegmentIndex: 1 } });

    await waitFor(() => expect(container.getByTestId('newSObjectLayout')).toBeTruthy());

    expect(container.toJSON()).toMatchSnapshot();
  });
});

The console log

./node_modules/.bin/jest src/containers/__tests__/NewSObject.spec.tsx --u
  console.error
    Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);

      at CustomConsole.console.error (node_modules/react-native/Libraries/YellowBox/YellowBox.js:63:9)
      at printWarning (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:120:30)
      at error (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:92:5)
      at node_modules/react-test-renderer/cjs/react-test-renderer.development.js:14953:13
      at tryCallOne (node_modules/promise/lib/core.js:37:12)
      at node_modules/promise/lib/core.js:123:15
      at flush (node_modules/asap/raw.js:50:29)

 PASS  src/containers/__tests__/NewSObject.spec.tsx
  NewSObjectContainer
    ✓ should render a NewSObjectContainer - with page layout exposed (499ms)

 › 1 snapshot written.
Snapshot Summary
 › 1 snapshot written from 1 test suite.

Test Suites: 1 passed, 1 total
Tests:       q passed, q total
Snapshots:   1 written, 1 passed, q total
Time:        4.865s, estimated 8s
Ran all test suites matching /src\/containers\/__tests__\/NewSObject.spec.tsx/i.
mdjastrzebski commented 4 years ago

This particular line looks a bit odd to me:

await waitFor(() => expect(container.getByTestId('recordTypePicker')).toBeTruthy())

I don't thing we support putting jest's expect into waitFor.

Could you try something like:

const element = await findByTestId('record...') // This is essentially waitFor & getByTestId
expect(element).toBeTruthy()

Let me know if you still got the same error.

jpmonette commented 4 years ago

@mdjastrzebski I did update my code and still get the issue. Here's the updated test:

import * as React from 'react';
import { fireEvent, render } from 'react-native-testing-library';
import { NewSObjectContainer } from '../NewSObject';

describe('NewSObjectContainer', () => {
  const setup = () => {
    const route = { params: { sobject: 'Account' } };
    const navigation = { setOptions: jest.fn() };

    const container = render(<NewSObjectContainer route={route} navigation={navigation} />);
    return { container };
  };

  it('should render a NewSObjectContainer - with Record Type picker exposed', async () => {
    const { container } = setup();

    const input = await container.findByTestId('recordTypePicker');
    expect(input).toBeTruthy();

    expect(container.toJSON()).toMatchSnapshot();
  });

  it('should render a NewSObjectContainer - with page layout exposed', async () => {
    const { container } = setup();

    const input = await container.findByTestId('recordTypePicker');
    expect(input).toBeTruthy();

    fireEvent(input, 'onChange', { nativeEvent: { selectedSegmentIndex: 1 } });

    const layout = await container.findByTestId('newSObjectLayout');
    expect(layout).toBeTruthy();

    expect(container.toJSON()).toMatchSnapshot();
  });
});
mdjastrzebski commented 4 years ago

@jpmonette can you provide a repro repo?

jsamr commented 4 years ago

@mdjastrzebski I ran into a similar issue (although, don't know if it is exactly the same problem) and made a mcve here: https://github.com/jsamr/react-native-testing-library-379

pillu00 commented 4 years ago

I also ran into same problem after upgrading to v7.0.1. Also, if you just run the react-navigation example in the repo itself you will see this problem.

mdjastrzebski commented 4 years ago

@jsamr thanks for posting repo, I'll try to reproduce that tomorrow.

mdjastrzebski commented 4 years ago

@jsamr You should wrap the reload() call in act like this:

    act(() => {
      result.instance.reload();
    });

As each state mutation should be wrapped in act. In case of fireEvent this is done for you.

This leaves your code with following warning logged to console

Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);

which is simingly caused by await findByText('load-cycle-1');

pillu00 commented 4 years ago

@mdjastrzebski , I ran reactnavigation example in this repo and still getting the above error. But when i downgrade to v6.0.0, it goes away. Can you please help me what's going wrong ?

DracotMolver commented 4 years ago

I'm facing the same error but my test is different. Before moving from version 2 to 7 my test was the same and I have no errors, but after updating to v7 i've been getting the same error.

I have something really simple like this:

 const { getByText, update } = render(<Component />);

 fireEvent(getByText('idx-1'), 'onPress');

 await act(async () => {
    update(<Component />);
 });

I use act because when I fire the event I have some async logic in my component that will re-render it. If i don't use the last code I've got the next error:

 Warning: An update to ForwardRef(NavigationContainer) 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 */

UPDATE: 11-08-2020 I changed (cleaned) my tests. I don't have to use anymore this awful thing I was doing adding await to all the act functions I was using and It worked fine. BUT, this is not always for all the cases, so you must be careful how is written your async code and then how is written your test... testing this async code. Also, you must be careful if you are using async callings inside the useEffect hook. I was concerned about this but I was doing it wrong in some parts. I'll share with you what I'm doing with my code now, which thanks to the tests and the library, I think is more solid:

I'm adding a variable to know if you component is mounted/unmounted. With this variable validate wether update or not the state of the componente, which cause to re-render the component.

// I prefer to use isMounted. This way I don't have to use a negation to validate if something is  `true` when is `false` :/
useEffect(() => {
  let isMounted = true;

 (async ( ) => {
   const result = await someAsyncMethod();

  // Here i want to update the state of my component
  if (isMounted) {
   setState(result);
  }
 })();

  return () => {
    isMounted = false;
  };
})

My test now it is like this:

it('test', () => {
 const { getByText, update } = render(<Component />);

 fireEvent(getByText('idx-1'), 'onPress');

  act(() => {
    update(<Component />);
  });
});

Hope this might help :)

One last thing! Don't forget to use the done function 😋

renanmav commented 4 years ago

This leaves your code with following warning logged to console

Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);

I’ve noticed that this error/warning happens when I use more than 1 await within a it statement

dnlsilva commented 4 years ago

This leaves your code with following warning logged to console

Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);

I’ve noticed that this error/warning happens when I use more than 1 await within a it statement

Same issue here. I tried all the things I mentioned earlier, but none worked here.

florian-milky commented 4 years ago

Same issue and I'm not doing anything in my test, just rendering and await findBy

mym0404 commented 4 years ago

I suggest use wait-for-expect directly.

hyunjinjeong commented 4 years ago

This leaves your code with following warning logged to console

Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);

I’ve noticed that this error/warning happens when I use more than 1 await within a it statement

I guess this is the issue, too. In my case, those messages disappeared when I replaced findBy queries with getBy and left only one findBy query.

StarryFire commented 4 years ago

Any updates here? This is happening when i use more than 1 await inside a it

renanmav commented 4 years ago

I patched react-test-renderer (as a temporary fix) since this error was failing my CI tests.

if(!argsWithFormat[0].includes(/You called act\(async \(\) => ...\) without await/)) {
  Function.prototype.apply.call(console.error, console, argsWithFormat);
}
kieran-osgood commented 4 years ago

I am getting this without having any awaits or findBy's, this is the simplest example I could reproduce it, the error appears when I get to the second changeText fireEvent.changeText(passwordInput, '12345')

  const { getByPlaceholderText, getByText } = render(<EmailPassword />);

  const emailInput = getByPlaceholderText(/email@example.com/i);
  const passwordInput = getByPlaceholderText(/\*\*\*\*\*\*\*\*/i);
  const loginButton = getByText(/Login/i);

  fireEvent.changeText(emailInput, 'test@gmail.com'); // valid email
  expect(loginButton).toBeDisabled(); // short password so disabled
  fireEvent.changeText(passwordInput, '12345');
  expect(loginButton).toBeDisabled(); // short password so disabled

  fireEvent.changeText(passwordInput, '123456');
  expect(loginButton).toBeEnabled();
});

Not entirely sure what I need to do?

milesj commented 4 years ago

I've dug into this a bit and I believe the issue is race conditions.

Here's the act code and the block where the console log happens: https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberWorkLoop.old.js#L3680 You'll notice this just creates a resolved promise and logs in a then block (2 next ticks).

To ignore the error, the then of the initial render needs to fire here: https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberWorkLoop.old.js#L3701 However, in my testing, this is actually firing after the resolved promise above, resulting in the warnings. Why? Because RN/React rendering is slow? Not really sure, but there's definitely not an easy way to fix it, or no way in general.

ccfz commented 4 years ago

I have had this problem come up in a variety of places and 99% of the time the problem was that I was waiting for synchronous code. That is why for some people the error went away, after they removed an await, it is a strong sign that the test only required one await or that the await is in the wrong place. Something that works quite nicely in these situations is going through the test and remove all awaits and then look at the code that is being tested to see where the async code is actually happening and to use await only once the async code is actually triggered. If the assertion still fails because the render did not finish have a look at the bottom examples, maybe they can help because they wait until something shows up or is finished running.

The following three examples are the ways I tend to use await the most, maybe it helps clean up tests:

  1. Waiting until a assertion is finished, this is great for circumstances where I only care that the final render is correct:
it('renders the send button', async () => {
    const utils = renderConversation();

    await waitFor(() => {
        expect(utils.getByText('Send')).toBeTruthy();
    });
});
  1. Wait for an element to render, this I only really need if I want to call fireEvent on the element. If you wait for an element to render with the below method and await the assertion as well, the error will probably come up. This is a classic case of unnecessary waiting that I describe at the top. Therefore, don't wait for synchronous code.
it('hides the reply', async () => {
    const utils = renderConversationScreen();
    const replyButton = await utils.findByTestId('hideReplyButton'); // findBy... uses waitFor internally i.e. await waitFor(() => utils.getByTestId('hideReplyButton')); workes just as well
    fireEvent(reply, 'Press');

    expect(utils.queryByTestId('hideReplyButton')).toBeNull();
});
  1. FireEvent triggers async code, which should finish before asserting anything
it('queues the conversation messages', async () => {
    const utils = renderConversationScreen();
    const conversation = utils.getByTestId('Conversation');

    const reply = await utils.findByTestId('replyView'); // I wait for the reply to render
    const replyNativeEvent = { layout: { height: 50 } };
    await act(async () => {
        fireEvent(reply, 'Layout', { nativeEvent: replyNativeEvent }); // I wait for all animations to complete
    });

    expect(conversation.props.queueMessages).toBe(true);
});

@KieranO547 though the code is quite simple two follow up question:

  1. When you get that error is the code you posted the only code that is running? To make sure that is the case I would remove all other renderings and tests in the file and only run that one file. The weird thing about the error showing up for you is that the spec you posted seems to contain nothing that could trigger the error.

  2. What does the code look like, which you are testing. Are there timeouts, state changes, animations etc. just the spec code is only one side of the medal and makes it very hard to debug.

kieran-osgood commented 4 years ago

@ccfz Thanks for all the info. So to answer the 2 follow up Q's,

  1. Yeah theres no other code running, these tests are just aiming to assert that my validation is disabling the login button based on the validation, I've implemented the validation via react-hook-forms + yup schema validation, it works just fine from a user perspective and its just a login form so there is definitely no other code running.
  2. Zero timeouts, react-hook-forms handles the form state, no animations. I do get that it wasn't enough info, i wasn't trying to hijack the post with a million lines of code. As an example, I just have a basic "form" with two input elements for username/password, the text-input comes from react-native-elements, which look like this:
      <Controller
        control={control}
        name="password"
        defaultValue=""
        render={(props) => (
          <Input
            {...props}
            onTouchStart={() => trigger('password')}
            onFocus={() => setTouched(touched + 1)}
            style={input}
            placeholder="********"
            leftIcon={
              <Icon name="md-key" size={20} color="black" style={inputIcon} />
            }
            errorStyle={inputError}
            errorMessage={errors.password?.message}
            label="Password"
            secureTextEntry={true}
            onChangeText={(text) => {
              props.onChange(text);
              trigger('password');
            }}
          />
        )}
      />

The touched state is only used for the disabled state of the login button which is exactly what I'm testing for.

Edit: I just refactored the test to be:

test.only('should disable button when password length is short', async () => {
  const { getByPlaceholderText, getByText } = render(<EmailPassword />);
  const emailInput = getByPlaceholderText(/email@example.com/i);
  const passwordInput = getByPlaceholderText(/\*\*\*\*\*\*\*\*/i);
  const loginButton = getByText(/Login/i);

  await act(async () => {
    fireEvent.changeText(emailInput, 'test@gmail.com'); // valid email
  });
  expect(loginButton).toBeDisabled(); // short password so disabled

  await act(async () => {
    fireEvent.changeText(passwordInput, '12345');
  });
  expect(loginButton).toBeDisabled(); // short password so disabled

  await act(async () => {
    fireEvent.changeText(passwordInput, '123456');
  });
  expect(loginButton).toBeEnabled();
});

Matching your third suggestion and thats got rid of the messages for me so i suppose thats what i was missing, the error message handler is part of the component library - still getting used to testing lol - And thank you again!

ryanhobsonsmith commented 4 years ago

I'm also getting this console error logged when I call findByText more than once. I'm trying to understand if this is an actual bug as @renanmav and @milesj seem to suggest or if I'm just not understanding something about how to use react testing library properly.

A simple (albeit contrived) example I've come up with to reproduce the bug is:

function Example() {
  const [value, setValue] = useState(0);
  const asyncIncrement = useCallback(() => {
    setTimeout(() => setValue((v) => v + 1), 0);
  }, []);

  return (
    <View>
      <Text>Value is {value}</Text>
      <Button title="Increment" onPress={asyncIncrement} />
    </View>
  );
};

test("it works", async () => {
  const { findByText, getByRole, getByText } = render(<Example />);
  getByText(/Value is 0/i);
  const button = getByRole("button");

  fireEvent.press(button);
  await findByText(/Value is 1/i);

  //If I comment out these lines and no error message will be logged
  fireEvent.press(button);
  await findByText(/Value is 2/i);

  // If I uncomment these lines and two error messages will be logged
  // fireEvent.press(button);
  // await findByText(/Value is 3/i);
});

The test seems to work fine (passes as written, fails if I change expected values), but I still see the console.error message warning complaining that "You called act(async () => ...) without await"

I get this console error once per additional time (beyond the first) that I make calls to fireEvent.press(button) and findByText in the same test. I've tried suggestions by @ccfz to wrap the fireEvent() calls with await act(async ...), but that didn't help (not sure I understand how that would have helped either? Doesn't fireEvent already wrap things in act()?)

Could someone help me understand what is going wrong here?

milesj commented 4 years ago

@ryanhobsonsmith By default fireEvent wraps in act and the handler being fired must be synchronous, but because of setTimeout, that onPress is technically asynchronous. So in your example above, the setValue is happening after act has ran.

Maybe try:

await waitFor(() => fireEvent.press(button));
ryanhobsonsmith commented 4 years ago

Thanks @milesj, your notes about the act handler make sense. Unfortunately, your proposed solution didn't work.

My understanding is that waitFor just periodically executes the interior function until it returns a value and, as you said, the fireEvent.press() looks like it returns synchronously (regardless of sync/async nature of the event handler) right?

Just in case fireEvent.press() was actually passing back the result of the event handler somehow, I switched it around so handler would actually return something to await on:

const asyncIncrement = useCallback(() => {
    return new Promise((resolve) => {
      setTimeout(() => {
        setValue((v) => v + 1);
        resolve();
      }, 0);
    });
  }, []);

Feels unnecessary, but with or without that above event handler change I'm still seeing that same error.

Again, reason I'm leaning towards this being a bug is that the test actually seems to pass and work as expected outside of this console error message. I.e. subsequent findByText calls properly wait for the value to appear like I'd expect, just getting the console spam that makes me feel like I must be doing something wrong or may have a subtle bug somewhere.

ryanhobsonsmith commented 4 years ago

Ok, Miles' solution was too far off actually. In case anyone else finds this and wants a quick workaround inspired by: https://github.com/facebook/react/issues/15416, then the best hack I've got work around things is to just build in some manual wait time inside of an act() call like so:

function wait(ms) {
  return act(() => new Promise((r) => setTimeout(r, ms)));
}

function fireAsyncPress(element, ms = 1) {
  fireEvent.press(element);
  return wait(ms);
}

test("it works", async () => {
  const { getByRole, getByText } = render(<Example />);
  getByText(/Value is 0/i);
  const button = getByRole("button");

  await fireAsyncPress(button);
  getByText(/Value is 1/i);

  await fireAsyncPress(button);
  getByText(/Value is 2/i);
});

Would be happy to learn if anyone has worked out a better practice for testing these kinds of async event handlers / state-changes though.

milesj commented 4 years ago

Yeah, I actually do something similar, like this.

async function waitForEvent(cb) {
  await waitFor(async () => {
    await cb();
    await new Promise(resolve => process.nextTick(resolve));
  });
}

await waitForEvent(() => fireEvent.press());
ccfz commented 4 years ago

@ryanhobsonsmith I played around with your examples and I would agree that there is a bug. waitFor seems to not play nice with timeouts, I had a similar issue with a Animated callback. As the findBy queries use waitFor on the inside I think that it is related to waitFor.

My workaround is:

fireEvent.press(button);
await act(wait);
expect(...)

@ryanhobsonsmith could you maybe create a PR with your example from above

function Example() { const [value, setValue] = useState(0); const asyncIncrement = useCallback(() => { setTimeout(() => setValue((v) => v + 1), 0); }, []); ...

because that should have definitely worked and the issue does not actually have a PR yet.

ccfz commented 4 years ago

@thymikee here is a repo with the example from @ryanhobsonsmith. It would be great if someone had a look.

sbalay commented 4 years ago

I have been playing a little bit with the repo that @ccfz created.

I changed the findBy queries for waitFor + getBy queries, still got warnings. Then, in this commit I extracted the implementation of waitFor from @testing-library/react-native (in the node_modules folder) and declared the function locally (myWaitFor). When I use that function the test still passes and there are no warnings. Finally, if I use the imported waitFor instead of its local version, warnings appear again (commit)

This is definitely not a proposed solution. I'm just sharing because I find it very weird and hope that it somehow helps the person that will debug this issue.

mdjastrzebski commented 4 years ago

@sbalay seems like a splendid idea! From what you describe this seems like a pontetial solution to the act issue.

Would you be able to submit a PR for that change so we could iron out the implementation details and make sure it passes our internal tests correctly?

milesj commented 4 years ago

@sbalay Is there duplicate react, react-test-renderer, or testing-library dependencies?

sbalay commented 4 years ago

@mdjastrzebski I don't see how we can extract a solution from what I posted above. When I extracted the waitFor implementation from the testing-library dependency and declared it locally in the example project, then the warnings disappear when I use that local function. That's good but I'm having a really hard time trying to understand why the warnings stopped 😅

I think I didn't explain myself very well, sorry for the confusion.

sbalay commented 4 years ago

@sbalay Is there duplicate react, react-test-renderer, or testing-library dependencies?

@milesj Hmm, no. What do you mean exactly? not sure how you can duplicate a dependency

milesj commented 4 years ago

@sbalay It's possible to have duplicate transitive dependencies. Like in Yarn for example, you may have a lock file that has an entry like the following (Yarn sometimes doesn't dedupe correctly).

react-test-renderer@^16.10.1:
  version "16.13.1"
  resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.13.1.tgz#de25ea358d9012606de51e012d9742e7f0deabc1"
  integrity sha512-Sn2VRyOK2YJJldOqoh8Tn/lWQ+ZiKhyZTPtaO0Q6yNj+QDbmRkVFap6pZPy3YQk8DScRDfyqm/KxKYP9gCMRiQ==
  dependencies:
    object-assign "^4.1.1"
    prop-types "^15.6.2"
    react-is "^16.8.6"
    scheduler "^0.19.1"

react-test-renderer@^16.13.1:
  version "16.13.1"
  resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.13.1.tgz#de25ea358d9012606de51e012d9742e7f0deabc1"
  integrity sha512-Sn2VRyOK2YJJldOqoh8Tn/lWQ+ZiKhyZTPtaO0Q6yNj+QDbmRkVFap6pZPy3YQk8DScRDfyqm/KxKYP9gCMRiQ==
  dependencies:
    object-assign "^4.1.1"
    prop-types "^15.6.2"
    react-is "^16.8.6"
    scheduler "^0.19.1"
sbalay commented 4 years ago

Found it! Same issue as https://github.com/facebook/react-native/issues/29303 or https://github.com/facebook/jest/issues/10221 (and also interfering in https://github.com/callstack/react-native-testing-library/pull/568)

Not sure how and not sure why, but the fact that the react-native preset for jest overrides the Promise object is part of the cause behind this warning.

When I remove that line from the preset (in my node_modules folder) the warning disappears.


@milesj thanks for the clarification! Not happening in this case.

milesj commented 4 years ago

Nice find!

sbalay commented 4 years ago

I've found a way of hijacking the react-native jest preset, to avoid replacing the global Promise object with PromiseJs. Here is how it's implemented

@mdjastrzebski now this could be a potential solution until https://github.com/facebook/react-native/issues/29303 is dealt with by the RN team. @testing-library/react-native could provide it's own jest preset as a wrapper of the original react-native preset in a similar way than my example before. This preset can be optional as this issue does not affect everyone.

We used to have a preset and it was removed, so I'm sure there are some arguments against doing it again. However, this solves this issue and issues in #568 as well.

rmathias86 commented 4 years ago

After upgrading from 5.x to 7.x, I started to get randomly this following error. Sometimes all tests pass, sometimes not. This is really random.

Unable to find node on an unmounted component.

  at node_modules/@testing-library/react-native/build/helpers/findByAPI.js:16:115

I suspect this is related to the same problem.

milesj commented 4 years ago

I tried replacing promise with the native Promise and it completely broke half of our tests. We even tried using native async/await instead of regenerator-runtime and it had the same problem. I'd wager a lot of RN APIs expect the timings provided by promise to work correctly, which is rather unfortunate.

chj-damon commented 4 years ago

so what is the solution eventually?

heisenbugged commented 4 years ago

Having the same issue as @rmathias86

brunnohofmann commented 4 years ago

Having the same problem here!!!

After upgrading from 5.x to 7.x, I started to get randomly this following error. Sometimes all tests pass, sometimes not. This is really random.

Unable to find node on an unmounted component.

  at node_modules/@testing-library/react-native/build/helpers/findByAPI.js:16:115

I suspect this is related to the same problem.

heisenbugged commented 4 years ago

@brunnohofmann @rmathias86 In my case, I found that the problem was caused by the following line:

expect(findByText('An error message')).toBeTruthy();

Since version 6, findBy internally uses async/await and polls until it finds the element in question. I found that wrapping this in an expect (where this is the last line in my test), it creates a race condition between the findBy polling and the end of the test. Since findBy has a 4.5 second timer before it errors out, the "unmounted" error would appear 4.5 seconds after it is triggered and appear to be tied to a random test, even though it wasn't actually caused by a random test.

Replacing the offending line with this:

expect(queryByText('An error message')).not.toBeNull();

Fixed my problem.

jakub-drozdek-mox commented 4 years ago

In my case, it happens when using await findByX at least two times in the same test, so when callbacks do something asynchronously and I need to wait for an element to appear:

const component = render(<MyComponent />)

const button = await component.findByText("First button")
fireEvent.press(button)

const submit = await component.findByText("Submit")
fireEvent.press(submit)

expect(someCallback).toBeCalled()

I still have no idea why does it complain in such case, though...

rmathias86 commented 4 years ago

@brunnohofmann @rmathias86 In my case, I found that the problem was caused by the following line:

expect(findByText('An error message')).toBeTruthy();

Since version 6, findBy internally uses async/await and polls until it finds the element in question. I found that wrapping this in an expect (where this is the last line in my test), it creates a race condition between the findBy polling and the end of the test. Since findBy has a 4.5 second timer before it errors out, the "unmounted" error would appear 4.5 seconds after it is triggered and appear to be tied to a random test, even though it wasn't actually caused by a random test.

Replacing the offending line with this:

expect(queryByText('An error message')).not.toBeNull();

Fixed my problem.

Great find!

There were 3 tests doing expect(findBy****). I extracted the findBy out of expect function, and put await before that. Solved my problem.

const element = await findBy****
expect(element).****
mehmetnyarar commented 3 years ago

I've asked this StackOverflow question.

tegandbiscuits commented 3 years ago

I'm also getting this error when using await multiple times.

Something I noticed though is that if I change the test from async/await to normal then with done then the warning goes away.

Test that gets a warning:

describe(ShoppingHeader, () => {
  let fixture: RenderAPI;

  beforeEach(() => {
    const Stack = createStackNavigator();
    const TestRoot = () => <Text>Hello</Text>;
    const TestCart = () => <Text>The Cart Screen</Text>;

    fixture = render(
      <NavigationContainer>
        <Stack.Navigator screenOptions={{ header: ShoppingHeader }}>
          <Stack.Screen name="Test Root" component={TestRoot} />
          <Stack.Screen name="Cart" component={TestCart} />
        </Stack.Navigator>
      </NavigationContainer>,
    );
  });

  afterEach(cleanup);

  describe('back button', () => {
    beforeEach(async () => {
      const cartButton = await fixture.findByA11yLabel('Open Cart');
      fireEvent.press(cartButton);
    });

    it('navigates back when tapped', async () => {
      const backButton = await fixture.findByA11yLabel('Back');
      fireEvent.press(backButton);

      await waitForElementToBeRemoved(() => fixture.getByText('The Cart Screen'));
      expect(fixture.getByText('Test Root')).toBeTruthy();
      expect(() => fixture.getByA11yLabel('Back')).toThrow();
    });
  });
});

Without using async/await:

it('navigates back when tapped', (done) => {
  fixture.findByA11yLabel('Back').then((backButton) => {
    fireEvent.press(backButton);
    return waitForElementToBeRemoved(() => fixture.getByText('The Cart Screen'));
  }).then(() => {
    expect(fixture.getByText('Test Root')).toBeTruthy();
    expect(() => fixture.getByA11yLabel('Back')).toThrow();
    done();
  });
});
The component if it's useful ```tsx const ShoppingHeader = (props: StackHeaderProps) => { const { scene, previous, navigation } = props; const { options } = scene.descriptor; let title: any; if (options.headerTitle) { title = options.headerTitle; } else if (options.title) { title = options.title; } else { title = scene.route.name; } return ( {previous && } navigation.navigate('Cart')} /> ); }; ```
MarceloPrado commented 3 years ago

Found it! Same issue as facebook/react-native#29303 or facebook/jest#10221 (and also interfering in #568)

Not sure how and not sure why, but the fact that the react-native preset for jest overrides the Promise object is part of the cause behind this warning.

When I remove that line from the preset (in my node_modules folder) the warning disappears.

@milesj thanks for the clarification! Not happening in this case.

I can confirm that removing line 24 from node-modules/react-native/jest/setup.js clears the warning.

Not sure if there's anything we can do besides patching it. I tried all proposed solutions and this was the only one that worked.

tom-sherman commented 3 years ago

After upgrading from 5.x to 7.x, I started to get randomly this following error. Sometimes all tests pass, sometimes not. This is really random.

Unable to find node on an unmounted component.

  at node_modules/@testing-library/react-native/build/helpers/findByAPI.js:16:115

I suspect this is related to the same problem.

@rmathias86 I'm getting this error too, tests failing intermittently on CI. For us, the tests that fail are pretty random but none of them are using async/await or the findBy/waitFor API. What makes you think it's related? Did you ever find a fix?

Btw, here's the full stack trace, it's worryingly small like most of it is missing

Unable to find node on an unmounted component.

  at node_modules/@testing-library/react-native/build/helpers/findByAPI.js:16:115
  at Timeout.runExpectation [as _onTimeout] (node_modules/@testing-library/react-native/build/waitFor.js:48:24)
erlanggatjhie commented 3 years ago

Hello all!,

I followed @MarceloPrado's suggestion which is to comment out this line in react-native package: https://github.com/facebook/react-native/blob/master/jest/setup.js#L24. It does clear out the warnings however it breaks a few tests in our codebase and I couldn't figure out on how to fix the tests.

I tried a different approach which is to update the waitFor function in the compiled js file here: https://github.com/callstack/react-native-testing-library/blob/master/src/waitFor.js#L57-L74 to be:

async function waitFor(expectation, options) {
  if (!checkReactVersionAtLeast(16, 9)) {
    return waitForInternal(expectation, options);
  }

  let result;

  //$FlowFixMe: `act` has incorrect flow typing
  const actResult = (0, _act.default)(async () => {
    result = await waitForInternal(expectation, options);
  });

  await new Promise(actResult.then)

  //$FlowFixMe: either we have result or `waitFor` threw error
  return result
}

It seems to remove the warnings in our codebase (although we need to fix a couple of tests due to not handling the async operation properly). The thing that I'm unsure about is whether this will create another problems that I'm not aware of.

marcos8896 commented 3 years ago

I tried to do the suggested steps from @sbalay in this comment: https://github.com/callstack/react-native-testing-library/issues/379#issuecomment-714341282, but I was not able to make it run as expected in my setup.

What I did is something similar, I installed a Promise pollyfil with npm install --save-dev promise-polyfill. After that I added this into my jest.setup.ts file (I guess it will work for .js files too):

import Promise from 'promise-polyfill';
global.Promise = Promise;

Before the workaround: image

After the workaround: image

Looks like a good workaround for now.