feat: Deprecate toBeEmpty in favour of toBeEmptyDOMElement (#216)

[DanielaValero]

What:

• Add deprecation note to toBeEmpty
• Create toBeEmptyDomElement to replace toBeEmpty
• Document changes

Why:
This is an alternative solution that allows users of jest-dom and jest-extended to check for emptiness of strings, etc. And DOM elements, using different matchers.

How:

• The code of toBeEmptyDomElement is a copy of toBeEmpty, the only changes in there is the name.

Checklist:

• Documentation
• Tests
• [N/A] Updated Type Definitions
• [?] Ready to be merged

[codecov]

Codecov Report

Merging #254 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #254   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        23    +1     
  Lines          305       309    +4     
  Branches        73        73           
=========================================
+ Hits           305       309    +4     

Impacted Files	Coverage Δ
src/to-be-empty-dom-element.js	100.00% <100.00%> (ø)
src/to-be-empty.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85cf707...e770bd2. Read the comment docs.

[gnapse]

Looks god.

I have only one doubt about the name. I wonder if it should be toBeEmptyDOMElement. I really wouldn't mind it like it is now, but I realized we also have toBeInTheDOM (which has been deprecated for a long while now, and I think it's time to remove it in breaking change release already, so maybe this is not a big deal).

[DanielaValero]

Hi @gnapse so should I leave it with this name? I could do another PR to remove toBeInTeDOM

What is the usual process to merge a PR? one approval and I merge myself? two approvals and the second approver merges?

[gnapse]

I'd rename this to toBeEmptyDOMElement regardless of the future removal of toBeInTheDOM. As @KevinHerklotz pointed out above, it is a better reflection of the words in the phrase.

@DanielaValero the removal of toBeInTheDOM is an independent issue. If you still want to take a crack at it, please do so, but do not do it to remove the discrepancy only. And thanks a lot!

[DanielaValero]

Hi @gnapse, about the renaming, sure, it took me a while to notice the part needs renaming is to make DOM all uppercase.

If I might suggest yo an improving area when giving feedback, in the case of renaming or typos, authors of the code not necessarily notice when they did the typo or renaming bit, and it helps a lot to point out specifically what is the part. i.e: Dom -> DOM 🙂

As of the toBeInTheDom I suggested I could do it in a separate PR definitely not with the goal of removing the discrepancy, I meant it more to offer help because you mentioned can be removed.

Thanks for the clarification on the feedback!

The renaming is done, please check again. :)

[gnapse]

@DanielaValero sorry. Yes, I assumed it was evident when it is not. Will keep that in mind. I realize I never mentioned the fact that this word is an acronym even, which would've given the clue about the all-caps vs. capitalized difference.

I meant it more to offer help because you mentioned can be removed.

Sure. Your help is greatly appreciated.

I'll review it all shortly. Thanks!

[gnapse]

@all-contributors add @DanielaValero for code, test, docs

[allcontributors]

@gnapse

I've put up a pull request to add @DanielaValero! 🎉

[gnapse]

🎉 This PR is included in version 5.9.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[charliematters]

Is there an ETA on this arriving in the @types repo?

[darekkay]

@gnapse @charliematters The types PR is merged and a new package version is released, so we only have to update the version here, right? If yes, I can provide a quick PR.

[gnapse]

Yes, please. Do so. Thanks!