answers to https://github.com/alexkrolick/react-testing-examples/blob/master/README.md#questions

answers.md

good doc! some quick answers -

It seems like act() is being recommended for wrapping all state updates in React tests, but is it necessary to use it everywhere if you can use waitForElement to turn the whole test async?

This is a very good question, and something I grappled with earlier. A couple of things that stood out for me -

• waiting for an element is indeed pretty close to what a user's experience is like; ie - a user 'waits' for the form to show itself, after which they fill it in and click a button, then 'wait' for the success screen etc. Ultimately, act() makes this test stronger - it'll ensure that effects, and queued promises, have been flushed before you interact with the element. wrapping waitForElement with act() (the async version, ie), will make this invisible to the user, but with the guarantee that their UI is 'stable'.

• I couldn't assume that all tests would use waitForElement. For example, using timers is common for testing transitions and such. In these scenarios too, act() is useful to ensure a stable UI.

Developers need to manually choose how to group updates to resemble batch patterns the app will actually experience - is there some way to "fuzz" this and somehow trigger more batching in JSDOM automatically? Maybe by removing this maximum frame rate check in test mode?

This is just the nature of unit tests for async/"time travel" based stuff. I don't have a particularly good answer here; the 'ideal' method would be to have a whole new language and infra. And maybe an actual time machine haha. What's interesting is act now provides a way to actually batch these actions and surface issues that would not have been apparent in tests before.

Doesn't batching promise updates gloss over some possible states that may be perceptible to the user? For example, an async call in cDM might be mocked by a Promise that resolves in 1 tick in test world, but 0.5s in real life. Squashing those updates seems wrong if there is no way to inspect the intermediate state.

Yes :D But those are usually explicit (and you'll note you have this same problem with waitForElement btw) It works relatively well for the needs of facebook.com and their many thousand tests. I don't have a better answer without getting too philosophical about the nature of tests and mocks, but like before, I'm happy we now have an actual primitive to batch these things.

Some cases, like Promises that cause state updates, aren't currently wrappable with synchronous act() and still cause a warning - do these cases actually cause bugs if you are using something like waitForElement?

The async version should capture this warning (unless you're willing to reproduce an environment described in react-act-examples, in which case you can use jest's fake timers to 'advance' synchronously). Like I mentioned above, it'll guarantee that the ui is relatively 'stable' at that point.

Is there anything that can be done to make this less obtrusive, either on the the react-testing-library side or through lint rules, async act(), etc.? Prior to this API, there were no React-specific details required in react-testing-library other than render.

There still mostly shouldn't, especially after the async version lands. It would be interesting if there were wrappers for jest's runAllTimers/advanceTimersByTime calls as well, but not many people use it in oss, so that maybe overkill.

good doc! some quick answers -

❤️ thank you @threepointone

cc @kentcdodds

It seems like act() is being recommended for wrapping all state updates in React tests, but is it necessary to use it everywhere if you can use waitForElement to turn the whole test async?

This is a very good question, and something I grappled with earlier. A couple of things that stood out for me - waiting for an element is indeed pretty close to what a user's experience is like; ie - a user 'waits' for the form to show itself, after which they fill it in and click a button, then 'wait' for the success screen etc. Ultimately, act() makes this test stronger - it'll ensure that effects, and queued promises, have been flushed before you interact with the element. wrapping waitForElement with act() (the async version, ie), will make this invisible to the user, but with the guarantee that their UI is 'stable'.

In that case, do we need to wrap the fireEvent calls, or would the waitForElement batch everything appropriately? If that works, I'm thinking some clarity could be achieved by pushing all the async functionality into async-by-default queries: testing-library/dom-testing-library#203 while leaving other parts of the library (events, render) unwrapped. Again, contingent on this achieving the same behavior.

I couldn't assume that all tests would use waitForElement. For example, using timers is common for testing transitions and such. In these scenarios too, act() is useful to ensure a stable UI.

Same question - do we have to wrap the originating event with act or can we wrap the followup-query?

Developers need to manually choose how to group updates to resemble batch patterns the app will actually experience - is there some way to "fuzz" this and somehow trigger more batching in JSDOM automatically? Maybe by removing this maximum frame rate check in test mode?

This is just the nature of unit tests for async/"time travel" based stuff. I don't have a particularly good answer here; the 'ideal' method would be to have a whole new language and infra. And maybe an actual time machine haha. What's interesting is act now provides a way to actually batch these actions and surface issues that would not have been apparent in tests before.

⏳ ok

Doesn't batching promise updates gloss over some possible states that may be perceptible to the user? For example, an async call in cDM might be mocked by a Promise that resolves in 1 tick in test world, but 0.5s in real life. Squashing those updates seems wrong if there is no way to inspect the intermediate state.

Yes :D But those are usually explicit (and you'll note you have this same problem with waitForElement btw) It works relatively well for the needs of facebook.com and their many thousand tests. I don't have a better answer without getting too philosophical about the nature of tests and mocks, but like before, I'm happy we now have an actual primitive to batch these things.

Wrapping render makes this less explicit, right? And you can't opt-out without reimplementing render? Does FB use a similar shared render util?

Seems like a workaround we can recommend to make this more explicit would be adding timers to mocks instead of mocking highly async promises with Promise.resolve() (one tick).

Some cases, like Promises that cause state updates, aren't currently wrappable with synchronous act() and still cause a warning - do these cases actually cause bugs if you are using something like waitForElement?

The async version should capture this warning (unless you're willing to reproduce an environment described in react-act-examples, in which case you can use jest's fake timers to 'advance' synchronously). Like I mentioned above, it'll guarantee that the ui is relatively 'stable' at that point.

Is there anything that can be done to make this less obtrusive, either on the the react-testing-library side or through lint rules, async act(), etc.? Prior to this API, there were no React-specific details required in react-testing-library other than render.

There still mostly shouldn't, especially after the async version lands. It would be interesting if there were wrappers for jest's runAllTimers/advanceTimersByTime calls as well, but not many people use it in oss, so that maybe overkill.

I think timers are going to be important in the pre-suspense-for-data era, in order to differentiate between side-effects that trigger high-latency (noticeable loading state) vs low-latency async requests. We can add more examples using fake timers.

do we need to wrap the fireEvent calls

Technically we should be fine to not wrap those because React runs event handlers in a batch anyway.

leaving other parts of the library (events, render) unwrapped.

If we left render unwrapped then you wouldn't be able to assert on the state of the post-effects world after render. You'd have to run act manually or use another utility which I don't think is what we want. I definitely prefer having render wrapped so you can instantly work with a DOM that has settled.

And you can't opt-out without reimplementing render?

I'm really struggling understanding why you would want to opt-out. I realize there are SSR implications and stuff, but I don't think that's a common scenario that you would need tests for. I could be wrong I guess. I could be convinced to make an opt-out configuration option in react-testing-library's render function, but I don't want to...

If we left render unwrapped then you wouldn't be able to assert on the state of the post-effects world after render. You'd have to run act manually or use another utility which I don't think is what we want. I definitely prefer having render wrapped so you can instantly work with a DOM that has settled.

1. The main issue I see with batching the initial render is when you mock an async call (eg for an API call in cDM) with a simple Promise.resolve(), you drop the intermediate loading state by batching. One way to fix this is to encourage the use of timers in mocks for anything that has real IO/network latency. Also in the Suspense-for-data world, you can mock the cache read instead of the API call.

2. Since batching and concurrent mode change the nature of the UI to be more async in almost every case, I think it might make sense to start encouraging the use of getBy* queries wrapped in waitForElement + act as proposed in testing-library/dom-testing-library#203. The only places I can think of that this would be an issue would be testing for non-existence of elements in the settled/post-batch DOM if they are initially present for some reason. I like this because you are explicitly opting into async checks in a very unobtrusive way and don't have to use fake timers as much. I assume concurrent mode is going to make it more likely that a) the default happy path can be tested primarily async b) edge cases will need special step-by-step testing that we shouldn't make overly difficult. On the other hand, sometimes there are big differences between "waiting for a batch to resolve" vs "waiting for DOM changes within timeout".

At the least if we continue with the batching approach we need way more examples with fake timers, including non-Jest test runners.

(Assuming that is the right approach)