timolins / react-hot-toast

Smoking Hot React Notifications 🔥
https://react-hot-toast.com
MIT License
9.68k stars 323 forks source link

Unable to test with React Testing Library #107

Closed SimeonRolev closed 1 year ago

SimeonRolev commented 3 years ago

Hi, guys, I work on a project where this library fits pretty well, but I want to test my end product. However, I wasn't able to do so, for some reason. I've used React Testing Library for tons of tests, so I guess it's something in the way these toasts work that I don't understand. Does the disappearance of the toasts depend on something that 'jsdom' can't handle?

Made a minimal reproduction repository with some simple explanations here: https://github.com/SimeonRolev/test-react-hot-toast. It has a single test for the "Close button".

Testing example: https://testing-library.com/docs/guide-disappearance

Thanks in advance!

SimeonRolev commented 3 years ago

https://github.com/SimeonRolev/test-react-hot-toast/tree/jest-fake-timers Created a new branch that I try to advance mocked jest timers. I get leaks.

silvenon commented 2 years ago

The problem is in the Toaster component:

https://github.com/timolins/react-hot-toast/blob/c5e59351d511d8702b065378a6859c795b05547d/src/components/toaster.tsx#L84-L88

In jsdom getBoundingClientRect used in createRectRef always returns an object of zeros, so the ref created createRectRef just gets created and run over and over again because t.height is always 0. In browser this would stop immediately because t.height would not be 0 after the first run.

I'm not sure what to do about this, though.

silvenon commented 2 years ago

The solution is basically to memoize the ref created by createRectRef instead of the ternary above, which requires creating an additional component for rendering toast messages. You can see the linked PR, we already had that in Orbit.

Would a PR be welcome? I can send one next week.

timolins commented 2 years ago

Wow, thanks @silvenon for your investigations! A PR would be very welcome.

CodeZea1ot commented 2 years ago

Will silvenon's PR be merged for this project? Would love to use react-hot-toast in RTL 😁

sharmabharti195 commented 2 years ago

@silvenon's PR will be merged?

pipboy3000 commented 1 year ago

I forked the code in the document and added a test. The test shows as successful, but I got an Infinit loop error.

https://codesandbox.io/s/react-hot-toast-usetoaster-headless-example-forked-j2ods8?file=/src/App.test.js

@silvenon and @ahuth code works fine.

silvenon commented 1 year ago

@pipboy3000 yep, the useToaster example has the same problem as the Toaster component, so I think it's enough to edit the condition to be el && typeof toast.height !== "number" instead of el && !toast.height. Note that the infinite loop you're seeing is happening in your test, not in the browser, but the test is falsely passing because you're not awaiting the click action, which is asynchronous. This is the fixed test, I also replaced getBy + waitFor with findBy queries:

test("show Toast", async () => {
  render(<App />);
  const addButton = await screen.findByText("Add Toast");
  await userEvent.click(addButton); // notice the await here!
  await screen.findByText(/Hello World/)
});

The reason why we know this fixes the test is that without the typeof fix the test no longer passes, it fails, as it should. Would you care to submit a PR to update the documentation? And see if there are any other code examples which need to be updated with the typeof fix.

One more thing regarding the test, I'm not entirely sure why the lack of await causes the test to falsely pass, considering that you're waiting for the toast message to appear anyway. I guess it has something to do with how @testing-library/user-event works, or maybe it's precisely the infinite loop that screws it up. 🤷 As for why you're seeing the infinite loop error in the Browser tab, I guess that's how CodeSandbox handles test errors that happen after tests finish running.

Btw, in case the reason for the infinite loop is not clear, it's same as in the Toaster component, in unit tests toast.height ends up being 0 so the condition el && !toast.height remains true, which causes updateHeight to get called infinitely.

silvenon commented 1 year ago

@timolins this can be closed.