Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(waitForElementToBeRemoved): Make initial check work with getBy*. #1094

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(waitForElementToBeRemoved): Make initial check work with getBy*. #1094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/__tests__/wait-for-element-to-be-removed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ test('requires an unempty array of elements to exist first (function form)', ()
)
})

test('requires a getBy* query to not throw first', () => {
const {getByTestId} = renderIntoDocument(`
<div data-testid="div"></div>
`)
return expect(
waitForElementToBeRemoved(() => getByTestId('non-existing-testid')),
).rejects.toThrowErrorMatchingInlineSnapshot(
`The element(s) given to waitForElementToBeRemoved are already removed. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal.`,
)
})

test('after successful removal, fullfills promise with empty value (undefined)', () => {
const {getByTestId} = renderIntoDocument(`
<div data-testid="div"></div>
Expand Down
29 changes: 18 additions & 11 deletions src/wait-for-element-to-be-removed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,28 @@ function initialCheck(elements) {
}
}

function wrapFunctionCallback(callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"wrap" it with what? It would help a lot if we could come up with a less generic name for this function.

return () => {
try {
return callback()
} catch (error) {
if (error.name === 'TestingLibraryElementError') {
return null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code return undefined. Is there a reason to change the return type now?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
throw error
}
}
}

async function waitForElementToBeRemoved(callback, options) {
// created here so we get a nice stacktrace
const timeoutError = new Error('Timed out in waitForElementToBeRemoved.')
if (typeof callback !== 'function') {
initialCheck(callback)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This use of initialCheck is replaced by a simpler check.

if (typeof callback === 'function') {
callback = wrapFunctionCallback(callback)
} else {
const elements = Array.isArray(callback) ? callback : [callback]
const getRemainingElements = elements.map(element => {
if (!element) return () => null
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line replaces the removed call to initialCheck.

let parent = element.parentElement
if (parent === null) return () => null
while (parent.parentElement) parent = parent.parentElement
Expand All @@ -30,15 +45,7 @@ async function waitForElementToBeRemoved(callback, options) {
initialCheck(callback())

return waitFor(() => {
let result
try {
result = callback()
} catch (error) {
if (error.name === 'TestingLibraryElementError') {
return undefined
}
throw error
}
const result = callback()
if (!isRemoved(result)) {
throw timeoutError
}
Expand Down