Skip to content

Commit 9b90d67

Browse files
mdjastrzebskithymikee
authored andcommitted
fix(breaking): getByTestId should work only for native elements (#324)
* Replaced getByTestId with fixed version * Added tests * Fixed flow-check * Updated docs * Fixed lint error * Used getByTestId in unit tests instead of queryByTestId * Updated negative checks in tests
1 parent b5bfb28 commit 9b90d67

File tree

4 files changed

+41
-18
lines changed

4 files changed

+41
-18
lines changed

src/__tests__/getByApi.test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// @flow
2+
import React from 'react';
3+
import { View, Text, TextInput, Button } from 'react-native';
4+
import { render } from '..';
5+
6+
const MyComponent = () => {
7+
return <Text>My Component</Text>;
8+
};
9+
10+
test('getByTestId returns only native elements', () => {
11+
const { getByTestId, getAllByTestId } = render(
12+
<View>
13+
<Text testID="text">Text</Text>
14+
<TextInput testID="textInput" />
15+
<View testID="view" />
16+
<Button testID="button" title="Button" onPress={jest.fn()} />
17+
<MyComponent testID="myComponent" />
18+
</View>
19+
);
20+
21+
expect(getByTestId('text')).toBeTruthy();
22+
expect(getByTestId('textInput')).toBeTruthy();
23+
expect(getByTestId('view')).toBeTruthy();
24+
expect(getByTestId('button')).toBeTruthy();
25+
26+
expect(getAllByTestId('text')).toHaveLength(1);
27+
expect(getAllByTestId('textInput')).toHaveLength(1);
28+
expect(getAllByTestId('view')).toHaveLength(1);
29+
expect(getAllByTestId('button')).toHaveLength(1);
30+
31+
expect(() => getByTestId('myComponent')).toThrowError(
32+
'No instances found with testID: myComponent'
33+
);
34+
expect(() => getAllByTestId('myComponent')).toThrowError(
35+
'No instances found with testID: myComponent'
36+
);
37+
});

src/helpers/findByAPI.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// @flow
22
import waitForElement from '../waitForElement';
33
import {
4-
fixedGetByTestId,
4+
getByTestId,
55
getAllByTestId,
66
getByText,
77
getAllByText,
@@ -31,7 +31,7 @@ const makeFindQuery = <Text, Result>(
3131
export const findByTestId = (instance: ReactTestInstance) => (
3232
testId: string,
3333
waitForOptions: WaitForOptions = {}
34-
) => makeFindQuery(instance, fixedGetByTestId, testId, waitForOptions);
34+
) => makeFindQuery(instance, getByTestId, testId, waitForOptions);
3535

3636
export const findAllByTestId = (instance: ReactTestInstance) => (
3737
testId: string,

src/helpers/getByAPI.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,6 @@ export const getByProps = (instance: ReactTestInstance, warnFn?: Function) =>
149149
};
150150

151151
export const getByTestId = (instance: ReactTestInstance) =>
152-
function getByTestIdFn(testID: string) {
153-
try {
154-
return instance.findByProps({ testID });
155-
} catch (error) {
156-
throw new ErrorWithStack(prepareErrorMessage(error), getByTestIdFn);
157-
}
158-
};
159-
160-
export const fixedGetByTestId = (instance: ReactTestInstance) =>
161152
function getByTestIdFn(testID: string) {
162153
try {
163154
const results = getAllByTestId(instance)(testID);

website/docs/Queries.md

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ title: Queries
3232

3333
`findAllBy` queries return a promise which resolves to an array when any matching elements are found. The promise is rejected if no elements match after a default timeout of 4500ms.
3434

35-
3635
:::info
37-
`findBy` and `findAllBy` queries accept optional `waitForOptions` object argument which can contain `timeout` and `interval` properies which have the same meaning as respective arguments to [`waitForElement`](https://callstack.github.io/react-native-testing-library/docs/api#waitforelement) function.
36+
`findBy` and `findAllBy` queries accept optional `waitForOptions` object argument which can contain `timeout` and `interval` properies which have the same meaning as respective arguments to [`waitForElement`](https://callstack.github.io/react-native-testing-library/docs/api#waitforelement) function.
3837
:::
3938

4039
## Queries
@@ -105,11 +104,7 @@ const element = getByTestId('unique-id');
105104
```
106105

107106
:::caution
108-
Please be mindful when using these API and **treat it as an escape hatch**. Your users can't interact with `testID` anyhow, so you may end up writing tests that provide false sense of security. Favor text and accessibility queries instead.
109-
:::
110-
111-
:::danger
112-
Current implementation of `getByTestId` and `queryByTestId` has a serious flaw, which results in finding more IDs than there really would be present in native React host components. Fixing it may break some of your tests so we'll do it in next major release (v2). As a temporary workaround, please use `getAllByTestId('your-id')[0]` or `queryAllByTestId('your-id')[0]` or migrate off testing with testID, which is considered to be an escape hatch.
107+
Please be mindful when using these API and **treat it as an escape hatch**. Your users can't interact with `testID` anyhow, so you may end up writing tests that provide false sense of security. Favor text and accessibility queries instead.
113108
:::
114109

115110
### `ByA11yLabel`, `ByAccessibilityLabel`

0 commit comments

Comments
 (0)