From b0f7f31fbc1c5a2b823ceba2abddbc9130f6f37c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= <thymikee@gmail.com>
Date: Mon, 19 Nov 2018 15:07:53 +0100
Subject: [PATCH] feat: deprecate getByName helpers

---
 README.md                 | 32 ++++++++++++++++++--------------
 src/helpers/errors.js     | 24 ++++++++++++++++++++++++
 src/helpers/getByAPI.js   | 10 +++++++---
 src/helpers/queryByAPI.js |  3 +++
 4 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/README.md b/README.md
index 4c309a3e2..9521c8afc 100644
--- a/README.md
+++ b/README.md
@@ -105,13 +105,21 @@ type ReactTestInstance = {
 };
 ```
 
-### `getByName: (name: React.ComponentType<*>)`
+### `getByText: (text: string | RegExp)`
 
-A method returning a `ReactTestInstance` with matching a React component type. Throws when no matches.
+A method returning a `ReactTestInstance` with matching text – may be a string or regular expression. Throws when no matches.
 
-### `getAllByName: (name: React.ComponentType<*>)`
+### `getAllByText: (text: string | RegExp)`
 
-A method returning an array of `ReactTestInstance`s with matching a React component type.
+A method returning an array of `ReactTestInstance`s with matching text – may be a string or regular expression.
+
+### `getByProps: (props: { [propName: string]: any })`
+
+A method returning a `ReactTestInstance` with matching props object. Throws when no matches.
+
+### `getAllByProps: (props: { [propName: string]: any })`
+
+A method returning an array of `ReactTestInstance`s with matching props object.
 
 ### `getByType: (type: React.ComponentType<*>)`
 
@@ -121,21 +129,17 @@ A method returning a `ReactTestInstance` with matching a React component type. T
 
 A method returning an array of `ReactTestInstance`s with matching a React component type.
 
-### `getByText: (text: string | RegExp)`
-
-A method returning a `ReactTestInstance` with matching text – may be a string or regular expression. Throws when no matches.
+### `[DEPRECATED] getByName: (name: React.ComponentType<*>)`
 
-### `getAllByText: (text: string | RegExp)`
-
-A method returning an array of `ReactTestInstance`s with matching text – may be a string or regular expression.
+A method returning a `ReactTestInstance` with matching a React component type. Throws when no matches.
 
-### `getByProps: (props: { [propName: string]: any })`
+> This method has been **deprecated** because using it results in fragile tests that may break between minor React Native versions. It will be removed in next major release (v2.0). Use [`getByType`](#getbytype-type-reactcomponenttype) instead.
 
-A method returning a `ReactTestInstance` with matching props object. Throws when no matches.
+### `[DEPRECATED] getAllByName: (name: React.ComponentType<*>)`
 
-### `getAllByProps: (props: { [propName: string]: any })`
+A method returning an array of `ReactTestInstance`s with matching a React component type.
 
-A method returning an array of `ReactTestInstance`s with matching props object.
+> This method has been **deprecated** because using it results in fragile tests that may break between minor React Native versions. It will be removed in next major release (v2.0). Use [`getAllByType`](#getallbytype-type-reactcomponenttype) instead.
 
 ### `update: (element: React.Element<*>) => void`
 
diff --git a/src/helpers/errors.js b/src/helpers/errors.js
index a0e998e60..c0369cbc2 100644
--- a/src/helpers/errors.js
+++ b/src/helpers/errors.js
@@ -14,3 +14,27 @@ export const createLibraryNotSupportedError = (error: Error) =>
       error.message
     }`
   );
+
+const warned = {
+  getByName: false,
+  getAllByName: false,
+  queryByName: false,
+  queryAllByName: false,
+};
+
+export const logDeprecationWarning = (
+  deprecatedFnName: string,
+  alternativeFnName: string
+) => {
+  if (warned[deprecatedFnName]) {
+    return;
+  }
+  console.warn(`Deprecation Warning:
+
+  "${deprecatedFnName}" is deprecated and will be removed in next major release. Please use "${alternativeFnName}" instead.
+
+  Docs: https://github.com/callstack/react-native-testing-library#${alternativeFnName.toLowerCase()}-type-reactcomponenttype
+    `);
+
+  warned[deprecatedFnName] = true;
+};
diff --git a/src/helpers/getByAPI.js b/src/helpers/getByAPI.js
index 5d8d38d92..5d5ca0dc7 100644
--- a/src/helpers/getByAPI.js
+++ b/src/helpers/getByAPI.js
@@ -1,7 +1,11 @@
 // @flow
 import * as React from 'react';
 import prettyFormat from 'pretty-format';
-import { ErrorWithStack, createLibraryNotSupportedError } from './errors';
+import {
+  ErrorWithStack,
+  createLibraryNotSupportedError,
+  logDeprecationWarning,
+} from './errors';
 
 const filterNodeByType = (node, type) => node.type === type;
 
@@ -28,9 +32,9 @@ const prepareErrorMessage = error =>
   // Strip info about custom predicate
   error.message.replace(/ matching custom predicate[^]*/gm, '');
 
-// TODO: deprecate getByName(string | type) in favor of getByType(type)
 export const getByName = (instance: ReactTestInstance) =>
   function getByNameFn(name: string | React.ComponentType<*>) {
+    logDeprecationWarning('getByName', 'getByType');
     try {
       return typeof name === 'string'
         ? instance.find(node => filterNodeByName(node, name))
@@ -76,9 +80,9 @@ export const getByTestId = (instance: ReactTestInstance) =>
     }
   };
 
-// TODO: deprecate getAllByName(string | type) in favor of getAllByType(type)
 export const getAllByName = (instance: ReactTestInstance) =>
   function getAllByNameFn(name: string | React.ComponentType<*>) {
+    logDeprecationWarning('getAllByName', 'getAllByType');
     const results =
       typeof name === 'string'
         ? instance.findAll(node => filterNodeByName(node, name))
diff --git a/src/helpers/queryByAPI.js b/src/helpers/queryByAPI.js
index b9f58d840..df9ee58f1 100644
--- a/src/helpers/queryByAPI.js
+++ b/src/helpers/queryByAPI.js
@@ -11,10 +11,12 @@ import {
   getAllByText,
   getAllByProps,
 } from './getByAPI';
+import { logDeprecationWarning } from './errors';
 
 export const queryByName = (instance: ReactTestInstance) => (
   name: string | React.ComponentType<*>
 ) => {
+  logDeprecationWarning('queryByName', 'getByName');
   try {
     return getByName(instance)(name);
   } catch (error) {
@@ -65,6 +67,7 @@ export const queryByTestId = (instance: ReactTestInstance) => (
 export const queryAllByName = (instance: ReactTestInstance) => (
   name: string | React.ComponentType<*>
 ) => {
+  logDeprecationWarning('queryAllByName', 'getAllByName');
   try {
     return getAllByName(instance)(name);
   } catch (error) {