fix(waitForElementToBeRemoved): Make initial check work with getBy*.#1094
Conversation
This commit includes a regression test. Fixes testing-library#1093.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d5b3f35:
|
| // created here so we get a nice stacktrace | ||
| const timeoutError = new Error('Timed out in waitForElementToBeRemoved.') | ||
| if (typeof callback !== 'function') { | ||
| initialCheck(callback) |
There was a problem hiding this comment.
This use of initialCheck is replaced by a simpler check.
| } else { | ||
| const elements = Array.isArray(callback) ? callback : [callback] | ||
| const getRemainingElements = elements.map(element => { | ||
| if (!element) return () => null |
There was a problem hiding this comment.
This line replaces the removed call to initialCheck.
Codecov Report
@@ Coverage Diff @@
## main #1094 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 954 959 +5
Branches 314 316 +2
=========================================
+ Hits 954 959 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
eps1lon
left a comment
eps1lon
left a comment
There was a problem hiding this comment.
Thanks. It does look like the new behavior is intended.
Though the implementation needs some work.
| } | ||
| } | ||
|
|
||
| function wrapFunctionCallback(callback) { |
There was a problem hiding this comment.
"wrap" it with what? It would help a lot if we could come up with a less generic name for this function.
| return callback() | ||
| } catch (error) { | ||
| if (error.name === 'TestingLibraryElementError') { | ||
| return null |
There was a problem hiding this comment.
The original code return undefined. Is there a reason to change the return type now?
There was a problem hiding this comment.
The previous return value of a similar code pattern was undefined because it was directly mapping to the return value needed by the waitFor callback. Now the code is almost the same but null gets translated to undefined through some indirection: isRemoved(null) yields undefined.
The function wrapFunctionCallback is currently defined as a convenience transformation of getBy* queries to make them behave like queryBy* queries. Maybe this help answer #1094 (comment)?
Another discussion point is that maybe trying to accept both getBy*- and queryBy*-based callbacks is a mistake. The way it is implemented now, we could remove wrapFunctionCallback if the user only used queryBy*-based callbacks.
2 participants
This commit includes a regression test. Fixes #1093.