From 9b90d6790dbab3a9a5186ddaa49bc78746d562e5 Mon Sep 17 00:00:00 2001
From: Maciej Jastrzebski <mdjastrzebski@gmail.com>
Date: Mon, 18 May 2020 13:00:18 +0200
Subject: [PATCH] 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
---
 src/__tests__/getByApi.test.js | 37 ++++++++++++++++++++++++++++++++++
 src/helpers/findByAPI.js       |  4 ++--
 src/helpers/getByAPI.js        |  9 ---------
 website/docs/Queries.md        |  9 ++-------
 4 files changed, 41 insertions(+), 18 deletions(-)
 create mode 100644 src/__tests__/getByApi.test.js

diff --git a/src/__tests__/getByApi.test.js b/src/__tests__/getByApi.test.js
new file mode 100644
index 00000000..db66b1c6
--- /dev/null
+++ b/src/__tests__/getByApi.test.js
@@ -0,0 +1,37 @@
+// @flow
+import React from 'react';
+import { View, Text, TextInput, Button } from 'react-native';
+import { render } from '..';
+
+const MyComponent = () => {
+  return <Text>My Component</Text>;
+};
+
+test('getByTestId returns only native elements', () => {
+  const { getByTestId, getAllByTestId } = render(
+    <View>
+      <Text testID="text">Text</Text>
+      <TextInput testID="textInput" />
+      <View testID="view" />
+      <Button testID="button" title="Button" onPress={jest.fn()} />
+      <MyComponent testID="myComponent" />
+    </View>
+  );
+
+  expect(getByTestId('text')).toBeTruthy();
+  expect(getByTestId('textInput')).toBeTruthy();
+  expect(getByTestId('view')).toBeTruthy();
+  expect(getByTestId('button')).toBeTruthy();
+
+  expect(getAllByTestId('text')).toHaveLength(1);
+  expect(getAllByTestId('textInput')).toHaveLength(1);
+  expect(getAllByTestId('view')).toHaveLength(1);
+  expect(getAllByTestId('button')).toHaveLength(1);
+
+  expect(() => getByTestId('myComponent')).toThrowError(
+    'No instances found with testID: myComponent'
+  );
+  expect(() => getAllByTestId('myComponent')).toThrowError(
+    'No instances found with testID: myComponent'
+  );
+});
diff --git a/src/helpers/findByAPI.js b/src/helpers/findByAPI.js
index 7b773979..ea4fb1a7 100644
--- a/src/helpers/findByAPI.js
+++ b/src/helpers/findByAPI.js
@@ -1,7 +1,7 @@
 // @flow
 import waitForElement from '../waitForElement';
 import {
-  fixedGetByTestId,
+  getByTestId,
   getAllByTestId,
   getByText,
   getAllByText,
@@ -31,7 +31,7 @@ const makeFindQuery = <Text, Result>(
 export const findByTestId = (instance: ReactTestInstance) => (
   testId: string,
   waitForOptions: WaitForOptions = {}
-) => makeFindQuery(instance, fixedGetByTestId, testId, waitForOptions);
+) => makeFindQuery(instance, getByTestId, testId, waitForOptions);
 
 export const findAllByTestId = (instance: ReactTestInstance) => (
   testId: string,
diff --git a/src/helpers/getByAPI.js b/src/helpers/getByAPI.js
index 052d3d56..cc607960 100644
--- a/src/helpers/getByAPI.js
+++ b/src/helpers/getByAPI.js
@@ -149,15 +149,6 @@ export const getByProps = (instance: ReactTestInstance, warnFn?: Function) =>
   };
 
 export const getByTestId = (instance: ReactTestInstance) =>
-  function getByTestIdFn(testID: string) {
-    try {
-      return instance.findByProps({ testID });
-    } catch (error) {
-      throw new ErrorWithStack(prepareErrorMessage(error), getByTestIdFn);
-    }
-  };
-
-export const fixedGetByTestId = (instance: ReactTestInstance) =>
   function getByTestIdFn(testID: string) {
     try {
       const results = getAllByTestId(instance)(testID);
diff --git a/website/docs/Queries.md b/website/docs/Queries.md
index ab1465f1..3a7af677 100644
--- a/website/docs/Queries.md
+++ b/website/docs/Queries.md
@@ -32,9 +32,8 @@ title: Queries
 
 `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.
 
-
 :::info
-`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. 
+`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.
 :::
 
 ## Queries
@@ -105,11 +104,7 @@ const element = getByTestId('unique-id');
 ```
 
 :::caution
-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. 
-:::
-
-:::danger
-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.
+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.
 :::
 
 ### `ByA11yLabel`, `ByAccessibilityLabel`