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

fix: toHaveTextContent collect properly text of children nodes #61

Merged
merged 2 commits into from
Aug 9, 2022
Merged

fix: toHaveTextContent collect properly text of children nodes #61

merged 2 commits into from
Aug 9, 2022

Conversation

Grohden
Copy link
Contributor

@Grohden Grohden commented Jun 19, 2021

What:

I've fixed #60 but this also cover the fixes from #59 (I've also copied the PR author test... but if the author wishes to merge his changes first and then have me apply my changes and rewrite this PR I can do it)

I've also added a missing test to assure that toHaveTextContent fails when it should

Why:

Described in #60, but to summarize: toHaveTextContent wasn't collecting properly children nodes to find text

How:

I've made a simple recursive function that transverses all children collecting text childs and join them for the match comparison

Checklist:

  • Documentation added to the docs (N/A)
  • Typescript definitions updated (N/A) - signatures doesn't change
  • Tests
  • Ready to be merged: all the tests are passing, so I'm assuming that this PR only fixes the current behaviour.
  • I've only used ramda compositions because this project seems to be using ramda, but if you wish I can rewrite things without ramda.
  • For another PR: I would recommend using babel-transform-imports for ramda imports (transform import {target} from 'ramda' to import target from 'ramda/es/{target}'), this would speed up things a little for jest/babel as it would not need to bundle/collect a lot of unused ramda functions from the ramda index.js

@cam-shaw
Copy link

Oh yay! Using the code in the PR solves an issue I've been having.
This PR resolves an issue when using the React-i18n Trans component, and proving it a components object as a prop.

It'd be great if we could have this merged in, assuming it doesn't present too many flaws by digging 'n' levels deep.

@cam-shaw
Copy link

@brunohkbx @thymikee aside from conflicts, thoughts on this PR?

@Grohden
Copy link
Contributor Author

Grohden commented Dec 21, 2021

Conflicts solved :D

@finnp
Copy link

finnp commented Jan 14, 2022

Hey ☺️ any updates here from the maintainers? Having a toHaveTextContent that actually collects all the textNodes would be superb :)

@codecov
Copy link

codecov bot commented Aug 8, 2022

Codecov Report

Merging #61 (1959684) into main (09d898d) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #61   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          149       143    -6     
  Branches        50        44    -6     
=========================================
- Hits           149       143    -6     
Flag Coverage Δ
node-12 100.00% <100.00%> (ø)
node-14 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/to-have-text-content.js 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@mdjastrzebski mdjastrzebski merged commit 8d0e79f into testing-library:main Aug 9, 2022
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

🎉 This PR is included in version 4.0.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Grohden Grohden deleted the fix/to-have-text-content branch September 5, 2022 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

toHaveTextContent doesn't work properly when a custom component doesn't use children
5 participants