ink-testing-library icon indicating copy to clipboard operation
ink-testing-library copied to clipboard

`act` equivalent or flush effects on render

Open mAAdhaTTah opened this issue 5 years ago • 13 comments

Testing a component with hooks is a bit of an issue currently. I have this as an example component that renders & ends the application:

export const Cancelled = () => {
  const { exit } = useContext(AppContext);

  useEffect(() => {
    exit();
  }, [exit]);

  return <Box>Cancelled!</Box>;
};

And I'm testing it like this:

it('should render and exit', () => {
    const exit = sinon.spy();
    const { lastFrame, unmount } = render(
        <AppContext.Provider value={{ exit }}>
          <Cancelled />
        </AppContext.Provider>
    );

    expect(lastFrame()).to.matchSnapshot();
    expect(exit.calledOnce).to.equal(true);

    unmount();
});

The problem is the effect doesn't run by the time the render completes. I have to wrap the check around exit.calledOnce in a setTimeout. An act equivalent would allow use to write something like this:

it('should render and exit', () => {
    let lastFrame, unmount;
    const exit = sinon.spy();
    act(() => {
        ({ lastFrame, unmount } = render(
            <AppContext.Provider value={{ exit }}>
              <Cancelled />
            </AppContext.Provider>
        ));
    });

    expect(lastFrame()).to.matchSnapshot();
    expect(exit.calledOnce).to.equal(true);

    unmount();
});

and we'd know the effects would have been flushed by the time we check exit.calledOnce.

Alternatively, this version of render could flush effects immediately, rather than using the scheduler, which uses setTimeout.

mAAdhaTTah avatar Mar 17 '19 22:03 mAAdhaTTah

Good suggestion! I've tried implementing act() already, but for some reason couldn't get it to work as expected. Any help is appreciated here!

vadimdemedes avatar Mar 18 '19 02:03 vadimdemedes

Hi. I am having a similar issue. I have created a <Counter /> that increments with a delay when you press space. https://github.com/mamachanko/ink-playground/tree/master/counter-ts

When I move the subscription to stdin.on into useEffect (see here) it still works, but the tests no longer pass.

Am I right in understanding that my and this issue here are related? My intuition is to wrap the stdin.write with act(() => ...).

@vadimdemedes do you want to share what you have tried so far? Feels like I am out of my depth, but maybe others have valuable input.

mamachanko avatar Mar 20 '19 15:03 mamachanko

I did a little bit of digging into the React codebase. act under the hood relies on ReactDOM.unstable_batchedUpdate. Tracing that back led to this:

https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberScheduler.js#L2486-L2497

I'm not familiar with writing custom reconcilers, but my first thought is this might be exposed for us already, as we're using react-reconciler already. That said, I'd echo @mamachanko and would like to hear from you @vadimdemedes as to what you attempted so far.

mAAdhaTTah avatar Mar 20 '19 19:03 mAAdhaTTah

I tried replicating that act() ponyfill https://github.com/kentcdodds/react-testing-library/blob/master/src/act-compat.js, but had no luck. I'm still kind of confused by how it's implemented in React, will need to dive into it again.

vadimdemedes avatar Mar 21 '19 04:03 vadimdemedes

I don't think I have made any progress here, but just want to share the context I collected. All of which is probably known to the advanced React practitioner. But here it goes anyway. Maybe it's helpful to others who want to look into this.

There's a really concise explanation for why this warning exists, what act() does and why you'd want that by @kentcdodds in one of his videos(link points to minute 16:54).

This (currently open) issue on React is about the act() warning and the conversation goes on to great detail about whats and ifs. @threepointone is working on a remedy PR and this PR seems to go out any day now.

What I haven't wrapped my head around yet is how (or whether at all) these things relate to ink. We are using a custom renderer and DOM and therefor reconciliation is custom too (is that correct?). But if React can give us the warning and figure out that there are possibly unflushed effects, then where are we leaving the React-paved road so that we can't just use act() for ink? This ties back into what @mAAdhaTTah said. act() is part of react-dom/test-utils. So since we use a custom DOM we need a custom act()? Not sure if I even make remotely sense here.

I forked and refactored the ink-testing-library's tests to use functional components and hooks to be able to easily reproduce the warning: https://github.com/mamachanko/ink-testing-library/blob/using-fc-and-hooks-in-test/test.js The problem is that the tests just fail and show no warning. I would have expected it when stdin.write('Hello World') happens. Maybe I missed something very obvious. Anyway, I've got this simple counter demo to consistenly reproduce the warning https://github.com/mamachanko/ink-playground/tree/master/counter-ts.

mamachanko avatar Mar 24 '19 09:03 mamachanko

Thanks for such a detailed explanation, very helpful!

vadimdemedes avatar Mar 31 '19 04:03 vadimdemedes

We are using a custom renderer and DOM and therefor reconciliation is custom too (is that correct?).

We do use a custom renderer and implement custom configuration for reconciliation, that's correct.

So since we use a custom DOM we need a custom act()?

Yeah, seems like we do.

vadimdemedes avatar Mar 31 '19 04:03 vadimdemedes

I'll try to have a look at this later this week, it should be possible for custom renderers. We just released an alpha that implements it for react-dom and react-test-renderer.

threepointone avatar Apr 04 '19 12:04 threepointone

Thanks @threepointone, let us know about your progress! Totally understandable if you won't have time to look into it too, no worries.

vadimdemedes avatar Apr 09 '19 23:04 vadimdemedes

Has there been any movement or explorations around this issue?

I'd be interested in picking this up for implementation in ITL, as I've run into a related issue in something I'm working on. I have a HOC that does something in useEffect. However, it doesn't appear that effects are flushed before the initial render completes. This means that if I call render and then attempt to interact with the result, the effects will not have been run yet, so I can't actually test those behaviors.

I think this is related to this issue because they're both related to batching & flushing pending effects. Like act, the initial render also needs to batch & flush any pending effects before returning.

mAAdhaTTah avatar May 03 '20 16:05 mAAdhaTTah

Sorry I never got around to this, but it’s still doable. Copy this file https://github.com/facebook/react/blob/master/packages/react-dom/src/test-utils/ReactTestUtilsAct.js, and replace the react-dom versions of isThisRendererActing, flushPassiveEffects and batchedUpdates with ink’s versions. Happy to assist on a PR if someone puts it up.

To be clear, this should be only used in tests, and not product code.

threepointone avatar May 03 '20 16:05 threepointone

^has anyone had any luck with implementing this or a workaround?

JaKXz avatar Feb 07 '21 19:02 JaKXz

Anyone got any solution for this?

benwainwright avatar Apr 17 '23 13:04 benwainwright