snapshot in a waitFor doesn't seem to work right in 7.x

[benmonro]

• dom-testing-library version: 10.x
• node version: 12.14
• npm (or yarn) version: 6.13

Relevant code or config

when using waitFor with expect().toMatchSnapshot() I've seen some strange behaviors.

What you did:

test("should match snapshot async", async () => {
  const mockApi = jest.fn();
  render(<App callApi={mockApi} />);
  fireEvent.click(screen.getByText("click me"));

  await waitFor(() => {
    // expect(mockApi).toBeCalled(); //fails without this.  but why?
    expect(mockApi.mock.calls).toMatchSnapshot()
  })

})


test("should show stuff", async () => {
  render(<App />);
  waitFor(() => {

    expect(document.body).toMatchSnapshot();
  });
});

What happened:

Reproduction repository:
https://github.com/testing-library/dom-testing-library-template
https://github.com/benmonro/rtl-wait-for-issue

Problem description:
expect.toMatchSnapshot doesn't seem to work properly in a waitFor. You'll notice i have 2 different usages. one in which the dom doesn't change, it just calls an api and matches on the mock for that api. another where it waits for a snapshot of the dom to match. My examples are a bit contrived, but they do highlight the problem we're seeing. I would of course use getByText to look for 'stuff' but it still makes me wonder why this usage of waitFor isn't working w/ toMatchSnapshot the way I would think it would
Suggested solution:

[kentcdodds]

Just to be clear, you need to use await anytime you use waitFor.

[alexkrolick]

Jest 25 (if you're using it) should be warning about dangling promises if you don't await

[benmonro]

sorry that was just a typo, the issue is still the same when i add await.

[benmonro]

https://github.com/benmonro/rtl-wait-for-issue/blob/master/src/App.test.js#L25

[alexkrolick]

Jest's runner does stuff like run snapshot tests twice after they are changed to detect non-idempotent/deterministic snaps, so maybe that has something to do with it?

[kentcdodds]

Ah, yeah, so we should probably document this, but because of the way all of the wait async things work, you can't use snapshots inside the callback. Jest gives this warning:

Jest: Multiple inline snapshots for the same call are not supported.

I literally just ran into this.

Moving this issue to the docs repo so we can add a note about this.

[alexkrolick]

Inline is different than "regular" though; you'd get the same thing in a test.each for inline but not standard

[kentcdodds]

Oh, fair point. Moving back 😅

[kentcdodds]

I think that this is still not recommended because the point of waitFor is so you can wait a non-deterministic amount of time for something to happen and that means that your callback will be called a non-deterministic number of times which will cause problems with snapshots. Every call to that assertion will result in a snapshot update which is not going to be what you want anyway.

So yeah, don't use snapshots in the async utils.

We should document this. I don't think we can do anything to change it...

[alexkrolick]

  await waitFor(() => {
    // expect(mockApi).toBeCalled(); // fails without this.  but why? 🚩🚩🚩
    expect(mockApi.mock.calls).toMatchSnapshot()
  })

FWIW, it looks like that line with the 🚩 failing shortcircuits the wait by throwing, so it only moves onto the snapshot call one time. Which agrees with what Kent it saying, which is that calling it more than once will error. I think it's because of how the snapshot reporter works in Jest (ie, how it says "N snapshots updated" in the test report, how it runs the test twice when a snapshot is written, and has the interactive prompt to update in watch mode). It might work in some other implementation like a Mocha plugin.

[kentcdodds]

Going to go ahead and close this. If someone wants to do more with docs feel free.