From 8c87340ca0d2b034db18aa2a65a27dbd79470c04 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Fri, 19 Aug 2022 21:39:15 +0200 Subject: [PATCH 01/34] feat: throw an error when a text is rendered outside a component on render --- src/__tests__/render.test.tsx | 6 ++++++ src/helpers/assertStringsWithinText.ts | 28 ++++++++++++++++++++++++++ src/render.tsx | 2 ++ 3 files changed, 36 insertions(+) create mode 100644 src/helpers/assertStringsWithinText.ts diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index d328abe92..6758aea24 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -283,3 +283,9 @@ test('RenderAPI type', () => { render() as RenderAPI; expect(true).toBeTruthy(); }); + +test('should throw when rendering a string outside a text component', () => { + expect(() => render(hello)).toThrowError( + 'Text strings must be rendered within a component.' + ); +}); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts new file mode 100644 index 000000000..85ddd3c62 --- /dev/null +++ b/src/helpers/assertStringsWithinText.ts @@ -0,0 +1,28 @@ +import React, { ReactElement } from 'react'; +import { ReactTestInstance } from 'react-test-renderer'; +import { filterNodeByType } from './filterNodeByType'; + +export const assertStringsWithinText = (instance: ReactTestInstance): void => { + const nodesWithStringChild = instance.findAll(hasNodeStringChild); + nodesWithStringChild.forEach((node) => { + const isTextComponent = filterNodeByType(node, 'Text'); + const isNativeComponent = typeof node.type === 'string'; + + if (!isTextComponent && isNativeComponent) { + throw new Error('Text strings must be rendered within a component.'); + } + }); +}; + +const hasNodeStringChild = (node: ReactTestInstance) => { + const children: (string | number | ReactElement)[] = node.props.children; + let nodeHasStringChild = false; + + React.Children.forEach(children, (child) => { + if (typeof child === 'string') { + nodeHasStringChild = true; + } + }); + + return nodeHasStringChild; +}; diff --git a/src/render.tsx b/src/render.tsx index 20ce89487..41d8c3639 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -7,6 +7,7 @@ import debugShallow from './helpers/debugShallow'; import debugDeep from './helpers/debugDeep'; import { getQueriesForElement } from './within'; import { setRenderResult } from './screen'; +import { assertStringsWithinText } from './helpers/assertStringsWithinText'; export type RenderOptions = { wrapper?: React.ComponentType; @@ -36,6 +37,7 @@ export default function render( ); const update = updateWithAct(renderer, wrap); const instance = renderer.root; + assertStringsWithinText(instance); const unmount = () => { act(() => { renderer.unmount(); From 1ecdf9784391b5a3d2fbbc14221803728a8a794b Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 20 Aug 2022 17:26:39 +0200 Subject: [PATCH 02/34] feat: check on every rerender if strings are rendered outside Text --- src/__tests__/fireEvent.test.tsx | 27 ++++++++++++++++++++++ src/__tests__/render.test.tsx | 8 +++++++ src/render.tsx | 39 ++++++++++++++++++++++++++++---- src/renderHook.tsx | 4 ++-- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/__tests__/fireEvent.test.tsx b/src/__tests__/fireEvent.test.tsx index 30afd8848..ab3bd2b42 100644 --- a/src/__tests__/fireEvent.test.tsx +++ b/src/__tests__/fireEvent.test.tsx @@ -1,4 +1,5 @@ import * as React from 'react'; +import { useState } from 'react'; import { View, TouchableOpacity, @@ -493,3 +494,29 @@ test('has only onMove', () => { }); expect(handleDrag).toHaveBeenCalled(); }); + +const ErrorComponent = () => { + const [shouldDisplayText, setShouldDisplayText] = useState(false); + + if (!shouldDisplayText) { + return ( + { + setShouldDisplayText(true); + }} + > + Display text + + ); + } + + return text rendered outside text component; +}; + +test('should throw an error when strings are rendered outside Text', () => { + const { getByText } = render(); + + expect(() => fireEvent.press(getByText('Display text'))).toThrowError( + 'Text strings must be rendered within a component.' + ); +}); diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 6758aea24..6965b7dba 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -289,3 +289,11 @@ test('should throw when rendering a string outside a text component', () => { 'Text strings must be rendered within a component.' ); }); + +test('should throw an error when rerendering with text outside of Text component', () => { + const { rerender } = render(); + + expect(() => rerender(hello)).toThrowError( + 'Text strings must be rendered within a component.' + ); +}); diff --git a/src/render.tsx b/src/render.tsx index 41d8c3639..966c80385 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -1,12 +1,13 @@ import TestRenderer from 'react-test-renderer'; import type { ReactTestInstance, ReactTestRenderer } from 'react-test-renderer'; import * as React from 'react'; +import { Profiler } from 'react'; import act from './act'; import { addToCleanupQueue } from './cleanup'; import debugShallow from './helpers/debugShallow'; import debugDeep from './helpers/debugDeep'; import { getQueriesForElement } from './within'; -import { setRenderResult } from './screen'; +import { setRenderResult, screen } from './screen'; import { assertStringsWithinText } from './helpers/assertStringsWithinText'; export type RenderOptions = { @@ -24,12 +25,18 @@ export type RenderResult = ReturnType; * Renders test component deeply using react-test-renderer and exposes helpers * to assert on the output. */ -export default function render( +export function render( component: React.ReactElement, - { wrapper: Wrapper, createNodeMock }: RenderOptions = {} + Wrapper?: RenderOptions['wrapper'], + createNodeMock?: RenderOptions['createNodeMock'], + internalWrap: (innerElement: React.ReactElement) => React.ReactElement = ( + element + ) => element ) { - const wrap = (innerElement: React.ReactElement) => - Wrapper ? {innerElement} : innerElement; + const wrap = (element: React.ReactElement) => + Wrapper + ? internalWrap({element}) + : internalWrap(element); const renderer = renderWithAct( wrap(component), @@ -60,6 +67,28 @@ export default function render( return result; } +export default function renderComponent( + component: React.ReactElement, + { wrapper: Wrapper, createNodeMock }: RenderOptions = {} +) { + const assertStringsWithinTextOnUpdate: React.ProfilerProps['onRender'] = ( + _, + phase + ) => { + if (phase === 'update') { + assertStringsWithinText(screen.container); + } + }; + + const wrap = (innerElement: React.ReactElement) => ( + + {innerElement} + + ); + + return render(component, Wrapper, createNodeMock, wrap); +} + function renderWithAct( component: React.ReactElement, options?: TestRendererOptions diff --git a/src/renderHook.tsx b/src/renderHook.tsx index 35965d00a..f403f2167 100644 --- a/src/renderHook.tsx +++ b/src/renderHook.tsx @@ -1,6 +1,6 @@ import React from 'react'; import type { ComponentType } from 'react'; -import render from './render'; +import { render } from './render'; export type RenderHookResult = { rerender: (props: Props) => void; @@ -45,7 +45,7 @@ export function renderHook( const { rerender: baseRerender, unmount } = render( // @ts-expect-error since option can be undefined, initialProps can be undefined when it should'nt , - { wrapper } + wrapper ); function rerender(rerenderCallbackProps: Props) { From 913a54f81bc01911062807cc4c4ae43eec701988 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Tue, 23 Aug 2022 22:45:16 +0200 Subject: [PATCH 03/34] feat: change error message thrown so it is clearer --- src/__tests__/fireEvent.test.tsx | 2 +- src/__tests__/render.test.tsx | 4 ++-- src/helpers/assertStringsWithinText.ts | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/__tests__/fireEvent.test.tsx b/src/__tests__/fireEvent.test.tsx index ab3bd2b42..c5e3537d3 100644 --- a/src/__tests__/fireEvent.test.tsx +++ b/src/__tests__/fireEvent.test.tsx @@ -517,6 +517,6 @@ test('should throw an error when strings are rendered outside Text', () => { const { getByText } = render(); expect(() => fireEvent.press(getByText('Display text'))).toThrowError( - 'Text strings must be rendered within a component.' + 'Text strings must be rendered within a host Text component.' ); }); diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 6965b7dba..10477afdf 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -286,7 +286,7 @@ test('RenderAPI type', () => { test('should throw when rendering a string outside a text component', () => { expect(() => render(hello)).toThrowError( - 'Text strings must be rendered within a component.' + 'Text strings must be rendered within a host Text component.' ); }); @@ -294,6 +294,6 @@ test('should throw an error when rerendering with text outside of Text component const { rerender } = render(); expect(() => rerender(hello)).toThrowError( - 'Text strings must be rendered within a component.' + 'Text strings must be rendered within a host Text component.' ); }); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index 85ddd3c62..fdcc5b7c5 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -5,11 +5,13 @@ import { filterNodeByType } from './filterNodeByType'; export const assertStringsWithinText = (instance: ReactTestInstance): void => { const nodesWithStringChild = instance.findAll(hasNodeStringChild); nodesWithStringChild.forEach((node) => { - const isTextComponent = filterNodeByType(node, 'Text'); + const isHostTextComponent = filterNodeByType(node, 'Text'); const isNativeComponent = typeof node.type === 'string'; - if (!isTextComponent && isNativeComponent) { - throw new Error('Text strings must be rendered within a component.'); + if (!isHostTextComponent && isNativeComponent) { + throw new Error( + 'Text strings must be rendered within a host Text component.' + ); } }); }; From 4bdf99823b0b8febb8efe06450b2ffacbdd0e2e7 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Tue, 23 Aug 2022 22:46:48 +0200 Subject: [PATCH 04/34] refactor: rename function assertStringsWithinTextOnUpdate with a shorter name --- src/render.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/render.tsx b/src/render.tsx index 966c80385..99df593e6 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -71,17 +71,14 @@ export default function renderComponent( component: React.ReactElement, { wrapper: Wrapper, createNodeMock }: RenderOptions = {} ) { - const assertStringsWithinTextOnUpdate: React.ProfilerProps['onRender'] = ( - _, - phase - ) => { + const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { if (phase === 'update') { assertStringsWithinText(screen.container); } }; const wrap = (innerElement: React.ReactElement) => ( - + {innerElement} ); From 925233e90707f5b6e7768495b717e167eeff188d Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 21:49:01 +0200 Subject: [PATCH 05/34] refactor: move test from fireEvent tests to render tests --- src/__tests__/fireEvent.test.tsx | 27 --------------------------- src/__tests__/render.test.tsx | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/__tests__/fireEvent.test.tsx b/src/__tests__/fireEvent.test.tsx index c5e3537d3..30afd8848 100644 --- a/src/__tests__/fireEvent.test.tsx +++ b/src/__tests__/fireEvent.test.tsx @@ -1,5 +1,4 @@ import * as React from 'react'; -import { useState } from 'react'; import { View, TouchableOpacity, @@ -494,29 +493,3 @@ test('has only onMove', () => { }); expect(handleDrag).toHaveBeenCalled(); }); - -const ErrorComponent = () => { - const [shouldDisplayText, setShouldDisplayText] = useState(false); - - if (!shouldDisplayText) { - return ( - { - setShouldDisplayText(true); - }} - > - Display text - - ); - } - - return text rendered outside text component; -}; - -test('should throw an error when strings are rendered outside Text', () => { - const { getByText } = render(); - - expect(() => fireEvent.press(getByText('Display text'))).toThrowError( - 'Text strings must be rendered within a host Text component.' - ); -}); diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 10477afdf..b002e601e 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -297,3 +297,29 @@ test('should throw an error when rerendering with text outside of Text component 'Text strings must be rendered within a host Text component.' ); }); + +const ErrorComponent = () => { + const [shouldDisplayText, setShouldDisplayText] = React.useState(false); + + if (!shouldDisplayText) { + return ( + { + setShouldDisplayText(true); + }} + > + Display text + + ); + } + + return text rendered outside text component; +}; + +test('should throw an error when strings are rendered outside Text', () => { + const { getByText } = render(); + + expect(() => fireEvent.press(getByText('Display text'))).toThrowError( + 'Text strings must be rendered within a host Text component.' + ); +}); From eb5f03a8cd48465d9c20fb14f5b6e42fd6b22c3f Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 21:50:42 +0200 Subject: [PATCH 06/34] tests: add test case for string validation with string nested in fragment --- src/__tests__/render.test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index b002e601e..5d769bb7e 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -323,3 +323,13 @@ test('should throw an error when strings are rendered outside Text', () => { 'Text strings must be rendered within a host Text component.' ); }); + +test('it should not throw for texts nested in fragments', () => { + expect(() => + render( + + <>hello + + ) + ).not.toThrow(); +}); From 7b79ce611ed6def12dbc508e8c8a7b83c04a7dff Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 22:02:20 +0200 Subject: [PATCH 07/34] refactor: use an object for render params --- src/render.tsx | 26 +++++++++++++++++--------- src/renderHook.tsx | 3 +-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/render.tsx b/src/render.tsx index 99df593e6..ddbe9e7fc 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -21,18 +21,21 @@ type TestRendererOptions = { export type RenderResult = ReturnType; +type RenderParams = RenderOptions & { + component: React.ReactElement; + internalWrap?: (innerElement: React.ReactElement) => React.ReactElement; +}; + /** * Renders test component deeply using react-test-renderer and exposes helpers * to assert on the output. */ -export function render( - component: React.ReactElement, - Wrapper?: RenderOptions['wrapper'], - createNodeMock?: RenderOptions['createNodeMock'], - internalWrap: (innerElement: React.ReactElement) => React.ReactElement = ( - element - ) => element -) { +export function render({ + component, + wrapper: Wrapper, + createNodeMock, + internalWrap = (element) => element, +}: RenderParams) { const wrap = (element: React.ReactElement) => Wrapper ? internalWrap({element}) @@ -83,7 +86,12 @@ export default function renderComponent( ); - return render(component, Wrapper, createNodeMock, wrap); + return render({ + component, + wrapper: Wrapper, + createNodeMock, + internalWrap: wrap, + }); } function renderWithAct( diff --git a/src/renderHook.tsx b/src/renderHook.tsx index f403f2167..bbc67859c 100644 --- a/src/renderHook.tsx +++ b/src/renderHook.tsx @@ -44,8 +44,7 @@ export function renderHook( const { rerender: baseRerender, unmount } = render( // @ts-expect-error since option can be undefined, initialProps can be undefined when it should'nt - , - wrapper + { component: , wrapper } ); function rerender(rerenderCallbackProps: Props) { From 1786fd608c925f7ffcb79c92f2eb7b5c437c67b3 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 22:07:23 +0200 Subject: [PATCH 08/34] feat: add an option false by default for string validation --- src/__tests__/render.test.tsx | 19 +++++++++++++------ src/render.tsx | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 5d769bb7e..e05062b52 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -285,13 +285,13 @@ test('RenderAPI type', () => { }); test('should throw when rendering a string outside a text component', () => { - expect(() => render(hello)).toThrowError( - 'Text strings must be rendered within a host Text component.' - ); + expect(() => + render(hello, { validateRenderedStrings: true }) + ).toThrowError('Text strings must be rendered within a host Text component.'); }); test('should throw an error when rerendering with text outside of Text component', () => { - const { rerender } = render(); + const { rerender } = render(, { validateRenderedStrings: true }); expect(() => rerender(hello)).toThrowError( 'Text strings must be rendered within a host Text component.' @@ -317,7 +317,9 @@ const ErrorComponent = () => { }; test('should throw an error when strings are rendered outside Text', () => { - const { getByText } = render(); + const { getByText } = render(, { + validateRenderedStrings: true, + }); expect(() => fireEvent.press(getByText('Display text'))).toThrowError( 'Text strings must be rendered within a host Text component.' @@ -329,7 +331,12 @@ test('it should not throw for texts nested in fragments', () => { render( <>hello - + , + { validateRenderedStrings: true } ) ).not.toThrow(); }); + +test('it should not throw if option validateRenderedString is false', () => { + expect(() => render(hello)).not.toThrow(); +}); diff --git a/src/render.tsx b/src/render.tsx index ddbe9e7fc..377d3865a 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -13,6 +13,7 @@ import { assertStringsWithinText } from './helpers/assertStringsWithinText'; export type RenderOptions = { wrapper?: React.ComponentType; createNodeMock?: (element: React.ReactElement) => any; + validateRenderedStrings?: boolean; }; type TestRendererOptions = { @@ -35,6 +36,7 @@ export function render({ wrapper: Wrapper, createNodeMock, internalWrap = (element) => element, + validateRenderedStrings = false, }: RenderParams) { const wrap = (element: React.ReactElement) => Wrapper @@ -47,7 +49,11 @@ export function render({ ); const update = updateWithAct(renderer, wrap); const instance = renderer.root; - assertStringsWithinText(instance); + + if (validateRenderedStrings) { + assertStringsWithinText(instance); + } + const unmount = () => { act(() => { renderer.unmount(); @@ -72,8 +78,16 @@ export function render({ export default function renderComponent( component: React.ReactElement, - { wrapper: Wrapper, createNodeMock }: RenderOptions = {} + { + wrapper: Wrapper, + createNodeMock, + validateRenderedStrings, + }: RenderOptions = {} ) { + if (!validateRenderedStrings) { + return render({ component, wrapper: Wrapper, createNodeMock }); + } + const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { if (phase === 'update') { assertStringsWithinText(screen.container); @@ -91,6 +105,7 @@ export default function renderComponent( wrapper: Wrapper, createNodeMock, internalWrap: wrap, + validateRenderedStrings: true, }); } From d3f8fa758a0e657ab0245a4b54382b60a44613fc Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 22:17:47 +0200 Subject: [PATCH 09/34] refactor: rewrite method hasNodeStringChild using Array.some --- src/__tests__/render.test.tsx | 14 ++++++++++++++ src/helpers/assertStringsWithinText.ts | 13 +++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index e05062b52..e80f78e47 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -340,3 +340,17 @@ test('it should not throw for texts nested in fragments', () => { test('it should not throw if option validateRenderedString is false', () => { expect(() => render(hello)).not.toThrow(); }); + +test(`it should throw + - when one of the children is a text + - and the parent is not a Text component`, () => { + expect(() => + render( + + hello + hello + , + { validateRenderedStrings: true } + ) + ).toThrowError('Text strings must be rendered within a host Text component.'); +}); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index fdcc5b7c5..19fdffa09 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -1,4 +1,4 @@ -import React, { ReactElement } from 'react'; +import { ReactElement } from 'react'; import { ReactTestInstance } from 'react-test-renderer'; import { filterNodeByType } from './filterNodeByType'; @@ -18,13 +18,10 @@ export const assertStringsWithinText = (instance: ReactTestInstance): void => { const hasNodeStringChild = (node: ReactTestInstance) => { const children: (string | number | ReactElement)[] = node.props.children; - let nodeHasStringChild = false; - React.Children.forEach(children, (child) => { - if (typeof child === 'string') { - nodeHasStringChild = true; - } - }); + if (Array.isArray(children)) { + return children.some((child) => typeof child === 'string'); + } - return nodeHasStringChild; + return typeof children === 'string'; }; From 2d497f47cb9f4e6253b5038673204ca42e6769c9 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 22:36:19 +0200 Subject: [PATCH 10/34] fix: also throw an error when a number is rendered outside text or withint a fragment outside a text --- src/__tests__/render.test.tsx | 18 +++++++++++++ src/helpers/assertStringsWithinText.ts | 36 +++++++++++++++++++++----- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index e80f78e47..923bc78ca 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -354,3 +354,21 @@ test(`it should throw ) ).toThrowError('Text strings must be rendered within a host Text component.'); }); + +test(`it should throw + - when a string is rendered within a fragment rendered outside a Text`, () => { + expect(() => + render( + + <>hello + , + { validateRenderedStrings: true } + ) + ).toThrowError('Text strings must be rendered within a host Text component.'); +}); + +test('it should throw if a number is rendered outside a text', () => { + expect(() => + render(0, { validateRenderedStrings: true }) + ).toThrowError('Text strings must be rendered within a host Text component.'); +}); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index 19fdffa09..5bd5d2b28 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -1,9 +1,11 @@ -import { ReactElement } from 'react'; +import React from 'react'; import { ReactTestInstance } from 'react-test-renderer'; import { filterNodeByType } from './filterNodeByType'; export const assertStringsWithinText = (instance: ReactTestInstance): void => { - const nodesWithStringChild = instance.findAll(hasNodeStringChild); + const nodesWithStringChild = instance.findAll((node) => + isSomeChildStringOrNumber(node.props.children) + ); nodesWithStringChild.forEach((node) => { const isHostTextComponent = filterNodeByType(node, 'Text'); const isNativeComponent = typeof node.type === 'string'; @@ -16,12 +18,32 @@ export const assertStringsWithinText = (instance: ReactTestInstance): void => { }); }; -const hasNodeStringChild = (node: ReactTestInstance) => { - const children: (string | number | ReactElement)[] = node.props.children; - +const isSomeChildStringOrNumber = ( + children: + | string + | number + | React.ReactElement> +): boolean => { if (Array.isArray(children)) { - return children.some((child) => typeof child === 'string'); + return children.some((child) => { + if (filterNodeByType(child, React.Fragment)) { + return isSomeChildStringOrNumber(child.props.children); + } + return typeof child === 'string' || typeof child === 'number'; + }); + } + + if (children === undefined || children === null) { + return false; + } + + if (typeof children === 'string' || typeof children === 'number') { + return true; + } + + if (filterNodeByType(children, React.Fragment)) { + return isSomeChildStringOrNumber(children.props.children); } - return typeof children === 'string'; + return false; }; From 3181513dfa4f2217d66becc7c52004a4be7d2001 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 29 Aug 2022 22:49:28 +0200 Subject: [PATCH 11/34] add skipped test for failing validation --- src/__tests__/render.test.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 923bc78ca..63c343489 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -372,3 +372,16 @@ test('it should throw if a number is rendered outside a text', () => { render(0, { validateRenderedStrings: true }) ).toThrowError('Text strings must be rendered within a host Text component.'); }); + +const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; + +// eslint-disable-next-line jest/no-disabled-tests +test.skip('it should throw with components returning string value not rendered in Text', () => { + expect(() => + render( + + + + ) + ).toThrow(); +}); From 9a545ca0f6dfd5bf552e0b51690591c5727aa5a4 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Tue, 30 Aug 2022 18:45:39 +0200 Subject: [PATCH 12/34] refactor: rename variable in assertStringsWithinText for more coherence --- src/helpers/assertStringsWithinText.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index 5bd5d2b28..fb413b1f5 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -8,9 +8,9 @@ export const assertStringsWithinText = (instance: ReactTestInstance): void => { ); nodesWithStringChild.forEach((node) => { const isHostTextComponent = filterNodeByType(node, 'Text'); - const isNativeComponent = typeof node.type === 'string'; + const isHostComponent = typeof node.type === 'string'; - if (!isHostTextComponent && isNativeComponent) { + if (!isHostTextComponent && isHostComponent) { throw new Error( 'Text strings must be rendered within a host Text component.' ); From 883e47841de45562368023bf4c53209f3b992c87 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Tue, 30 Aug 2022 19:06:34 +0200 Subject: [PATCH 13/34] refactor: use the json representation for validating strings rendered in text --- src/__tests__/render.test.tsx | 8 ++-- src/helpers/assertStringsWithinText.ts | 59 +++++++++----------------- src/render.tsx | 4 +- 3 files changed, 25 insertions(+), 46 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 63c343489..a2876c57b 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -375,13 +375,13 @@ test('it should throw if a number is rendered outside a text', () => { const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; -// eslint-disable-next-line jest/no-disabled-tests -test.skip('it should throw with components returning string value not rendered in Text', () => { +test('it should throw with components returning string value not rendered in Text', () => { expect(() => render( - + - + , + { validateRenderedStrings: true } ) ).toThrow(); }); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index fb413b1f5..cabca406e 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -1,49 +1,28 @@ -import React from 'react'; -import { ReactTestInstance } from 'react-test-renderer'; -import { filterNodeByType } from './filterNodeByType'; +import { ReactTestRendererNode } from 'react-test-renderer'; -export const assertStringsWithinText = (instance: ReactTestInstance): void => { - const nodesWithStringChild = instance.findAll((node) => - isSomeChildStringOrNumber(node.props.children) - ); - nodesWithStringChild.forEach((node) => { - const isHostTextComponent = filterNodeByType(node, 'Text'); - const isHostComponent = typeof node.type === 'string'; +export const assertStringsWithinText = ( + rendererJSON: ReactTestRendererNode | null | ReactTestRendererNode[] +) => { + if (!rendererJSON) return; - if (!isHostTextComponent && isHostComponent) { - throw new Error( - 'Text strings must be rendered within a host Text component.' - ); - } - }); -}; - -const isSomeChildStringOrNumber = ( - children: - | string - | number - | React.ReactElement> -): boolean => { - if (Array.isArray(children)) { - return children.some((child) => { - if (filterNodeByType(child, React.Fragment)) { - return isSomeChildStringOrNumber(child.props.children); - } - return typeof child === 'string' || typeof child === 'number'; - }); + if (Array.isArray(rendererJSON)) { + rendererJSON.forEach(assertStringsWithinText); + return; } - if (children === undefined || children === null) { - return false; + if (typeof rendererJSON === 'string') { + return; } - if (typeof children === 'string' || typeof children === 'number') { - return true; - } + if (rendererJSON.type !== 'Text') { + if (rendererJSON.children?.some((child) => typeof child === 'string')) { + throw new Error( + 'Text strings must be rendered within a host Text component.' + ); + } - if (filterNodeByType(children, React.Fragment)) { - return isSomeChildStringOrNumber(children.props.children); + if (rendererJSON.children) { + rendererJSON.children.forEach(assertStringsWithinText); + } } - - return false; }; diff --git a/src/render.tsx b/src/render.tsx index 377d3865a..9c6f3c525 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -51,7 +51,7 @@ export function render({ const instance = renderer.root; if (validateRenderedStrings) { - assertStringsWithinText(instance); + assertStringsWithinText(renderer.toJSON()); } const unmount = () => { @@ -90,7 +90,7 @@ export default function renderComponent( const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { if (phase === 'update') { - assertStringsWithinText(screen.container); + assertStringsWithinText(screen.toJSON()); } }; From dba3554cc18cfaf65357ff7f07d858b0418e3da8 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Wed, 31 Aug 2022 22:40:52 +0200 Subject: [PATCH 14/34] tests: override console.error to hide profiler error messages --- src/__tests__/render.test.tsx | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index a2876c57b..eca15c4de 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -9,6 +9,8 @@ import { import stripAnsi from 'strip-ansi'; import { render, fireEvent, RenderAPI } from '..'; +const originalConsoleError = console.error; + type ConsoleLogMock = jest.Mock>; const PLACEHOLDER_FRESHNESS = 'Add custom freshness'; @@ -291,11 +293,23 @@ test('should throw when rendering a string outside a text component', () => { }); test('should throw an error when rerendering with text outside of Text component', () => { + // eslint-disable-next-line no-console + console.error = (errorMessage: string) => { + if ( + !errorMessage.includes( + 'The above error occurred in the component' + ) + ) { + originalConsoleError(errorMessage); + } + }; const { rerender } = render(, { validateRenderedStrings: true }); expect(() => rerender(hello)).toThrowError( 'Text strings must be rendered within a host Text component.' ); + // eslint-disable-next-line no-console + console.error = originalConsoleError; }); const ErrorComponent = () => { @@ -317,6 +331,16 @@ const ErrorComponent = () => { }; test('should throw an error when strings are rendered outside Text', () => { + // eslint-disable-next-line no-console + console.error = (errorMessage: string) => { + if ( + !errorMessage.includes( + 'The above error occurred in the component' + ) + ) { + originalConsoleError(errorMessage); + } + }; const { getByText } = render(, { validateRenderedStrings: true, }); @@ -324,6 +348,8 @@ test('should throw an error when strings are rendered outside Text', () => { expect(() => fireEvent.press(getByText('Display text'))).toThrowError( 'Text strings must be rendered within a host Text component.' ); + // eslint-disable-next-line no-console + console.error = originalConsoleError; }); test('it should not throw for texts nested in fragments', () => { From 0608359bd78002b029a5a0db8c2d48186894e3f7 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Wed, 31 Aug 2022 22:45:09 +0200 Subject: [PATCH 15/34] feat: change name of option for string validation to experimentalValidateStringsRenderedInText --- src/__tests__/render.test.tsx | 20 ++++++++++++-------- src/render.tsx | 12 ++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index eca15c4de..b49ade6d0 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -288,7 +288,9 @@ test('RenderAPI type', () => { test('should throw when rendering a string outside a text component', () => { expect(() => - render(hello, { validateRenderedStrings: true }) + render(hello, { + experimentalValidateStringsRenderedInText: true, + }) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -303,7 +305,9 @@ test('should throw an error when rerendering with text outside of Text component originalConsoleError(errorMessage); } }; - const { rerender } = render(, { validateRenderedStrings: true }); + const { rerender } = render(, { + experimentalValidateStringsRenderedInText: true, + }); expect(() => rerender(hello)).toThrowError( 'Text strings must be rendered within a host Text component.' @@ -342,7 +346,7 @@ test('should throw an error when strings are rendered outside Text', () => { } }; const { getByText } = render(, { - validateRenderedStrings: true, + experimentalValidateStringsRenderedInText: true, }); expect(() => fireEvent.press(getByText('Display text'))).toThrowError( @@ -358,7 +362,7 @@ test('it should not throw for texts nested in fragments', () => { <>hello , - { validateRenderedStrings: true } + { experimentalValidateStringsRenderedInText: true } ) ).not.toThrow(); }); @@ -376,7 +380,7 @@ test(`it should throw hello hello , - { validateRenderedStrings: true } + { experimentalValidateStringsRenderedInText: true } ) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -388,14 +392,14 @@ test(`it should throw <>hello , - { validateRenderedStrings: true } + { experimentalValidateStringsRenderedInText: true } ) ).toThrowError('Text strings must be rendered within a host Text component.'); }); test('it should throw if a number is rendered outside a text', () => { expect(() => - render(0, { validateRenderedStrings: true }) + render(0, { experimentalValidateStringsRenderedInText: true }) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -407,7 +411,7 @@ test('it should throw with components returning string value not rendered in Tex , - { validateRenderedStrings: true } + { experimentalValidateStringsRenderedInText: true } ) ).toThrow(); }); diff --git a/src/render.tsx b/src/render.tsx index 9c6f3c525..bb76b3763 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -13,7 +13,7 @@ import { assertStringsWithinText } from './helpers/assertStringsWithinText'; export type RenderOptions = { wrapper?: React.ComponentType; createNodeMock?: (element: React.ReactElement) => any; - validateRenderedStrings?: boolean; + experimentalValidateStringsRenderedInText?: boolean; }; type TestRendererOptions = { @@ -36,7 +36,7 @@ export function render({ wrapper: Wrapper, createNodeMock, internalWrap = (element) => element, - validateRenderedStrings = false, + experimentalValidateStringsRenderedInText = false, }: RenderParams) { const wrap = (element: React.ReactElement) => Wrapper @@ -50,7 +50,7 @@ export function render({ const update = updateWithAct(renderer, wrap); const instance = renderer.root; - if (validateRenderedStrings) { + if (experimentalValidateStringsRenderedInText) { assertStringsWithinText(renderer.toJSON()); } @@ -81,10 +81,10 @@ export default function renderComponent( { wrapper: Wrapper, createNodeMock, - validateRenderedStrings, + experimentalValidateStringsRenderedInText, }: RenderOptions = {} ) { - if (!validateRenderedStrings) { + if (!experimentalValidateStringsRenderedInText) { return render({ component, wrapper: Wrapper, createNodeMock }); } @@ -105,7 +105,7 @@ export default function renderComponent( wrapper: Wrapper, createNodeMock, internalWrap: wrap, - validateRenderedStrings: true, + experimentalValidateStringsRenderedInText: true, }); } From 72ca0971578bc58a31f6234fb0ee0bc771dceae6 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 12:35:47 +0200 Subject: [PATCH 16/34] refactor: rename experimentalValidateStringsRenderedInText option to unstable_validateStringsRenderedInText --- src/__tests__/render.test.tsx | 16 ++++++++-------- src/render.tsx | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index b49ade6d0..203f0f007 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -289,7 +289,7 @@ test('RenderAPI type', () => { test('should throw when rendering a string outside a text component', () => { expect(() => render(hello, { - experimentalValidateStringsRenderedInText: true, + unstable_validateStringsRenderedInText: true, }) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -306,7 +306,7 @@ test('should throw an error when rerendering with text outside of Text component } }; const { rerender } = render(, { - experimentalValidateStringsRenderedInText: true, + unstable_validateStringsRenderedInText: true, }); expect(() => rerender(hello)).toThrowError( @@ -346,7 +346,7 @@ test('should throw an error when strings are rendered outside Text', () => { } }; const { getByText } = render(, { - experimentalValidateStringsRenderedInText: true, + unstable_validateStringsRenderedInText: true, }); expect(() => fireEvent.press(getByText('Display text'))).toThrowError( @@ -362,7 +362,7 @@ test('it should not throw for texts nested in fragments', () => { <>hello , - { experimentalValidateStringsRenderedInText: true } + { unstable_validateStringsRenderedInText: true } ) ).not.toThrow(); }); @@ -380,7 +380,7 @@ test(`it should throw hello hello , - { experimentalValidateStringsRenderedInText: true } + { unstable_validateStringsRenderedInText: true } ) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -392,14 +392,14 @@ test(`it should throw <>hello , - { experimentalValidateStringsRenderedInText: true } + { unstable_validateStringsRenderedInText: true } ) ).toThrowError('Text strings must be rendered within a host Text component.'); }); test('it should throw if a number is rendered outside a text', () => { expect(() => - render(0, { experimentalValidateStringsRenderedInText: true }) + render(0, { unstable_validateStringsRenderedInText: true }) ).toThrowError('Text strings must be rendered within a host Text component.'); }); @@ -411,7 +411,7 @@ test('it should throw with components returning string value not rendered in Tex , - { experimentalValidateStringsRenderedInText: true } + { unstable_validateStringsRenderedInText: true } ) ).toThrow(); }); diff --git a/src/render.tsx b/src/render.tsx index bb76b3763..3bc6f2703 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -13,7 +13,7 @@ import { assertStringsWithinText } from './helpers/assertStringsWithinText'; export type RenderOptions = { wrapper?: React.ComponentType; createNodeMock?: (element: React.ReactElement) => any; - experimentalValidateStringsRenderedInText?: boolean; + unstable_validateStringsRenderedInText?: boolean; }; type TestRendererOptions = { @@ -36,7 +36,7 @@ export function render({ wrapper: Wrapper, createNodeMock, internalWrap = (element) => element, - experimentalValidateStringsRenderedInText = false, + unstable_validateStringsRenderedInText = false, }: RenderParams) { const wrap = (element: React.ReactElement) => Wrapper @@ -50,7 +50,7 @@ export function render({ const update = updateWithAct(renderer, wrap); const instance = renderer.root; - if (experimentalValidateStringsRenderedInText) { + if (unstable_validateStringsRenderedInText) { assertStringsWithinText(renderer.toJSON()); } @@ -81,10 +81,10 @@ export default function renderComponent( { wrapper: Wrapper, createNodeMock, - experimentalValidateStringsRenderedInText, + unstable_validateStringsRenderedInText, }: RenderOptions = {} ) { - if (!experimentalValidateStringsRenderedInText) { + if (!unstable_validateStringsRenderedInText) { return render({ component, wrapper: Wrapper, createNodeMock }); } @@ -105,7 +105,7 @@ export default function renderComponent( wrapper: Wrapper, createNodeMock, internalWrap: wrap, - experimentalValidateStringsRenderedInText: true, + unstable_validateStringsRenderedInText: true, }); } From c235e7b5ce0e068796a6cb9f2062cf23f641f1e6 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 12:44:57 +0200 Subject: [PATCH 17/34] refactor: extract function assertStringsWithinTextForNode from assertStringsWithinText --- src/helpers/assertStringsWithinText.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index cabca406e..fdbb62025 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -1,28 +1,32 @@ import { ReactTestRendererNode } from 'react-test-renderer'; export const assertStringsWithinText = ( - rendererJSON: ReactTestRendererNode | null | ReactTestRendererNode[] + rendererJSON: ReactTestRendererNode | Array | null ) => { if (!rendererJSON) return; if (Array.isArray(rendererJSON)) { - rendererJSON.forEach(assertStringsWithinText); + rendererJSON.forEach(assertStringsWithinTextForNode); return; } - if (typeof rendererJSON === 'string') { + return assertStringsWithinTextForNode(rendererJSON); +}; + +const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { + if (typeof node === 'string') { return; } - if (rendererJSON.type !== 'Text') { - if (rendererJSON.children?.some((child) => typeof child === 'string')) { + if (node.type !== 'Text') { + if (node.children?.some((child) => typeof child === 'string')) { throw new Error( 'Text strings must be rendered within a host Text component.' ); } - if (rendererJSON.children) { - rendererJSON.children.forEach(assertStringsWithinText); + if (node.children) { + node.children.forEach(assertStringsWithinText); } } }; From b5eec798ed2d200ba6dc57c338246ab397daf41e Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 12:52:23 +0200 Subject: [PATCH 18/34] fix: validate strings also inside of text components --- src/__tests__/render.test.tsx | 11 +++++++++++ src/helpers/assertStringsWithinText.ts | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 203f0f007..5ff706947 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -415,3 +415,14 @@ test('it should throw with components returning string value not rendered in Tex ) ).toThrow(); }); + +test('it should throw when rendering string in a View in a Text', () => { + expect(() => + render( + + hello + , + { unstable_validateStringsRenderedInText: true } + ) + ).toThrowError('Text strings must be rendered within a host Text component.'); +}); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index fdbb62025..79abfb9c1 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -24,9 +24,9 @@ const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { 'Text strings must be rendered within a host Text component.' ); } + } - if (node.children) { - node.children.forEach(assertStringsWithinText); - } + if (node.children) { + node.children.forEach(assertStringsWithinText); } }; From bbcac9f525b48eb0ac852f3344a5e7ad1bd8b21c Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 13:02:55 +0200 Subject: [PATCH 19/34] tests: use pressable instead of touchable in render tests --- .../__snapshots__/render.test.tsx.snap | 30 ++++++------------- src/__tests__/render.test.tsx | 16 ++++------ 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/src/__tests__/__snapshots__/render.test.tsx.snap b/src/__tests__/__snapshots__/render.test.tsx.snap index 3e0dd949c..54a4b292f 100644 --- a/src/__tests__/__snapshots__/render.test.tsx.snap +++ b/src/__tests__/__snapshots__/render.test.tsx.snap @@ -32,18 +32,15 @@ exports[`debug 1`] = ` accessible={true} collapsable={false} focusable={true} + onBlur={[Function onBlur]} onClick={[Function onClick]} + onFocus={[Function onFocus]} onResponderGrant={[Function onResponderGrant]} onResponderMove={[Function onResponderMove]} onResponderRelease={[Function onResponderRelease]} onResponderTerminate={[Function onResponderTerminate]} onResponderTerminationRequest={[Function onResponderTerminationRequest]} onStartShouldSetResponder={[Function onStartShouldSetResponder]} - style={ - Object { - "opacity": 1, - } - } > Change freshness! @@ -97,18 +94,15 @@ exports[`debug changing component: bananaFresh button message should now be "fre accessible={true} collapsable={false} focusable={true} + onBlur={[Function onBlur]} onClick={[Function onClick]} + onFocus={[Function onFocus]} onResponderGrant={[Function onResponderGrant]} onResponderMove={[Function onResponderMove]} onResponderRelease={[Function onResponderRelease]} onResponderTerminate={[Function onResponderTerminate]} onResponderTerminationRequest={[Function onResponderTerminationRequest]} onStartShouldSetResponder={[Function onStartShouldSetResponder]} - style={ - Object { - "opacity": 1, - } - } > Change freshness! @@ -266,18 +260,15 @@ exports[`debug: with message 1`] = ` accessible={true} collapsable={false} focusable={true} + onBlur={[Function onBlur]} onClick={[Function onClick]} + onFocus={[Function onFocus]} onResponderGrant={[Function onResponderGrant]} onResponderMove={[Function onResponderMove]} onResponderRelease={[Function onResponderRelease]} onResponderTerminate={[Function onResponderTerminate]} onResponderTerminationRequest={[Function onResponderTerminationRequest]} onStartShouldSetResponder={[Function onStartShouldSetResponder]} - style={ - Object { - "opacity": 1, - } - } > Change freshness! @@ -303,19 +294,16 @@ exports[`toJSON 1`] = ` press me diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 5ff706947..ffa2d9fb7 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -1,11 +1,5 @@ import * as React from 'react'; -import { - View, - Text, - TextInput, - TouchableOpacity, - SafeAreaView, -} from 'react-native'; +import { View, Text, TextInput, Pressable, SafeAreaView } from 'react-native'; import stripAnsi from 'strip-ansi'; import { render, fireEvent, RenderAPI } from '..'; @@ -23,9 +17,9 @@ const DEFAULT_INPUT_CUSTOMER = 'What banana?'; class MyButton extends React.Component { render() { return ( - + {this.props.children} - + ); } } @@ -321,13 +315,13 @@ const ErrorComponent = () => { if (!shouldDisplayText) { return ( - { setShouldDisplayText(true); }} > Display text - + ); } From f6a5be69f0286b0631dc352e961994bf12409313 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 13:07:12 +0200 Subject: [PATCH 20/34] refactor: extract profiler error message in a variable --- src/__tests__/render.test.tsx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index ffa2d9fb7..80ed91dc7 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -288,14 +288,13 @@ test('should throw when rendering a string outside a text component', () => { ).toThrowError('Text strings must be rendered within a host Text component.'); }); +const profilerErrorMessage = + 'The above error occurred in the component'; + test('should throw an error when rerendering with text outside of Text component', () => { // eslint-disable-next-line no-console console.error = (errorMessage: string) => { - if ( - !errorMessage.includes( - 'The above error occurred in the component' - ) - ) { + if (!errorMessage.includes(profilerErrorMessage)) { originalConsoleError(errorMessage); } }; @@ -331,11 +330,7 @@ const ErrorComponent = () => { test('should throw an error when strings are rendered outside Text', () => { // eslint-disable-next-line no-console console.error = (errorMessage: string) => { - if ( - !errorMessage.includes( - 'The above error occurred in the component' - ) - ) { + if (!errorMessage.includes(profilerErrorMessage)) { originalConsoleError(errorMessage); } }; From 69a88528d41db60c337e0d1cf47f0552edb850d2 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 13:08:43 +0200 Subject: [PATCH 21/34] tests: add another test case for string validation --- src/__tests__/render.test.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 80ed91dc7..07f75c801 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -405,6 +405,17 @@ test('it should throw with components returning string value not rendered in Tex ).toThrow(); }); +test('it should not throw with components returning string value rendered in Text', () => { + expect(() => + render( + + + , + { unstable_validateStringsRenderedInText: true } + ) + ).not.toThrow(); +}); + test('it should throw when rendering string in a View in a Text', () => { expect(() => render( From 6333cce9ae996d13d1c732d19e5ac87fa904d35c Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 20:22:27 +0200 Subject: [PATCH 22/34] refactor: export a single render function for better readability --- src/render.tsx | 104 ++++++++++++++++++++++----------------------- src/renderHook.tsx | 5 ++- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/src/render.tsx b/src/render.tsx index 3bc6f2703..5d5e1d4a2 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -22,38 +22,71 @@ type TestRendererOptions = { export type RenderResult = ReturnType; -type RenderParams = RenderOptions & { - component: React.ReactElement; - internalWrap?: (innerElement: React.ReactElement) => React.ReactElement; -}; - /** * Renders test component deeply using react-test-renderer and exposes helpers * to assert on the output. */ -export function render({ - component, - wrapper: Wrapper, - createNodeMock, - internalWrap = (element) => element, - unstable_validateStringsRenderedInText = false, -}: RenderParams) { +export default function render( + component: React.ReactElement, + { + wrapper: Wrapper, + createNodeMock, + unstable_validateStringsRenderedInText, + }: RenderOptions = {} +) { + if (unstable_validateStringsRenderedInText) { + return renderWithStringValidation(component, { + wrapper: Wrapper, + createNodeMock, + }); + } + const wrap = (element: React.ReactElement) => - Wrapper - ? internalWrap({element}) - : internalWrap(element); + Wrapper ? {element} : element; const renderer = renderWithAct( wrap(component), createNodeMock ? { createNodeMock } : undefined ); + + return enrichRenderResult(renderer, wrap); +} + +function renderWithStringValidation( + component: React.ReactElement, + { + wrapper: Wrapper, + createNodeMock, + }: Omit = {} +) { + const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { + if (phase === 'update') { + assertStringsWithinText(screen.toJSON()); + } + }; + + const wrap = (innerElement: React.ReactElement) => ( + + {Wrapper ? {innerElement} : innerElement} + + ); + + const renderer = renderWithAct( + wrap(component), + createNodeMock ? { createNodeMock } : undefined + ); + assertStringsWithinText(renderer.toJSON()); + + return enrichRenderResult(renderer, wrap); +} + +function enrichRenderResult( + renderer: ReactTestRenderer, + wrap: (element: React.ReactElement) => JSX.Element +) { const update = updateWithAct(renderer, wrap); const instance = renderer.root; - if (unstable_validateStringsRenderedInText) { - assertStringsWithinText(renderer.toJSON()); - } - const unmount = () => { act(() => { renderer.unmount(); @@ -76,39 +109,6 @@ export function render({ return result; } -export default function renderComponent( - component: React.ReactElement, - { - wrapper: Wrapper, - createNodeMock, - unstable_validateStringsRenderedInText, - }: RenderOptions = {} -) { - if (!unstable_validateStringsRenderedInText) { - return render({ component, wrapper: Wrapper, createNodeMock }); - } - - const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { - if (phase === 'update') { - assertStringsWithinText(screen.toJSON()); - } - }; - - const wrap = (innerElement: React.ReactElement) => ( - - {innerElement} - - ); - - return render({ - component, - wrapper: Wrapper, - createNodeMock, - internalWrap: wrap, - unstable_validateStringsRenderedInText: true, - }); -} - function renderWithAct( component: React.ReactElement, options?: TestRendererOptions diff --git a/src/renderHook.tsx b/src/renderHook.tsx index bbc67859c..35965d00a 100644 --- a/src/renderHook.tsx +++ b/src/renderHook.tsx @@ -1,6 +1,6 @@ import React from 'react'; import type { ComponentType } from 'react'; -import { render } from './render'; +import render from './render'; export type RenderHookResult = { rerender: (props: Props) => void; @@ -44,7 +44,8 @@ export function renderHook( const { rerender: baseRerender, unmount } = render( // @ts-expect-error since option can be undefined, initialProps can be undefined when it should'nt - { component: , wrapper } + , + { wrapper } ); function rerender(rerenderCallbackProps: Props) { From 92ff85284bb3dc36ba59ba7d9507a946de7e0fe9 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sat, 3 Sep 2022 20:43:32 +0200 Subject: [PATCH 23/34] feat: add informations in validation string error messages --- src/__tests__/render.test.tsx | 30 ++++++++++++++++++-------- src/helpers/assertStringsWithinText.ts | 12 ++++++----- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index 07f75c801..61a695d75 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -285,7 +285,9 @@ test('should throw when rendering a string outside a text component', () => { render(hello, { unstable_validateStringsRenderedInText: true, }) - ).toThrowError('Text strings must be rendered within a host Text component.'); + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` + ); }); const profilerErrorMessage = @@ -302,8 +304,8 @@ test('should throw an error when rerendering with text outside of Text component unstable_validateStringsRenderedInText: true, }); - expect(() => rerender(hello)).toThrowError( - 'Text strings must be rendered within a host Text component.' + expect(() => rerender(hello)).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` ); // eslint-disable-next-line no-console console.error = originalConsoleError; @@ -338,8 +340,10 @@ test('should throw an error when strings are rendered outside Text', () => { unstable_validateStringsRenderedInText: true, }); - expect(() => fireEvent.press(getByText('Display text'))).toThrowError( - 'Text strings must be rendered within a host Text component.' + expect(() => + fireEvent.press(getByText('Display text')) + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text text rendered outside text component was rendered in a View"` ); // eslint-disable-next-line no-console console.error = originalConsoleError; @@ -371,7 +375,9 @@ test(`it should throw , { unstable_validateStringsRenderedInText: true } ) - ).toThrowError('Text strings must be rendered within a host Text component.'); + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` + ); }); test(`it should throw @@ -383,13 +389,17 @@ test(`it should throw , { unstable_validateStringsRenderedInText: true } ) - ).toThrowError('Text strings must be rendered within a host Text component.'); + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` + ); }); test('it should throw if a number is rendered outside a text', () => { expect(() => render(0, { unstable_validateStringsRenderedInText: true }) - ).toThrowError('Text strings must be rendered within a host Text component.'); + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text 0 was rendered in a View"` + ); }); const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; @@ -424,5 +434,7 @@ test('it should throw when rendering string in a View in a Text', () => { , { unstable_validateStringsRenderedInText: true } ) - ).toThrowError('Text strings must be rendered within a host Text component.'); + ).toThrowErrorMatchingInlineSnapshot( + `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` + ); }); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index 79abfb9c1..e4a33dd7b 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -19,11 +19,13 @@ const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { } if (node.type !== 'Text') { - if (node.children?.some((child) => typeof child === 'string')) { - throw new Error( - 'Text strings must be rendered within a host Text component.' - ); - } + node.children?.forEach((child) => { + if (typeof child === 'string') { + throw new Error( + `Text strings must be rendered within a host Text component. Text ${child} was rendered in a ${node.type}` + ); + } + }); } if (node.children) { From 5b3c94bf3d4ec1c4e4d74e02a2e6e8a03e46635f Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:16:14 +0200 Subject: [PATCH 24/34] feat: change error message to match more closely original message of rn --- .../__snapshots__/render.test.tsx.snap | 49 +++++++++++++++++++ src/__tests__/render.test.tsx | 34 +++++-------- src/helpers/assertStringsWithinText.ts | 2 +- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/__tests__/__snapshots__/render.test.tsx.snap b/src/__tests__/__snapshots__/render.test.tsx.snap index 54a4b292f..3134eebd2 100644 --- a/src/__tests__/__snapshots__/render.test.tsx.snap +++ b/src/__tests__/__snapshots__/render.test.tsx.snap @@ -290,6 +290,55 @@ exports[`debug: with message 1`] = ` " `; +exports[`it should throw + - when one of the children is a text + - and the parent is not a Text component 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "hello" string within a component." +`; + +exports[`it should throw + - when a string is rendered within a fragment rendered outside a Text 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "hello" string within a component." +`; + +exports[`it should throw if a number is rendered outside a text: + "Invariant Violation: Text strings must be rendered within a component. + + Detected attempt to render "0" string within a component." + 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "0" string within a component." +`; + +exports[`it should throw when rendering string in a View in a Text 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "hello" string within a component." +`; + +exports[`should throw an error when rerendering with text outside of Text component 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "hello" string within a component." +`; + +exports[`should throw an error when strings are rendered outside Text 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "text rendered outside text component" string within a component." +`; + +exports[`should throw when rendering a string outside a text component 1`] = ` +"Invariant Violation: Text strings must be rendered within a component. + +Detected attempt to render "hello" string within a component." +`; + exports[`toJSON 1`] = ` { render(hello, { unstable_validateStringsRenderedInText: true, }) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(); }); const profilerErrorMessage = @@ -304,9 +302,8 @@ test('should throw an error when rerendering with text outside of Text component unstable_validateStringsRenderedInText: true, }); - expect(() => rerender(hello)).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` - ); + expect(() => rerender(hello)).toThrowErrorMatchingSnapshot(); + // eslint-disable-next-line no-console console.error = originalConsoleError; }); @@ -342,9 +339,8 @@ test('should throw an error when strings are rendered outside Text', () => { expect(() => fireEvent.press(getByText('Display text')) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text text rendered outside text component was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(); + // eslint-disable-next-line no-console console.error = originalConsoleError; }); @@ -375,9 +371,7 @@ test(`it should throw , { unstable_validateStringsRenderedInText: true } ) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(); }); test(`it should throw @@ -389,17 +383,17 @@ test(`it should throw , { unstable_validateStringsRenderedInText: true } ) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(); }); test('it should throw if a number is rendered outside a text', () => { expect(() => render(0, { unstable_validateStringsRenderedInText: true }) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text 0 was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(` + "Invariant Violation: Text strings must be rendered within a component. + + Detected attempt to render "0" string within a component." + `); }); const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; @@ -434,7 +428,5 @@ test('it should throw when rendering string in a View in a Text', () => { , { unstable_validateStringsRenderedInText: true } ) - ).toThrowErrorMatchingInlineSnapshot( - `"Text strings must be rendered within a host Text component. Text hello was rendered in a View"` - ); + ).toThrowErrorMatchingSnapshot(); }); diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index e4a33dd7b..92eb823fc 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -22,7 +22,7 @@ const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { node.children?.forEach((child) => { if (typeof child === 'string') { throw new Error( - `Text strings must be rendered within a host Text component. Text ${child} was rendered in a ${node.type}` + `Invariant Violation: Text strings must be rendered within a component.\n\nDetected attempt to render "${child}" string within a <${node.type}> component.` ); } }); From 8a46e01d8d0a2338fd2d92617328b76aa4883797 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:25:18 +0200 Subject: [PATCH 25/34] refactor: dont call assertStringsWithinText in assertStringsWithinTextForNode --- src/helpers/assertStringsWithinText.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/assertStringsWithinText.ts index 92eb823fc..57f1f7ad4 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/assertStringsWithinText.ts @@ -29,6 +29,6 @@ const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { } if (node.children) { - node.children.forEach(assertStringsWithinText); + node.children.forEach(assertStringsWithinTextForNode); } }; From e2ec15dc3a039a7cb7e7daf013d0e00df2dbe81f Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:29:11 +0200 Subject: [PATCH 26/34] refactor: rename enrichRenderResult to buildRenderResult --- src/render.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/render.tsx b/src/render.tsx index 5d5e1d4a2..c29c77c66 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -49,7 +49,7 @@ export default function render( createNodeMock ? { createNodeMock } : undefined ); - return enrichRenderResult(renderer, wrap); + return buildRenderResult(renderer, wrap); } function renderWithStringValidation( @@ -77,10 +77,10 @@ function renderWithStringValidation( ); assertStringsWithinText(renderer.toJSON()); - return enrichRenderResult(renderer, wrap); + return buildRenderResult(renderer, wrap); } -function enrichRenderResult( +function buildRenderResult( renderer: ReactTestRenderer, wrap: (element: React.ReactElement) => JSX.Element ) { From f59eaa33ce0faa3eb260e14e5189d6c9956b330b Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:29:55 +0200 Subject: [PATCH 27/34] refactor: change naming for more consistency --- src/render.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/render.tsx b/src/render.tsx index c29c77c66..6fb2c23a6 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -65,9 +65,9 @@ function renderWithStringValidation( } }; - const wrap = (innerElement: React.ReactElement) => ( + const wrap = (element: React.ReactElement) => ( - {Wrapper ? {innerElement} : innerElement} + {Wrapper ? {element} : element} ); From 82c254d43564d0efa94e93c2a2f4b84db4b62872 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:33:07 +0200 Subject: [PATCH 28/34] refactor: rename string validation function to match option name --- ...nText.ts => validateStringsRenderedWithinText.ts} | 12 +++++++----- src/render.tsx | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) rename src/helpers/{assertStringsWithinText.ts => validateStringsRenderedWithinText.ts} (65%) diff --git a/src/helpers/assertStringsWithinText.ts b/src/helpers/validateStringsRenderedWithinText.ts similarity index 65% rename from src/helpers/assertStringsWithinText.ts rename to src/helpers/validateStringsRenderedWithinText.ts index 57f1f7ad4..a281fcc54 100644 --- a/src/helpers/assertStringsWithinText.ts +++ b/src/helpers/validateStringsRenderedWithinText.ts @@ -1,19 +1,21 @@ import { ReactTestRendererNode } from 'react-test-renderer'; -export const assertStringsWithinText = ( +export const validateStringsRenderedWithinText = ( rendererJSON: ReactTestRendererNode | Array | null ) => { if (!rendererJSON) return; if (Array.isArray(rendererJSON)) { - rendererJSON.forEach(assertStringsWithinTextForNode); + rendererJSON.forEach(validateStringsRenderedWithinTextForNode); return; } - return assertStringsWithinTextForNode(rendererJSON); + return validateStringsRenderedWithinTextForNode(rendererJSON); }; -const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { +const validateStringsRenderedWithinTextForNode = ( + node: ReactTestRendererNode +) => { if (typeof node === 'string') { return; } @@ -29,6 +31,6 @@ const assertStringsWithinTextForNode = (node: ReactTestRendererNode) => { } if (node.children) { - node.children.forEach(assertStringsWithinTextForNode); + node.children.forEach(validateStringsRenderedWithinTextForNode); } }; diff --git a/src/render.tsx b/src/render.tsx index 6fb2c23a6..5e5ded8b3 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -8,7 +8,7 @@ import debugShallow from './helpers/debugShallow'; import debugDeep from './helpers/debugDeep'; import { getQueriesForElement } from './within'; import { setRenderResult, screen } from './screen'; -import { assertStringsWithinText } from './helpers/assertStringsWithinText'; +import { validateStringsRenderedWithinText } from './helpers/validateStringsRenderedWithinText'; export type RenderOptions = { wrapper?: React.ComponentType; @@ -61,7 +61,7 @@ function renderWithStringValidation( ) { const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { if (phase === 'update') { - assertStringsWithinText(screen.toJSON()); + validateStringsRenderedWithinText(screen.toJSON()); } }; @@ -75,7 +75,7 @@ function renderWithStringValidation( wrap(component), createNodeMock ? { createNodeMock } : undefined ); - assertStringsWithinText(renderer.toJSON()); + validateStringsRenderedWithinText(renderer.toJSON()); return buildRenderResult(renderer, wrap); } From aacc77fa1996639943ca410b407d1696f3293801 Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Sun, 18 Sep 2022 13:34:31 +0200 Subject: [PATCH 29/34] refactor: change name of the option to be closer to rn error message --- src/__tests__/render.test.tsx | 20 ++++++++++---------- src/render.tsx | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index bde99ad08..a6e355c91 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -283,7 +283,7 @@ test('RenderAPI type', () => { test('should throw when rendering a string outside a text component', () => { expect(() => render(hello, { - unstable_validateStringsRenderedInText: true, + unstable_validateStringsRenderedWithinText: true, }) ).toThrowErrorMatchingSnapshot(); }); @@ -299,7 +299,7 @@ test('should throw an error when rerendering with text outside of Text component } }; const { rerender } = render(, { - unstable_validateStringsRenderedInText: true, + unstable_validateStringsRenderedWithinText: true, }); expect(() => rerender(hello)).toThrowErrorMatchingSnapshot(); @@ -334,7 +334,7 @@ test('should throw an error when strings are rendered outside Text', () => { } }; const { getByText } = render(, { - unstable_validateStringsRenderedInText: true, + unstable_validateStringsRenderedWithinText: true, }); expect(() => @@ -351,7 +351,7 @@ test('it should not throw for texts nested in fragments', () => { <>hello , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).not.toThrow(); }); @@ -369,7 +369,7 @@ test(`it should throw hello hello , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).toThrowErrorMatchingSnapshot(); }); @@ -381,14 +381,14 @@ test(`it should throw <>hello , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).toThrowErrorMatchingSnapshot(); }); test('it should throw if a number is rendered outside a text', () => { expect(() => - render(0, { unstable_validateStringsRenderedInText: true }) + render(0, { unstable_validateStringsRenderedWithinText: true }) ).toThrowErrorMatchingSnapshot(` "Invariant Violation: Text strings must be rendered within a component. @@ -404,7 +404,7 @@ test('it should throw with components returning string value not rendered in Tex , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).toThrow(); }); @@ -415,7 +415,7 @@ test('it should not throw with components returning string value rendered in Tex , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).not.toThrow(); }); @@ -426,7 +426,7 @@ test('it should throw when rendering string in a View in a Text', () => { hello , - { unstable_validateStringsRenderedInText: true } + { unstable_validateStringsRenderedWithinText: true } ) ).toThrowErrorMatchingSnapshot(); }); diff --git a/src/render.tsx b/src/render.tsx index 5e5ded8b3..0bf1aff93 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -13,7 +13,7 @@ import { validateStringsRenderedWithinText } from './helpers/validateStringsRend export type RenderOptions = { wrapper?: React.ComponentType; createNodeMock?: (element: React.ReactElement) => any; - unstable_validateStringsRenderedInText?: boolean; + unstable_validateStringsRenderedWithinText?: boolean; }; type TestRendererOptions = { @@ -31,10 +31,10 @@ export default function render( { wrapper: Wrapper, createNodeMock, - unstable_validateStringsRenderedInText, + unstable_validateStringsRenderedWithinText, }: RenderOptions = {} ) { - if (unstable_validateStringsRenderedInText) { + if (unstable_validateStringsRenderedWithinText) { return renderWithStringValidation(component, { wrapper: Wrapper, createNodeMock, @@ -57,7 +57,7 @@ function renderWithStringValidation( { wrapper: Wrapper, createNodeMock, - }: Omit = {} + }: Omit = {} ) { const handleRender: React.ProfilerProps['onRender'] = (_, phase) => { if (phase === 'update') { From 60b8be62b8483027da036ab2ed4d671d0c90ab9b Mon Sep 17 00:00:00 2001 From: pierrezimmermann Date: Mon, 19 Sep 2022 14:45:34 +0200 Subject: [PATCH 30/34] refactor: make render test names single line --- src/__tests__/render.test.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index a6e355c91..b2e63eda8 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -360,9 +360,7 @@ test('it should not throw if option validateRenderedString is false', () => { expect(() => render(hello)).not.toThrow(); }); -test(`it should throw - - when one of the children is a text - - and the parent is not a Text component`, () => { +test(`it should throw when one of the children is a text and the parent is not a Text component`, () => { expect(() => render( @@ -374,8 +372,7 @@ test(`it should throw ).toThrowErrorMatchingSnapshot(); }); -test(`it should throw - - when a string is rendered within a fragment rendered outside a Text`, () => { +test(`it should throw when a string is rendered within a fragment rendered outside a Text`, () => { expect(() => render( From 57f9917f95c7231b472c555cb06cab8092a8f6b8 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 19 Sep 2022 14:54:12 +0200 Subject: [PATCH 31/34] refactor: extract validate strings tests into separate file --- .../__snapshots__/render.test.tsx.snap | 49 ------ src/__tests__/render-validateStrings.test.tsx | 157 ++++++++++++++++++ src/__tests__/render.test.tsx | 150 ----------------- .../validateStringsRenderedWithinText.ts | 2 +- 4 files changed, 158 insertions(+), 200 deletions(-) create mode 100644 src/__tests__/render-validateStrings.test.tsx diff --git a/src/__tests__/__snapshots__/render.test.tsx.snap b/src/__tests__/__snapshots__/render.test.tsx.snap index 3134eebd2..54a4b292f 100644 --- a/src/__tests__/__snapshots__/render.test.tsx.snap +++ b/src/__tests__/__snapshots__/render.test.tsx.snap @@ -290,55 +290,6 @@ exports[`debug: with message 1`] = ` " `; -exports[`it should throw - - when one of the children is a text - - and the parent is not a Text component 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "hello" string within a component." -`; - -exports[`it should throw - - when a string is rendered within a fragment rendered outside a Text 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "hello" string within a component." -`; - -exports[`it should throw if a number is rendered outside a text: - "Invariant Violation: Text strings must be rendered within a component. - - Detected attempt to render "0" string within a component." - 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "0" string within a component." -`; - -exports[`it should throw when rendering string in a View in a Text 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "hello" string within a component." -`; - -exports[`should throw an error when rerendering with text outside of Text component 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "hello" string within a component." -`; - -exports[`should throw an error when strings are rendered outside Text 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "text rendered outside text component" string within a component." -`; - -exports[`should throw when rendering a string outside a text component 1`] = ` -"Invariant Violation: Text strings must be rendered within a component. - -Detected attempt to render "hello" string within a component." -`; - exports[`toJSON 1`] = ` component.'; +const PROFILER_ERROR = 'The above error occurred in the component'; + +beforeEach(() => { + // eslint-disable-next-line no-console + console.error = (errorMessage: string) => { + if (!errorMessage.includes(PROFILER_ERROR)) { + originalConsoleError(errorMessage); + } + }; +}); + +afterEach(() => { + // eslint-disable-next-line no-console + console.error = originalConsoleError; +}); + +test('should throw when rendering a string outside a text component', () => { + expect(() => + render(hello, { + unstable_validateStringsRenderedWithinText: true, + }) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); + +test('should throw an error when rerendering with text outside of Text component', () => { + const { rerender } = render(, { + unstable_validateStringsRenderedWithinText: true, + }); + + expect(() => rerender(hello)).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); + +const InvalidTextAfterPress = () => { + const [showText, setShowText] = React.useState(false); + + if (!showText) { + return ( + setShowText(true)}> + Show text + + ); + } + + return text rendered outside text component; +}; + +test('should throw an error when strings are rendered outside Text', () => { + const { getByText } = render(, { + unstable_validateStringsRenderedWithinText: true, + }); + + expect(() => fireEvent.press(getByText('Show text'))).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "text rendered outside text component" string within a component.` + ); +}); + +test('should not throw for texts nested in fragments', () => { + expect(() => + render( + + <>hello + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).not.toThrow(); +}); + +test('should not throw if option validateRenderedString is false', () => { + expect(() => render(hello)).not.toThrow(); +}); + +test(`should throw when one of the children is a text and the parent is not a Text component`, () => { + expect(() => + render( + + hello + hello + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); + +test(`should throw when a string is rendered within a fragment rendered outside a Text`, () => { + expect(() => + render( + + <>hello + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); + +test('should throw if a number is rendered outside a text', () => { + expect(() => + render(0, { unstable_validateStringsRenderedWithinText: true }) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "0" string within a component.` + ); +}); + +const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; + +test('should throw with components returning string value not rendered in Text', () => { + expect(() => + render( + + + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); + +test('should not throw with components returning string value rendered in Text', () => { + expect(() => + render( + + + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).not.toThrow(); +}); + +test('should throw when rendering string in a View in a Text', () => { + expect(() => + render( + + hello + , + { unstable_validateStringsRenderedWithinText: true } + ) + ).toThrow( + `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + ); +}); diff --git a/src/__tests__/render.test.tsx b/src/__tests__/render.test.tsx index b2e63eda8..b492ed98b 100644 --- a/src/__tests__/render.test.tsx +++ b/src/__tests__/render.test.tsx @@ -3,8 +3,6 @@ import { View, Text, TextInput, Pressable, SafeAreaView } from 'react-native'; import stripAnsi from 'strip-ansi'; import { render, fireEvent, RenderAPI } from '..'; -const originalConsoleError = console.error; - type ConsoleLogMock = jest.Mock>; const PLACEHOLDER_FRESHNESS = 'Add custom freshness'; @@ -279,151 +277,3 @@ test('RenderAPI type', () => { render() as RenderAPI; expect(true).toBeTruthy(); }); - -test('should throw when rendering a string outside a text component', () => { - expect(() => - render(hello, { - unstable_validateStringsRenderedWithinText: true, - }) - ).toThrowErrorMatchingSnapshot(); -}); - -const profilerErrorMessage = - 'The above error occurred in the component'; - -test('should throw an error when rerendering with text outside of Text component', () => { - // eslint-disable-next-line no-console - console.error = (errorMessage: string) => { - if (!errorMessage.includes(profilerErrorMessage)) { - originalConsoleError(errorMessage); - } - }; - const { rerender } = render(, { - unstable_validateStringsRenderedWithinText: true, - }); - - expect(() => rerender(hello)).toThrowErrorMatchingSnapshot(); - - // eslint-disable-next-line no-console - console.error = originalConsoleError; -}); - -const ErrorComponent = () => { - const [shouldDisplayText, setShouldDisplayText] = React.useState(false); - - if (!shouldDisplayText) { - return ( - { - setShouldDisplayText(true); - }} - > - Display text - - ); - } - - return text rendered outside text component; -}; - -test('should throw an error when strings are rendered outside Text', () => { - // eslint-disable-next-line no-console - console.error = (errorMessage: string) => { - if (!errorMessage.includes(profilerErrorMessage)) { - originalConsoleError(errorMessage); - } - }; - const { getByText } = render(, { - unstable_validateStringsRenderedWithinText: true, - }); - - expect(() => - fireEvent.press(getByText('Display text')) - ).toThrowErrorMatchingSnapshot(); - - // eslint-disable-next-line no-console - console.error = originalConsoleError; -}); - -test('it should not throw for texts nested in fragments', () => { - expect(() => - render( - - <>hello - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).not.toThrow(); -}); - -test('it should not throw if option validateRenderedString is false', () => { - expect(() => render(hello)).not.toThrow(); -}); - -test(`it should throw when one of the children is a text and the parent is not a Text component`, () => { - expect(() => - render( - - hello - hello - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).toThrowErrorMatchingSnapshot(); -}); - -test(`it should throw when a string is rendered within a fragment rendered outside a Text`, () => { - expect(() => - render( - - <>hello - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).toThrowErrorMatchingSnapshot(); -}); - -test('it should throw if a number is rendered outside a text', () => { - expect(() => - render(0, { unstable_validateStringsRenderedWithinText: true }) - ).toThrowErrorMatchingSnapshot(` - "Invariant Violation: Text strings must be rendered within a component. - - Detected attempt to render "0" string within a component." - `); -}); - -const Trans = ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}; - -test('it should throw with components returning string value not rendered in Text', () => { - expect(() => - render( - - - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).toThrow(); -}); - -test('it should not throw with components returning string value rendered in Text', () => { - expect(() => - render( - - - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).not.toThrow(); -}); - -test('it should throw when rendering string in a View in a Text', () => { - expect(() => - render( - - hello - , - { unstable_validateStringsRenderedWithinText: true } - ) - ).toThrowErrorMatchingSnapshot(); -}); diff --git a/src/helpers/validateStringsRenderedWithinText.ts b/src/helpers/validateStringsRenderedWithinText.ts index a281fcc54..844697cf8 100644 --- a/src/helpers/validateStringsRenderedWithinText.ts +++ b/src/helpers/validateStringsRenderedWithinText.ts @@ -24,7 +24,7 @@ const validateStringsRenderedWithinTextForNode = ( node.children?.forEach((child) => { if (typeof child === 'string') { throw new Error( - `Invariant Violation: Text strings must be rendered within a component.\n\nDetected attempt to render "${child}" string within a <${node.type}> component.` + `Invariant Violation: Text strings must be rendered within a component. Detected attempt to render "${child}" string within a <${node.type}> component.` ); } }); From ba213808eb8a61a83b958bbde3c30e29001408eb Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 19 Sep 2022 14:58:17 +0200 Subject: [PATCH 32/34] chore: tweak file naming --- ...validateStringsRenderedWithinText.ts => stringValidation.ts} | 0 src/render.tsx | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/helpers/{validateStringsRenderedWithinText.ts => stringValidation.ts} (100%) diff --git a/src/helpers/validateStringsRenderedWithinText.ts b/src/helpers/stringValidation.ts similarity index 100% rename from src/helpers/validateStringsRenderedWithinText.ts rename to src/helpers/stringValidation.ts diff --git a/src/render.tsx b/src/render.tsx index 0bf1aff93..88951f515 100644 --- a/src/render.tsx +++ b/src/render.tsx @@ -8,7 +8,7 @@ import debugShallow from './helpers/debugShallow'; import debugDeep from './helpers/debugDeep'; import { getQueriesForElement } from './within'; import { setRenderResult, screen } from './screen'; -import { validateStringsRenderedWithinText } from './helpers/validateStringsRenderedWithinText'; +import { validateStringsRenderedWithinText } from './helpers/stringValidation'; export type RenderOptions = { wrapper?: React.ComponentType; From 417c2babf91671d35b869659b833c7aeeb6a5fd4 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 19 Sep 2022 14:59:30 +0200 Subject: [PATCH 33/34] chore: file naming consistency --- ...-validateStrings.test.tsx => render-stringValidation.test.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/__tests__/{render-validateStrings.test.tsx => render-stringValidation.test.tsx} (100%) diff --git a/src/__tests__/render-validateStrings.test.tsx b/src/__tests__/render-stringValidation.test.tsx similarity index 100% rename from src/__tests__/render-validateStrings.test.tsx rename to src/__tests__/render-stringValidation.test.tsx From e8890fd60a7447aeea32aa6954cce01b28da3141 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 19 Sep 2022 15:01:40 +0200 Subject: [PATCH 34/34] refactor: final tweaks --- src/__tests__/render-stringValidation.test.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/__tests__/render-stringValidation.test.tsx b/src/__tests__/render-stringValidation.test.tsx index 6ce7da2d0..a0be42672 100644 --- a/src/__tests__/render-stringValidation.test.tsx +++ b/src/__tests__/render-stringValidation.test.tsx @@ -6,7 +6,7 @@ import { render, fireEvent } from '..'; const originalConsoleError = console.error; const VALIDATION_ERROR = - 'Invariant Violation: Text strings must be rendered within a component.'; + 'Invariant Violation: Text strings must be rendered within a component'; const PROFILER_ERROR = 'The above error occurred in the component'; beforeEach(() => { @@ -29,7 +29,7 @@ test('should throw when rendering a string outside a text component', () => { unstable_validateStringsRenderedWithinText: true, }) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); }); @@ -39,7 +39,7 @@ test('should throw an error when rerendering with text outside of Text component }); expect(() => rerender(hello)).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); }); @@ -63,7 +63,7 @@ test('should throw an error when strings are rendered outside Text', () => { }); expect(() => fireEvent.press(getByText('Show text'))).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "text rendered outside text component" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "text rendered outside text component" string within a component.` ); }); @@ -92,7 +92,7 @@ test(`should throw when one of the children is a text and the parent is not a Te { unstable_validateStringsRenderedWithinText: true } ) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); }); @@ -105,7 +105,7 @@ test(`should throw when a string is rendered within a fragment rendered outside { unstable_validateStringsRenderedWithinText: true } ) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); }); @@ -113,7 +113,7 @@ test('should throw if a number is rendered outside a text', () => { expect(() => render(0, { unstable_validateStringsRenderedWithinText: true }) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "0" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "0" string within a component.` ); }); @@ -128,7 +128,7 @@ test('should throw with components returning string value not rendered in Text', { unstable_validateStringsRenderedWithinText: true } ) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); }); @@ -152,6 +152,6 @@ test('should throw when rendering string in a View in a Text', () => { { unstable_validateStringsRenderedWithinText: true } ) ).toThrow( - `${VALIDATION_ERROR} Detected attempt to render "hello" string within a component.` + `${VALIDATION_ERROR}. Detected attempt to render "hello" string within a component.` ); });