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.

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

[BREAKING]: Deprecate waitForElement #416

Closed
kentcdodds opened this issue Dec 13, 2019 · 3 comments
Closed

[BREAKING]: Deprecate waitForElement #416

kentcdodds opened this issue Dec 13, 2019 · 3 comments
Labels
BREAKING CHANGE This change will require a major version bump needs discussion We need to discuss this to come up with a good solution released

Comments

@kentcdodds
Copy link
Member

Just a thought. What if we remove waitForElement in favor of find* queries?

For history: waitForElement was created before find* queries. Now that we have find* queries, I don't see much of a reason to continue to support it. Most of the time you can use a query, and in the rare situation where you can't use a query to get an element, then you can use wait:

const element = await wait(() => {
  const el = container.querySelector('.blah')
  if (!el) throw new Error('cannot find element with .blah')
  return el
})

It's a bit of boilerplate, but keep in mind that this is not something people should be doing very often nor is it something we want to encourage people to do.

I believe that some of the usages of waitForElement could be codemodable, but not all of it.

Thoughts?

@kentcdodds kentcdodds added needs discussion We need to discuss this to come up with a good solution BREAKING CHANGE This change will require a major version bump labels Dec 13, 2019
@timdeschryver
Copy link
Member

Could the same thing be said about waitForElementToBeRemoved?
Asking because the API design with waitForElement and waitForElementToBeRemoved compliments each other nicely imho, but both functions can be written with wait. That's why I would expect to see both functions, or none of the two.

@kentcdodds
Copy link
Member Author

Fair point... Hmmm... I'm leaning toward keeping it as-is then.

@kentcdodds kentcdodds changed the title [BREAKING]: Remove waitForElement [BREAKING]: Deprecate waitForElement Mar 4, 2020
kentcdodds pushed a commit that referenced this issue Mar 4, 2020
Closes #376
Closes #416

BREAKING CHANGE: `waitForElement` is deprecated in favor of `find*` queries or `wait`.
BREAKING CHANGE: `waitForDomChange` is deprecated in favor of `wait`
BREAKING CHANGE: default timeout for async utilities is now 1000ms rather than 4500ms. This can be configured: https://testing-library.com/docs/dom-testing-library/api-configuration
kentcdodds pushed a commit that referenced this issue Mar 4, 2020
Closes #376
Closes #416

BREAKING CHANGE: `waitForElement` is deprecated in favor of `find*` queries or `wait`.
BREAKING CHANGE: `waitForDomChange` is deprecated in favor of `wait`
BREAKING CHANGE: default timeout for async utilities is now 1000ms rather than 4500ms. This can be configured: https://testing-library.com/docs/dom-testing-library/api-configuration
kentcdodds pushed a commit that referenced this issue Mar 12, 2020
Closes #376
Closes #416

BREAKING CHANGE: wait is now deprecated in favor of waitFor (which has basically the same API except it requires a callback and it also accepts optional mutation observer arguments).
BREAKING CHANGE: `waitForElement` is deprecated in favor of `find*` queries or `wait`.
BREAKING CHANGE: `waitForDomChange` is deprecated in favor of `wait`
BREAKING CHANGE: default timeout for async utilities is now 1000ms rather than 4500ms. This can be configured: https://testing-library.com/docs/dom-testing-library/api-configuration
@kentcdodds
Copy link
Member Author

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

rmainwork pushed a commit to seas-computing/course-planner that referenced this issue Nov 15, 2021
Aside from being deprecated in later versions(
testing-library/dom-testing-library#416) it's also
not needed here, as we can just return `findByDisplayValue` directly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump needs discussion We need to discuss this to come up with a good solution released
Projects
None yet
Development

No branches or pull requests

2 participants