chore(deps): update testing library user events to v14 #1419
Conversation
| await user.type(combobox, '{arrowup}'); | ||
| await user.type(combobox, '{enter}'); | ||
| await user.type(input, '{arrowup}'); | ||
| await user.type(input, '{enter}'); |
There was a problem hiding this comment.
This test case has been questionable. I was only able to get it passing when targeting the input in the Combobox and the only test that has been "altered". When I tested manually, this behavior didn't behave as expected - nothing happened. Is there anything historical to note with this test case?
There was a problem hiding this comment.
🤔 I wonder if the combobox is somehow no longer considered to be the recipient of the keystrokes. I don't see a problem with the updated code.
| for (const _ of new Array(5).fill(null)) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await user.click(getByRole('button')); | ||
| } |
There was a problem hiding this comment.
The for of loop was favorable here to keep the async calls that was originally updated like so:
await Promise.all(new Array(5).fill(null).map(() => user.click(getByRole('button'))));
as it was throwing a warning on what appeared to be stacking act calls being done internally by the user.click API.
This was a for loop before the need to be async.
There was a problem hiding this comment.
Can this be refactored as an internal async function as shown by https://eslint.org/docs/latest/rules/no-await-in-loop?
There was a problem hiding this comment.
Having a classic for loop as it's shown above is equivalent to:
await Promise.all(new Array(5).fill(null).map(() => user.click(getByRole('button'))));
Since this mixes a sync invocation to an async operation, which makes all the async operations run simultaneously (which is undesired). This doesn't guarantee sequence nor waiting for atomic completion per invocation before the next call is started synchronously - this caused warnings to show up. An alternative to for of loop that would wait per call is to:
await user.click(getByRole('button'));
await user.click(getByRole('button'));
await user.click(getByRole('button'));
await user.click(getByRole('button'));
await user.click(getByRole('button'));
which is verbose, but effective.
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for (const _ of new Array(10).fill(null)) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await user.click(getByRole('button')); | ||
| } |
There was a problem hiding this comment.
Should this be refactored as shown by https://eslint.org/docs/latest/rules/no-await-in-loop?
There was a problem hiding this comment.
It's not the most elegant way of doing it (adding the disable rule comments does not help) but the for of loop in this use case, allows us to handle async calls in a step by step fashion. I mentioned more here: #1419 (comment)
| for (const _ of new Array(5).fill(null)) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await user.click(getByRole('button')); | ||
| } |
There was a problem hiding this comment.
Can this be refactored as an internal async function as shown by https://eslint.org/docs/latest/rules/no-await-in-loop?
Francois-Esquire
force-pushed
the
francois-esquire/update-testing-library-user-events
branch
from
4819337
to
953a723
Compare
Francois-Esquire
deleted the
francois-esquire/update-testing-library-user-events
branch
Description
This PR updates
@testing-library/user-eventsto v14.Details
Some notable differences in the migration.
userEventin a test file, a setup function call was madeconst user = userEvents.setup(), whereuserwas used in place ofuserEventasyncuserEventneeded to be setup withuserEvents.setup({ delay: null })fireEventdid take their place (datepickersanddropdowns).Checklist
design updates will be Garden Designer approved (add the designer as a reviewer)demo is up-to-date (yarn start)renders as expected with reversed (RTL) directionrenders as expected with Bedrock CSS (?bedrock)includes new unit teststested for WCAG 2.1 AA compliancetested in Chrome, Firefox, Safari, and Edge