fix(breaking): correct faulty implementation of finding by name (#62)

* fix(breaking): correct faulty implementation of finding by name

* chore: update docs

* refactor: code review

Author: thymikee

README.md

@@ -30,14 +30,15 @@ This library is a replacement for [Enzyme](http://airbnb.io/enzyme/).
 ```jsx
 import { render, fireEvent } from 'react-native-testing-library';
 import { QuestionsBoard } from '../QuestionsBoard';
+import { Question } from '../Question';
 
 function setAnswer(question, answer) {
   fireEvent.changeText(question, answer);
 }
 
 test('should verify two questions', () => {
   const { getAllByName, getByText } = render(<QuestionsBoard {...props} />);
-  const allQuestions = getAllByName('Question');
+  const allQuestions = getAllByName(Question);
 
   setAnswer(allQuestions[0], 'a1');
   setAnswer(allQuestions[1], 'a2');
@@ -84,7 +85,7 @@ Deeply render given React element and returns helpers to query the output.
 ```jsx
 import { render } from 'react-native-testing-library';
 
-const { getByTestId, getByName /*...*/ } = render(<Component />);
+const { getByTestId, getByText /*...*/ } = render(<Component />);
 ```
 
 Returns a `RenderResult` object with following properties:
@@ -104,13 +105,13 @@ type ReactTestInstance = {
 };
 ```
 
-### `getByName: (name: string | React.ComponentType<*>)`
+### `getByName: (name: React.ComponentType<*>)`
 
-A method returning a `ReactTestInstance` with matching name – may be a string or React Element. Throws when no matches.
+A method returning a `ReactTestInstance` with matching a React component type. Throws when no matches.
 
-### `getAllByName: (name: string | React.ComponentType<*>)`
+### `getAllByName: (name: React.ComponentType<*>)`
 
-A method returning an array of `ReactTestInstance`s with matching name – may be a string or React Element.
+A method returning an array of `ReactTestInstance`s with matching a React component type.
 
 ### `getByText: (text: string | RegExp)`
 

package.json

@@ -24,7 +24,7 @@
     "jest": "^23.6.0",
     "metro-react-native-babel-preset": "^0.49.0",
     "react": "16.6.1",
-    "react-native": "^0.57.3",
+    "react-native": "^0.57.5",
     "react-test-renderer": "16.6.1",
     "release-it": "^8.0.0",
     "strip-ansi": "^5.0.0",

src/__tests__/__snapshots__/shallow.test.js.snap

@@ -13,11 +13,11 @@ exports[`shallow rendering React Test Instance 1`] = `
 `;
 
 exports[`shallow rendering React elements 1`] = `
-<Component>
-  <Component
+<View>
+  <Text
     testID="text-button"
   >
     Press me
-  </Component>
-</Component>
+  </Text>
+</View>
 `;

src/__tests__/render.test.js

@@ -57,7 +57,7 @@ test('getByTestId, queryByTestId', () => {
   const component = getByTestId('bananaFresh');
 
   expect(component.props.children).toBe('not fresh');
-  expect(() => getByTestId('InExistent')).toThrow();
+  expect(() => getByTestId('InExistent')).toThrow('No instances found');
 
   expect(getByTestId('bananaFresh')).toBe(component);
   expect(queryByTestId('InExistent')).toBeNull();
@@ -76,7 +76,8 @@ test('getByName, queryByName', () => {
   sameButton.props.onPress();
 
   expect(bananaFresh.props.children).toBe('not fresh');
-  expect(() => getByName('InExistent')).toThrow();
+  expect(() => getByName('InExistent')).toThrow('No instances found');
+  expect(() => getByName(Text)).toThrow('Expected 1 but found 3');
 
   expect(queryByName('Button')).toBe(button);
   expect(queryByName('InExistent')).toBeNull();
@@ -89,7 +90,7 @@ test('getAllByName, queryAllByName', () => {
   expect(text.props.children).toBe('Is the banana fresh?');
   expect(status.props.children).toBe('not fresh');
   expect(button.props.children).toBe('Change freshness!');
-  expect(() => getAllByName('InExistent')).toThrow();
+  expect(() => getAllByName('InExistent')).toThrow('No instances found');
 
   expect(queryAllByName('Text')[1]).toBe(status);
   expect(queryAllByName('InExistent')).toHaveLength(0);
@@ -104,7 +105,7 @@ test('getByText, queryByText', () => {
   const sameButton = getByText('not fresh');
 
   expect(sameButton.props.children).toBe('not fresh');
-  expect(() => getByText('InExistent')).toThrow();
+  expect(() => getByText('InExistent')).toThrow('No instances found');
 
   expect(queryByText(/change/i)).toBe(button);
   expect(queryByText('InExistent')).toBeNull();
@@ -115,7 +116,7 @@ test('getAllByText, queryAllByText', () => {
   const buttons = getAllByText(/fresh/i);
 
   expect(buttons).toHaveLength(3);
-  expect(() => getAllByText('InExistent')).toThrow();
+  expect(() => getAllByText('InExistent')).toThrow('No instances found');
 
   expect(queryAllByText(/fresh/i)).toEqual(buttons);
   expect(queryAllByText('InExistent')).toHaveLength(0);
@@ -126,7 +127,9 @@ test('getByProps, queryByProps', () => {
   const primaryType = getByProps({ type: 'primary' });
 
   expect(primaryType.props.children).toBe('Change freshness!');
-  expect(() => getByProps({ type: 'inexistent' })).toThrow();
+  expect(() => getByProps({ type: 'inexistent' })).toThrow(
+    'No instances found'
+  );
 
   expect(queryByProps({ type: 'primary' })).toBe(primaryType);
   expect(queryByProps({ type: 'inexistent' })).toBeNull();
@@ -137,7 +140,9 @@ test('getAllByProp, queryAllByProps', () => {
   const primaryTypes = getAllByProps({ type: 'primary' });
 
   expect(primaryTypes).toHaveLength(1);
-  expect(() => getAllByProps({ type: 'inexistent' })).toThrow();
+  expect(() => getAllByProps({ type: 'inexistent' })).toThrow(
+    'No instances found'
+  );
 
   expect(queryAllByProps({ type: 'primary' })).toEqual(primaryTypes);
   expect(queryAllByProps({ type: 'inexistent' })).toHaveLength(0);

src/__tests__/renderRNComponents.test.js

@@ -0,0 +1,55 @@
+// @flow
+import React from 'react';
+import {
+  Image,
+  Text,
+  TextInput,
+  Modal,
+  View,
+  ScrollView,
+  ActivityIndicator,
+  TouchableOpacity,
+} from 'react-native';
+
+import { render } from '..';
+
+class RNComponents extends React.Component<*> {
+  render() {
+    return (
+      <View>
+        <Modal visible>
+          <ScrollView>
+            <Image />
+            <TextInput value="value" />
+            <TouchableOpacity onPress={() => {}}>
+              <Text>t1</Text>
+              <Text>t2</Text>
+              <Text>t3</Text>
+            </TouchableOpacity>
+            <ActivityIndicator show />
+          </ScrollView>
+        </Modal>
+      </View>
+    );
+  }
+}
+
+test('getByName smoke test to see how unstable it gets', () => {
+  const { getByName } = render(<RNComponents />);
+  expect(() => getByName('Image')).toThrow(); // – doesn't have displayName set properly
+  getByName('TextInput');
+  expect(() => getByName('Modal')).toThrow(); // – doesn't have displayName set properly
+  getByName('View');
+  getByName('ScrollView');
+  expect(() => getByName('ActivityIndicator')).toThrow(); // – doesn't have displayName set properly
+  getByName('TouchableOpacity');
+});
+
+test('getAllByName smoke test to see how unstable it gets', () => {
+  const { getAllByName } = render(<RNComponents />);
+
+  const [t1, t2, t3] = getAllByName('Text');
+  expect(t1.props.children).toBe('t1');
+  expect(t2.props.children).toBe('t2');
+  expect(t3.props.children).toBe('t3');
+});

src/debug.js

@@ -1,7 +1,7 @@
 // @flow
 /* eslint-disable no-console */
 import * as React from 'react';
-import prettyFormat, { plugins } from 'pretty-format'; // eslint-disable-line import/no-extraneous-dependencies
+import prettyFormat, { plugins } from 'pretty-format';
 import shallow from './shallow';
 import render from './render';
 

src/fireEvent.js

@@ -1,5 +1,5 @@
 // @flow
-import ErrorWithStack from './helpers/errorWithStack';
+import { ErrorWithStack } from './helpers/errors';
 
 const findEventHandler = (element: ReactTestInstance, eventName: string) => {
   const eventHandler = toEventHandlerName(eventName);

src/helpers/errorWithStack.js

@@ -0,0 +1,16 @@
+// @flow
+export class ErrorWithStack extends Error {
+  constructor(message: ?string, callsite: Function) {
+    super(message);
+    if (Error.captureStackTrace) {
+      Error.captureStackTrace(this, callsite);
+    }
+  }
+}
+
+export const createLibraryNotSupportedError = (error: Error) =>
+  new Error(
+    `Currently the only supported library to search by text is "react-native".\n\n${
+      error.message
+    }`
+  );

src/helpers/errors.js

@@ -1,24 +1,42 @@
 // @flow
 import * as React from 'react';
-import ErrorWithStack from './errorWithStack';
+import prettyFormat from 'pretty-format';
+import { ErrorWithStack, createLibraryNotSupportedError } from './errors';
 
-const getNodeByName = (node, name) =>
-  node.type.name === name ||
-  node.type.displayName === name ||
-  node.type === name;
+const filterNodeByType = (node, type) => node.type === type;
 
-const getNodeByText = (node, text) =>
-  (getNodeByName(node, 'Text') || getNodeByName(node, 'TextInput')) &&
-  (typeof text === 'string'
-    ? text === node.props.children
-    : text.test(node.props.children));
+const filterNodeByName = (node, name) =>
+  typeof node.type !== 'string' &&
+  (node.type.displayName === name || node.type.name === name);
 
+const getNodeByText = (node, text) => {
+  try {
+    // eslint-disable-next-line
+    const { Text, TextInput } = require('react-native');
+    return (
+      (filterNodeByType(node, Text) || filterNodeByType(node, TextInput)) &&
+      (typeof text === 'string'
+        ? text === node.props.children
+        : text.test(node.props.children))
+    );
+  } catch (error) {
+    throw createLibraryNotSupportedError(error);
+  }
+};
+
+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<*>) {
     try {
-      return instance.find(node => getNodeByName(node, name));
+      return typeof name === 'string'
+        ? instance.find(node => filterNodeByName(node, name))
+        : instance.findByType(name);
     } catch (error) {
-      throw new ErrorWithStack(`Component not found.`, getByNameFn);
+      throw new ErrorWithStack(prepareErrorMessage(error), getByNameFn);
     }
   };
 
@@ -27,7 +45,7 @@ export const getByText = (instance: ReactTestInstance) =>
     try {
       return instance.find(node => getNodeByText(node, text));
     } catch (error) {
-      throw new ErrorWithStack(`Component not found.`, getByTextFn);
+      throw new ErrorWithStack(prepareErrorMessage(error), getByTextFn);
     }
   };
 
@@ -36,7 +54,7 @@ export const getByProps = (instance: ReactTestInstance) =>
     try {
       return instance.findByProps(props);
     } catch (error) {
-      throw new ErrorWithStack(`Component not found.`, getByPropsFn);
+      throw new ErrorWithStack(prepareErrorMessage(error), getByPropsFn);
     }
   };
 
@@ -45,15 +63,19 @@ export const getByTestId = (instance: ReactTestInstance) =>
     try {
       return instance.findByProps({ testID });
     } catch (error) {
-      throw new ErrorWithStack(`Component not found.`, getByTestIdFn);
+      throw new ErrorWithStack(prepareErrorMessage(error), getByTestIdFn);
     }
   };
 
+// TODO: deprecate getAllByName(string | type) in favor of getAllByType(type)
 export const getAllByName = (instance: ReactTestInstance) =>
   function getAllByNameFn(name: string | React.ComponentType<*>) {
-    const results = instance.findAll(node => getNodeByName(node, name));
+    const results =
+      typeof name === 'string'
+        ? instance.findAll(node => filterNodeByName(node, name))
+        : instance.findAllByType(name);
     if (results.length === 0) {
-      throw new ErrorWithStack(`Components not found.`, getAllByNameFn);
+      throw new ErrorWithStack('No instances found', getAllByNameFn);
     }
     return results;
   };
@@ -62,7 +84,10 @@ export const getAllByText = (instance: ReactTestInstance) =>
   function getAllByTextFn(text: string | RegExp) {
     const results = instance.findAll(node => getNodeByText(node, text));
     if (results.length === 0) {
-      throw new ErrorWithStack(`Components not found.`, getAllByTextFn);
+      throw new ErrorWithStack(
+        `No instances found with text: ${String(text)}`,
+        getAllByTextFn
+      );
     }
     return results;
   };
@@ -71,7 +96,10 @@ export const getAllByProps = (instance: ReactTestInstance) =>
   function getAllByPropsFn(props: { [propName: string]: any }) {
     const results = instance.findAllByProps(props);
     if (results.length === 0) {
-      throw new ErrorWithStack(`Components not found.`, getAllByPropsFn);
+      throw new ErrorWithStack(
+        `No instances found with props:\n${prettyFormat(props)}`,
+        getAllByPropsFn
+      );
     }
     return results;
   };