Skip to content

ByText queries don't seem to match across multiple Text elements #1221

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

Closed
leepowelldev opened this issue Nov 10, 2022 · 7 comments · Fixed by #1222
Closed

ByText queries don't seem to match across multiple Text elements #1221

leepowelldev opened this issue Nov 10, 2022 · 7 comments · Fixed by #1222
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@leepowelldev
Copy link

Give this test:

import React from 'react';
import { screen, render } from '@testing-library/react-native';
import { Text, View } from 'react-native';

function MyComponent() {
  return (
    <View>
      <Text>Hello</Text>
      <Text>World</Text>
    </View>
  );
}

it('renders correctly', async () => {
  render(<MyComponent />);
  const text = screen.getByText('Hello World');
  expect(text).toBeTruthy();
});

I would expect the test to pass, according to the docs the Text elements should be joined together. I've tried various permutations (regex's, non-exact match, extra Text with a space) and nothing seems to match.

Expected behavior

Test should pass

Steps to Reproduce

Run simple test as above (I can put in a repo if it helps)

Screenshots

Screenshot 2022-11-10 at 15 09 06

Versions

npmPackages:
@testing-library/react-native: ^11.4.0 => 11.4.0
react: 18.1.0 => 18.1.0
react-native: 0.70.3 => 0.70.3
react-test-renderer: 18.1.0 => 18.1.0

@mdjastrzebski
Copy link
Member

The behaviour is party correct. In my opinion it should behave in this way if Text elements are separate. The issue is however that it should match if in following case:

function MyComponent() {
  return (
    <Text>
      <Text>Hello</Text>
      <Text>World</Text>
    </Text>
  );
}

However it does not, and this is the bug that we should fix.

As a temporary workaround I can suggest using some other query + toHaveTextContent matcher from Jest Native.

@leepowelldev
Copy link
Author

Sure, it's just we're in the middle of a upgrade and lots of our tests are now breaking on this one.

@mdjastrzebski
Copy link
Member

Understood, I cannot promise any fix date for that bug, as proper text matching subject is more complex than it looks at first glance, we had couple of back and forth changes in the implementation, and we are ware around introducing potential breaking changes to other users.

If you have interest in this issue and some spare time I invite to provide a PR for that issue. Otherwise, changing query type and/or changing app structure to put string content into single Text instead of separate one might be the fastest workaround.

@mdjastrzebski mdjastrzebski added bug Something isn't working help wanted Extra attention is needed labels Nov 10, 2022
@leepowelldev
Copy link
Author

I've had a dig, and I have it working, but I'm really not sure of the consequences of the changes I've made :D

The first bit I've removed is this:

// Bail on traversing text children down the tree if current node (child)
// has no text. In such situations, react-test-renderer will traverse down
// this tree in a separate call and run this query again. As a result, the
// query will match the deepest text node that matches requested text.
if ((0, _filterNodeByType.filterNodeByType)(child, _reactNative.Text)) {
  return;
}

Maybe react-test-renderer has changed how it traverses, as this seems to be one reason we don't recurse through the children. The minute we hit text children to concat them together we exit.

The second part is removing the condition of this:

if ((0, _filterNodeByType.filterNodeByType)(child, React.Fragment)) {
  textContent.push(...getChildrenAsText(child.props.children));
}

To just:

textContent.push(...getChildrenAsText(child.props.children));

I don't know why it's limited to only fragments, but I'm sure there is a reason.

It does also mean changing the component to havea wrapping Text, to work:

function MyComponent() {
  return (
    <View testID="view">
      <Text>
        <Text testID="text-1">Hello</Text>
        <Text testID="text-2">World</Text>
      </Text>
    </View>
  );
}

So that's a limiting factor ... as you say this is a fragile piece of functionality, but the documentation implies things to be "just right" in order to work.

We were using 7.2.0. and I think I'll be able to use a patch-package to get it close to that behaviour for a temporary fix.

@mdjastrzebski
Copy link
Member

The reason why current implementation is not recursive, is that in following example:

 <Text testID="outer-text">
    <Text testID="text-1">Hello</Text>
    <Text testID="text-2">World</Text>
  </Text>

recursive query for Hello will match both text-1 and outer-text. A proper implementation should match only text-1, while in case of querying for Hello World it should match outer-text. That leads to implementation that match given Text element, only if none of its children is matched. So the current simple findAll tree walking algorithm is insufficient.

@mdjastrzebski
Copy link
Member

@leepowelldev v11.5.0 contains a fix for matching nested Text content.

It should work with:

<Text>
  <Text>Hello</Text>
  <Text>World</Text>
</Text>

but not with

<View>
  <Text>Hello</Text>
  <Text>World</Text>
</View>

@leepowelldev
Copy link
Author

leepowelldev commented Nov 17, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants