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

toHaveTextContent matching rules are not aligned with jest-dom #1613

Open
AugustinLF opened this issue May 22, 2024 · 3 comments
Open

toHaveTextContent matching rules are not aligned with jest-dom #1613

AugustinLF opened this issue May 22, 2024 · 3 comments

Comments

@AugustinLF
Copy link
Collaborator

Hey hey, long time not posted here^^'

Describe the bug

toHaveTextContent() does exact matching by default, while the same helper exposed by jest-dom does partial matching.

test('toHaveTextContent() does exact match by default', () => {
  render(<Text testID="text">Hello World</Text>);

  const text = screen.getByTestId('text');
  expect(text).toHaveTextContent('Hello World');
  // this will error with jest-dom when using RTL.
  expect(text).not.toHaveTextContent('Hello');
  expect(text).not.toHaveTextContent('World');
});

Expected behavior

I would expect both APIs to behave similarly. Furthermore, the jest-dom matcher doesn't support the exact option, which makes it a bit hard to run both web and native tests with the same code.

I'll happily open a conversion in jest-dom's repo, but we should be sure of what we want before hand. Have you had discussions with them on this topic?

@mdjastrzebski
Copy link
Member

mdjastrzebski commented May 22, 2024

@AugustinLF good to see you back mate! 🙌

Regarding the change I have couple of thoughts:
1). Making the default behavior match Jest DOM would be a breaking change, and since Jest matchers are now in RNTL that would be a major version for RNTL as a whole.
2) I find the Jest DOM matcher name confusing, as toHaveTextContent kinda suggest content == text condition. A more suitable name would be toContainText or similar. In such case the non-exact default behavior would make sense.
3) The semantic of this matcher in JestDOM is different from getByText which checks the whole text, and has exact option. In contrast the RNLT matcher has the same semantic and options as getByText.

As for your use case, if you use exactly the same code to run RTL and RNTL tests, then consider creating own Jest matcher, that would conditionally call JestDOM or RNTL matchers with proper options. That seems like a more bulletproof solution.

Alternatively you can pass regex instead of string, and in such case both implementations should work the same.

Wdyt?

@AugustinLF
Copy link
Collaborator Author

You're raising good points. I don't think the fact that such a change would require a major bump is a blocker (we can definitely wait to do such a change when planning to release the next major version), but as highlighted in your answer, the question is indeed "do we think this API change would be beneficial".

Personally I think that the API exposed by RNTL is the better one (aligning with getByText only makes sense IMO). But there is a value nonetheless in having similar API between both libraries, I'd guess that there is a non negligible part of our user base that does use both libraries. They might not have like me a setup where they "write once, test everywhere", but having to keep in mind which behaviour belongs to which platform is definitely not great. Especially when it comes to utils like these, where the difference is not driven by a platform difference.

But then I'll raise that on the jest-dom repo, and se what they think of it.

In the meanwhile, would you be up for a PR that allows exporting the matchers, so I can (similarly to jest-dom) use part of them?

import matchers from '@testing-library/react-native/matchers'

const { toHaveTextContent, ...rest } = matchers

expect.extend({
  ...rest,
  toHaveTextContent: myCustomMatcher,
})

That way I could go indeed go over each matchers, and add what makes sense for both platforms?

@mdjastrzebski
Copy link
Member

@AugustinLF sorry for late reply. Yes, you can submit a PR with export for matchers in RNTL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants