Skip to content

Allow for text to match children who are composed as array #132

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

Merged
merged 4 commits into from
Mar 18, 2019

Conversation

eliperkins
Copy link
Contributor

Summary

Given a component that renders text by composing together literal text with an inline expression for a dynamic variable, React Native will render this as a <Text> element with multiple children. For example:

const BananaCounter = ({ numBananas }) => (
  <Text>There are {numBananas} bananas in the bunch</Text>
);

const { toJSON, debug } = render(<BananaCounter numBananas={3} />);

debug();
/*
      <Text>
        There are
        3
         bananas in the bunch
      </Text>
*/

expect(toJSON()).toMatchInlineSnapshot(`
<Text>
  There are 
  3
   bananas in the bunch
</Text>
`);

This makes sense, as the component is a:

  • literal string
  • a dynamic evaluation
  • literal string

Unfortunately, this means that writing a test that finds an element based on that dynamic evaluation fails when using getByText.

const { getByText } = render(<BananaCounter numBananas={3} />);
expect(getByText('There are 3 bananas in the bunch')).toBeTruthy(); // Fails

This is because we compare the given test string directly against children.

? text === node.props.children
: text.test(node.props.children))

This results in comparing 'There are 3 bananas in the bunch' === ['There are ' 3, ' bananas in the bunch'], which of course will fail.

The solution is to join children and compare the joined result against the given text string.

Test plan

A test for this new functionality is provided!

Given a component that renders text by composing together literal text with an inline expression for a dynamic variable, React Native will render this as a `<Text>` element with multiple children. For example:

```js
const BananaCounter = ({ numBananas }) => (
  <Text>There are {numBananas} bananas in the bunch</Text>
);

const { toJSON, debug } = render(<BananaCounter numBananas={3} />);

debug();
/*
      <Text>
        There are
        3
         bananas in the bunch
      </Text>
*/

expect(toJSON()).toMatchInlineSnapshot(`
<Text>
  There are
  3
   bananas in the bunch
</Text>
`);
```

This makes sense, as the component is a:
- literal string
- a dynamic evaluation
- literal string

Unfortunately, this means that writing a test that finds an element based on that dynamic evaluation fails when using `getByText`.

```js

const { getByText } = render(<BananaCounter numBananas={3} />);
expect(getByText('There are 3 bananas in the bunch')).toBeTruthy(); // Fails
```

This is because we compare the given test string directly against `children`. https://github.com/callstack/react-native-testing-library/blob/ce3bf28f308728672bc5502e120048821c60218b/src/helpers/getByAPI.js#L23-L24

This results in comparing `'There are 3 bananas in the bunch' === ['There are ' 3, ' bananas in the bunch']`, which of course will fail.

The solution is to join children and compare the joined result against the given text string.
Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@thymikee thymikee requested a review from Esemesek March 14, 2019 16:03
@Esemesek
Copy link
Collaborator

@eliperkins I think we should also mention this behavior in the documentation. Can you also make changes there?

@Esemesek Esemesek merged commit 97ab842 into callstack:master Mar 18, 2019
@eliperkins eliperkins deleted the text-as-array branch March 18, 2019 15:25
@eliperkins
Copy link
Contributor Author

Thanks y'all! 🎊

@Esemesek
Copy link
Collaborator

Thank you too ❤️ ❤️ ❤️

@thymikee
Copy link
Member

Available in 1.6.1, thank you :)

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

Successfully merging this pull request may close these issues.

3 participants