From 8ab41f7d915c7d50eec7d08472e06aa10cab24fa Mon Sep 17 00:00:00 2001 From: Sota Ogo Date: Fri, 23 Jul 2021 22:56:22 -0700 Subject: [PATCH 1/3] Show soft errors when a text string or number is supplied as a child instead of throwing an error --- .../src/ReactFabricHostConfig.js | 11 +++++------ .../src/ReactNativeHostConfig.js | 10 +++++----- .../src/__tests__/ReactFabric-test.internal.js | 6 +++--- .../src/__tests__/ReactNativeMount-test.internal.js | 10 +++++----- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 842a33291fb07..7fb63982191cd 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -21,8 +21,6 @@ import type { import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils'; import {create, diff} from './ReactNativeAttributePayload'; -import invariant from 'shared/invariant'; - import {dispatchEvent} from './ReactFabricEventEmitter'; import { @@ -251,10 +249,11 @@ export function createTextInstance( hostContext: HostContext, internalInstanceHandle: Object, ): TextInstance { - invariant( - hostContext.isInAParentText, - 'Text strings must be rendered within a component.', - ); + if (!hostContext.isInAParentText) { + if (__DEV__) { + console.error('Text strings must be rendered within a component.'); + } + } const tag = nextReactTag; nextReactTag += 2; diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 205f8189b7a7c..0cd74ad91211f 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -147,11 +147,11 @@ export function createTextInstance( hostContext: HostContext, internalInstanceHandle: Object, ): TextInstance { - invariant( - hostContext.isInAParentText, - 'Text strings must be rendered within a component.', - ); - + if (!hostContext.isInAParentText) { + if (__DEV__) { + console.error('Text strings must be rendered within a component.'); + } + } const tag = allocateTag(); UIManager.createView( diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 6b776c085fd40..0a0afc4adc2f8 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -665,7 +665,7 @@ describe('ReactFabric', () => { }); }); - it('should throw for text not inside of a ancestor', () => { + it('should console error for text not inside of a ancestor', () => { const ScrollView = createReactNativeComponentClass('RCTScrollView', () => ({ validAttributes: {}, uiViewClassName: 'RCTScrollView', @@ -683,7 +683,7 @@ describe('ReactFabric', () => { act(() => { ReactFabric.render(this should warn, 11); }); - }).toThrow('Text strings must be rendered within a component.'); + }).toErrorDev(['Text strings must be rendered within a component.']); expect(() => { act(() => { @@ -694,7 +694,7 @@ describe('ReactFabric', () => { 11, ); }); - }).toThrow('Text strings must be rendered within a component.'); + }).toErrorDev(['Text strings must be rendered within a component.']); }); it('should not throw for text inside of an indirect ancestor', () => { diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js index 9026bef793767..4d1ffce09fe35 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js @@ -473,7 +473,7 @@ describe('ReactNative', () => { ); }); - it('should throw for text not inside of a ancestor', () => { + it('should console error for text not inside of a ancestor', () => { const ScrollView = createReactNativeComponentClass('RCTScrollView', () => ({ validAttributes: {}, uiViewClassName: 'RCTScrollView', @@ -487,9 +487,9 @@ describe('ReactNative', () => { uiViewClassName: 'RCTView', })); - expect(() => ReactNative.render(this should warn, 11)).toThrow( - 'Text strings must be rendered within a component.', - ); + expect(() => + ReactNative.render(this should warn, 11), + ).toErrorDev(['Text strings must be rendered within a component.']); expect(() => ReactNative.render( @@ -498,7 +498,7 @@ describe('ReactNative', () => { , 11, ), - ).toThrow('Text strings must be rendered within a component.'); + ).toErrorDev(['Text strings must be rendered within a component.']); }); it('should not throw for text inside of an indirect ancestor', () => { From 973eaadad257490749b304ffa7e89cb659db0ccd Mon Sep 17 00:00:00 2001 From: Sota <5866096+sota000@users.noreply.github.com> Date: Mon, 26 Jul 2021 10:11:43 -0700 Subject: [PATCH 2/3] bring __DEV__ check first so that things inside get removed in prod. --- .../react-native-renderer/src/ReactFabricHostConfig.js | 4 ++-- .../react-native-renderer/src/ReactNativeHostConfig.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 7fb63982191cd..5f0d0853a5ef1 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -249,8 +249,8 @@ export function createTextInstance( hostContext: HostContext, internalInstanceHandle: Object, ): TextInstance { - if (!hostContext.isInAParentText) { - if (__DEV__) { + if (__DEV__) { + if (!hostContext.isInAParentText) { console.error('Text strings must be rendered within a component.'); } } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 0cd74ad91211f..991a5cf6af79b 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -147,10 +147,10 @@ export function createTextInstance( hostContext: HostContext, internalInstanceHandle: Object, ): TextInstance { - if (!hostContext.isInAParentText) { - if (__DEV__) { - console.error('Text strings must be rendered within a component.'); - } + if (__DEV__) { + if (!hostContext.isInAParentText) { + console.error('Text strings must be rendered within a component.'); + } } const tag = allocateTag(); From b60d3fe41d3d2638d85019bf83a118b79e7703cb Mon Sep 17 00:00:00 2001 From: Sota <5866096+sota000@users.noreply.github.com> Date: Mon, 26 Jul 2021 11:23:12 -0700 Subject: [PATCH 3/3] fix lint --- packages/react-native-renderer/src/ReactNativeHostConfig.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 991a5cf6af79b..63337a04df1af 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -149,8 +149,8 @@ export function createTextInstance( ): TextInstance { if (__DEV__) { if (!hostContext.isInAParentText) { - console.error('Text strings must be rendered within a component.'); - } + console.error('Text strings must be rendered within a component.'); + } } const tag = allocateTag();