Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use HostContext to warn about invalid View/Text nesting #12766

Merged
merged 15 commits into from
May 14, 2018
Merged

Use HostContext to warn about invalid View/Text nesting #12766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c9c1ca6
Use HostContext to warn about invalid view/text nesting
May 8, 2018
fcf2f7a
Added tests for HostContext warning behavior
May 8, 2018
97709b5
Return host context in DEV only
May 8, 2018
e0bd283
Fixed ReactNativeEvents
May 8, 2018
3b214bc
Simplified HostContext; dont recreate unless value has changed
May 8, 2018
61d1067
Added same HostContext changes to fabric renderer
May 8, 2018
9e8850f
Account for TextInput as well
May 8, 2018
f0f1db2
Account for Images inside of Text. Remove Android-specific check in a…
May 8, 2018
c1c0805
Updated warning in test
May 8, 2018
4821bf8
Bailout earlier when isInAParentText true
May 8, 2018
b810501
Added a test for <Text><ScrollingView>foo case
May 8, 2018
079e4ea
Replaced warning() with console.error()
May 10, 2018
2cab4bf
Made native/fabric HostContext type more explicit
May 11, 2018
a1c01b8
Replace DEV Text/View nesting check with invariant
May 11, 2018
2ca4555
Added an inline comment
May 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions packages/react-native-renderer/src/ReactFabricRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as ReactNativeViewConfigRegistry from 'ReactNativeViewConfigRegistry';
import ReactFiberReconciler from 'react-reconciler';

import deepFreezeAndThrowOnMutationInDev from 'deepFreezeAndThrowOnMutationInDev';
import emptyObject from 'fbjs/lib/emptyObject';
import invariant from 'fbjs/lib/invariant';

// Modules provided by RN:
import TextInputState from 'TextInputState';
Expand All @@ -35,6 +35,10 @@ import UIManager from 'UIManager';
// This means that they never overlap.
let nextReactTag = 2;

type HostContext = $ReadOnly<{|
isInAParentText: boolean,
|}>;

/**
* This is used for refs on host components.
*/
Expand Down Expand Up @@ -135,7 +139,7 @@ const ReactFabricRenderer = ReactFiberReconciler({
type: string,
props: Props,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
internalInstanceHandle: Object,
): Instance {
const tag = nextReactTag;
Expand All @@ -151,6 +155,11 @@ const ReactFabricRenderer = ReactFiberReconciler({
}
}

invariant(
type !== 'RCTView' || !hostContext.isInAParentText,
'Nesting of <View> within <Text> is not currently supported.',
);

const updatePayload = ReactNativeAttributePayload.create(
props,
viewConfig.validAttributes,
Expand All @@ -175,9 +184,14 @@ const ReactFabricRenderer = ReactFiberReconciler({
createTextInstance(
text: string,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
internalInstanceHandle: Object,
): TextInstance {
invariant(
hostContext.isInAParentText,
'Text strings must be rendered within a <Text> component.',
);

const tag = nextReactTag;
nextReactTag += 2;

Expand All @@ -203,12 +217,27 @@ const ReactFabricRenderer = ReactFiberReconciler({
return false;
},

getRootHostContext(): {} {
return emptyObject;
getRootHostContext(rootContainerInstance: Container): HostContext {
return {isInAParentText: false};
},

getChildHostContext(): {} {
return emptyObject;
getChildHostContext(
parentHostContext: HostContext,
type: string,
): HostContext {
const prevIsInAParentText = parentHostContext.isInAParentText;
const isInAParentText =
type === 'AndroidTextInput' || // Android
type === 'RCTMultilineTextInputView' || // iOS
type === 'RCTSinglelineTextInputView' || // iOS
type === 'RCTText' ||
type === 'RCTVirtualText';

if (prevIsInAParentText !== isInAParentText) {
return {isInAParentText};
} else {
return parentHostContext;
}
},

getPublicInstance(instance) {
Expand All @@ -227,7 +256,7 @@ const ReactFabricRenderer = ReactFiberReconciler({
oldProps: Props,
newProps: Props,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
): null | Object {
const viewConfig = instance.canonical.viewConfig;
const updatePayload = ReactNativeAttributePayload.diff(
Expand Down
44 changes: 37 additions & 7 deletions packages/react-native-renderer/src/ReactNativeFiberRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {ReactNativeBaseComponentViewConfig} from './ReactNativeTypes';
import ReactFiberReconciler from 'react-reconciler';
import emptyObject from 'fbjs/lib/emptyObject';
import invariant from 'fbjs/lib/invariant';

// Modules provided by RN:
import UIManager from 'UIManager';
import deepFreezeAndThrowOnMutationInDev from 'deepFreezeAndThrowOnMutationInDev';
Expand All @@ -35,6 +36,10 @@ export type Instance = {
type Props = Object;
type TextInstance = number;

type HostContext = $ReadOnly<{|
isInAParentText: boolean,
|}>;

// Counter for uniquely identifying views.
// % 10 === 1 means it is a rootTag.
// % 2 === 0 means it is a Fabric tag.
Expand Down Expand Up @@ -71,7 +76,7 @@ const NativeRenderer = ReactFiberReconciler({
type: string,
props: Props,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
internalInstanceHandle: Object,
): Instance {
const tag = allocateTag();
Expand All @@ -85,6 +90,11 @@ const NativeRenderer = ReactFiberReconciler({
}
}

invariant(
type !== 'RCTView' || !hostContext.isInAParentText,
'Nesting of <View> within <Text> is not currently supported.',
);

const updatePayload = ReactNativeAttributePayload.create(
props,
viewConfig.validAttributes,
Expand All @@ -110,9 +120,14 @@ const NativeRenderer = ReactFiberReconciler({
createTextInstance(
text: string,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
internalInstanceHandle: Object,
): TextInstance {
invariant(
hostContext.isInAParentText,
'Text strings must be rendered within a <Text> component.',
);

const tag = allocateTag();

UIManager.createView(
Expand Down Expand Up @@ -155,12 +170,27 @@ const NativeRenderer = ReactFiberReconciler({
return false;
},

getRootHostContext(): {} {
return emptyObject;
getRootHostContext(rootContainerInstance: Container): HostContext {
return {isInAParentText: false};
},

getChildHostContext(): {} {
return emptyObject;
getChildHostContext(
parentHostContext: HostContext,
type: string,
): HostContext {
const prevIsInAParentText = parentHostContext.isInAParentText;
const isInAParentText =
type === 'AndroidTextInput' || // Android
type === 'RCTMultilineTextInputView' || // iOS
type === 'RCTSinglelineTextInputView' || // iOS
type === 'RCTText' ||
type === 'RCTVirtualText';

if (prevIsInAParentText !== isInAParentText) {
return {isInAParentText};
} else {
return parentHostContext;
}
},

getPublicInstance(instance) {
Expand All @@ -179,7 +209,7 @@ const NativeRenderer = ReactFiberReconciler({
oldProps: Props,
newProps: Props,
rootContainerInstance: Container,
hostContext: {},
hostContext: HostContext,
): null | Object {
return emptyObject;
},
Expand Down
Loading