From 4436dea64e310c3136cf1129b89278e67875874a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 3 Apr 2019 00:55:31 +0100 Subject: [PATCH 1/9] Add implementation for TouchHitTarget to ReactDOM --- packages/react-art/src/ReactARTHostConfig.js | 10 ++-- .../react-dom/src/client/ReactDOMComponent.js | 34 ++++++++++++++ .../src/client/ReactDOMComponentTree.js | 20 ++++++-- .../src/client/ReactDOMHostConfig.js | 44 +++++++++++++---- .../src/ReactFabricHostConfig.js | 10 ++-- .../src/ReactNativeHostConfig.js | 10 ++-- .../src/createReactNoop.js | 11 ++--- .../src/ReactFiberCompleteWork.js | 47 ++++++++++++++----- .../src/forks/ReactFiberHostConfig.custom.js | 3 +- .../src/ReactTestHostConfig.js | 14 +++--- 10 files changed, 151 insertions(+), 52 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index 34769341659..fe268370c7c 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -447,11 +447,13 @@ export function handleEventComponent( // TODO: add handleEventComponent implementation } -export function handleEventTarget( - type: Symbol | number, - props: Props, +export function createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, ) { - // TODO: add handleEventTarget implementation + // TODO: add createTouchHitTargetInstance implementation + return (null: any); } diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index bf5508c72cb..ddf02b1fd36 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -98,6 +98,7 @@ const AUTOFOCUS = 'autoFocus'; const CHILDREN = 'children'; const STYLE = 'style'; const HTML = '__html'; +const EMPTY_OBJECT = {}; const {html: HTML_NAMESPACE} = Namespaces; @@ -1339,6 +1340,39 @@ export function listenToEventResponderEventTypes( } } +export function createTouchHitTargetElement( + bottom: number | void | null, + left: number | void | null, + right: number | void | null, + top: number | void | null, + rootContainerInstance: Element | Document, + parentNamespace: string, +): HTMLElement { + const touchHitTargetElement = ((createElement( + 'div', + EMPTY_OBJECT, + rootContainerInstance, + parentNamespace, + ): any): HTMLElement); + const touchHitTargetElementStyle = touchHitTargetElement.style; + + touchHitTargetElementStyle.position = 'absolute'; + if (top != null) { + touchHitTargetElementStyle.top = `-${top}px`; + } + if (left != null) { + touchHitTargetElementStyle.left = `-${left}px`; + } + if (right != null) { + touchHitTargetElementStyle.right = `-${right}px`; + } + if (bottom != null) { + touchHitTargetElementStyle.bottom = `-${bottom}px`; + } + + return touchHitTargetElement; +} + // We can remove this once the event API is stable and out of a flag if (enableEventAPI) { setListenToResponderEventTypes(listenToEventResponderEventTypes); diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index a3b61d6e49e..52cd9602432 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {HostComponent, HostText} from 'shared/ReactWorkTags'; +import {HostComponent, HostText, EventTarget} from 'shared/ReactWorkTags'; import invariant from 'shared/invariant'; const randomKey = Math.random() @@ -38,7 +38,11 @@ export function getClosestInstanceFromNode(node) { } let inst = node[internalInstanceKey]; - if (inst.tag === HostComponent || inst.tag === HostText) { + if ( + inst.tag === HostComponent || + inst.tag === HostText || + inst.tag === EventTarget + ) { // In Fiber, this will always be the deepest root. return inst; } @@ -53,7 +57,11 @@ export function getClosestInstanceFromNode(node) { export function getInstanceFromNode(node) { const inst = node[internalInstanceKey]; if (inst) { - if (inst.tag === HostComponent || inst.tag === HostText) { + if ( + inst.tag === HostComponent || + inst.tag === HostText || + inst.tag === EventTarget + ) { return inst; } else { return null; @@ -67,7 +75,11 @@ export function getInstanceFromNode(node) { * DOM node. */ export function getNodeFromInstance(inst) { - if (inst.tag === HostComponent || inst.tag === HostText) { + if ( + inst.tag === HostComponent || + inst.tag === HostText || + inst.tag === EventTarget + ) { // In Fiber this, is just the state node right now. We assume it will be // a host component or host text. return inst.stateNode; diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index ef96ee8f944..e8b1de8b333 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -25,6 +25,7 @@ import { warnForInsertedHydratedElement, warnForInsertedHydratedText, listenToEventResponderEventTypes, + createTouchHitTargetElement, } from './ReactDOMComponent'; import {getSelectionInformation, restoreSelection} from './ReactInputSelection'; import setTextContent from './setTextContent'; @@ -33,7 +34,7 @@ import { isEnabled as ReactBrowserEventEmitterIsEnabled, setEnabled as ReactBrowserEventEmitterSetEnabled, } from '../events/ReactBrowserEventEmitter'; -import {getChildNamespace} from '../shared/DOMNamespaces'; +import {Namespaces, getChildNamespace} from '../shared/DOMNamespaces'; import { ELEMENT_NODE, TEXT_NODE, @@ -57,6 +58,10 @@ export type Props = { style?: { display?: string, }, + bottom?: null | number, + left?: null | number, + right?: null | number, + top?: null | number, }; export type Container = Element | Document; export type Instance = Element; @@ -86,6 +91,8 @@ import { } from 'shared/ReactFeatureFlags'; import warning from 'shared/warning'; +const {html: HTML_NAMESPACE} = Namespaces; + // Intentionally not named imports because Rollup would // use dynamic dispatch for CommonJS interop named imports. const { @@ -899,16 +906,33 @@ export function handleEventComponent( } } -export function handleEventTarget( - type: Symbol | number, - props: Props, +export function createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, -): void { - if (enableEventAPI) { - // Touch target hit slop handling - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - // TODO - } +): Instance { + const {bottom, left, right, top} = props; + let parentNamespace: string; + if (__DEV__) { + const hostContextDev = ((hostContext: any): HostContextDev); + parentNamespace = hostContextDev.namespace; + throw new Error( + ' was used in an unsupported DOM namespace. ' + + 'Ensure the is used in an HTML namespace.', + ); + } else { + parentNamespace = ((hostContext: any): HostContextProd); } + const touchHitTargetInstance = createTouchHitTargetElement( + bottom, + left, + right, + top, + rootContainerInstance, + parentNamespace, + ); + precacheFiberNode(internalInstanceHandle, touchHitTargetInstance); + return touchHitTargetInstance; } diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index c2ccde544cb..dc3b82dbb54 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -442,11 +442,13 @@ export function handleEventComponent( // TODO: add handleEventComponent implementation } -export function handleEventTarget( - type: Symbol | number, - props: Props, +export function createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, ) { - // TODO: add handleEventTarget implementation + // TODO: add createTouchHitTargetInstance implementation + return (null: any); } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index f4b24a1c39f..2683031faab 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -501,11 +501,13 @@ export function handleEventComponent( // TODO: add handleEventComponent implementation } -export function handleEventTarget( - type: Symbol | number, - props: Props, +export function createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, ) { - // TODO: add handleEventTarget implementation + // TODO: add createTouchHitTargetInstance implementation + return (null: any); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 8976372bf6c..4b56db94229 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -432,15 +432,14 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // NO-OP }, - handleEventTarget( - type: Symbol | number, - props: Props, + createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, ) { - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - // TODO - } + // TODO: add createTouchHitTargetInstance implementation }, }; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index abd125d3acb..26999dc79d5 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -66,7 +66,7 @@ import { appendChildToContainerChildSet, finalizeContainerChildren, handleEventComponent, - handleEventTarget, + createTouchHitTargetInstance, } from './ReactFiberHostConfig'; import { getRootHostContainer, @@ -90,6 +90,7 @@ import { enableSuspenseServerRenderer, enableEventAPI, } from 'shared/ReactFeatureFlags'; +import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -784,18 +785,40 @@ function completeWork( if (enableEventAPI) { popHostContext(workInProgress); const type = workInProgress.type.type; - let node = workInProgress.return; - let parentHostInstance = null; - // Traverse up the fiber tree till we find a host component fiber - while (node !== null) { - if (node.tag === HostComponent) { - parentHostInstance = node.stateNode; - break; + + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + let node = workInProgress.return; + let parentHostInstance = null; + // Traverse up the fiber tree till we find a host component fiber + while (node !== null) { + if (node.tag === HostComponent) { + parentHostInstance = node.stateNode; + break; + } + node = node.return; + } + invariant( + parentHostInstance !== null, + 'A touch hit target was completed without a parent host component node. ' + + 'This is probably a bug in React.', + ); + + if (current !== null && workInProgress.stateNode != null) { + // Update + } else { + const currentHostContext = getHostContext(); + const rootContainerInstance = getRootHostContainer(); + const instance = createTouchHitTargetInstance( + parentHostInstance, + newProps, + rootContainerInstance, + currentHostContext, + workInProgress, + ); + + appendAllChildren(instance, workInProgress, false, false); + workInProgress.stateNode = instance; } - node = node.return; - } - if (parentHostInstance !== null) { - handleEventTarget(type, newProps, parentHostInstance, workInProgress); } } break; diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index d1f38c65d22..d4a4be27c3f 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -64,7 +64,8 @@ export const supportsMutation = $$$hostConfig.supportsMutation; export const supportsPersistence = $$$hostConfig.supportsPersistence; export const supportsHydration = $$$hostConfig.supportsHydration; export const handleEventComponent = $$$hostConfig.handleEventComponent; -export const handleEventTarget = $$$hostConfig.handleEventTarget; +export const createTouchHitTargetInstance = + $$$hostConfig.createTouchHitTargetInstance; // ------------------- // Mutation diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 8020282394c..3d7054e79c4 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -333,13 +333,13 @@ export function handleEventComponent( // TODO: add handleEventComponent implementation } -export function handleEventTarget( - type: Symbol | number, - props: Props, +export function createTouchHitTargetInstance( parentInstance: Container, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, -) { - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - // TODO - } +): Instance { + // TODO: add createTouchHitTargetInstance implementation + return (null: any); } From 32b70b32d8df9f45a907685638fc8d8fde86313f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 3 Apr 2019 14:21:15 +0100 Subject: [PATCH 2/9] Adds commit phase effects for touch hit targets --- packages/react-art/src/ReactARTHostConfig.js | 14 +- .../react-dom/src/client/ReactDOMComponent.js | 44 ++++- .../src/client/ReactDOMHostConfig.js | 45 +++-- .../src/ReactFabricHostConfig.js | 39 +++- .../src/ReactNativeHostConfig.js | 24 ++- .../src/createReactNoop.js | 20 ++- .../src/ReactFiberCommitWork.js | 43 +++++ .../src/ReactFiberCompleteWork.js | 168 +++++++++++++++++- .../src/forks/ReactFiberHostConfig.custom.js | 4 + .../src/ReactTestHostConfig.js | 26 ++- .../shared/HostConfigWithNoPersistence.js | 1 + 11 files changed, 379 insertions(+), 49 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index fe268370c7c..dded3ecd295 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -443,17 +443,19 @@ export function handleEventComponent( eventResponder: ReactEventResponder, rootContainerInstance: Container, internalInstanceHandle: Object, -) { - // TODO: add handleEventComponent implementation +): void { + throw new Error('Not yet implemented.'); } export function createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, -) { - // TODO: add createTouchHitTargetInstance implementation - return (null: any); +): Instance { + throw new Error('Not yet implemented.'); } diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index ddf02b1fd36..3b2e16d7397 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -1341,10 +1341,10 @@ export function listenToEventResponderEventTypes( } export function createTouchHitTargetElement( - bottom: number | void | null, - left: number | void | null, - right: number | void | null, - top: number | void | null, + bottom: number, + left: number, + right: number, + top: number, rootContainerInstance: Element | Document, parentNamespace: string, ): HTMLElement { @@ -1357,22 +1357,50 @@ export function createTouchHitTargetElement( const touchHitTargetElementStyle = touchHitTargetElement.style; touchHitTargetElementStyle.position = 'absolute'; - if (top != null) { + if (top !== 0) { touchHitTargetElementStyle.top = `-${top}px`; } - if (left != null) { + if (left !== 0) { touchHitTargetElementStyle.left = `-${left}px`; } - if (right != null) { + if (right !== 0) { touchHitTargetElementStyle.right = `-${right}px`; } - if (bottom != null) { + if (bottom !== 0) { touchHitTargetElementStyle.bottom = `-${bottom}px`; } return touchHitTargetElement; } +export function updateTouchHitTargetElement( + oldBottom: number, + oldLeft: number, + oldRight: number, + oldTop: number, + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + touchHitTargetElement: Element, +) { + const touchHitTargetElementStyle = ((touchHitTargetElement: any): HTMLElement) + .style; + + if (oldTop !== newTop) { + touchHitTargetElementStyle.top = `-${newTop}px`; + } + if (oldLeft !== newLeft) { + touchHitTargetElementStyle.left = `-${newLeft}px`; + } + if (oldRight !== newRight) { + touchHitTargetElementStyle.right = `-${newRight}px`; + } + if (oldBottom !== newBottom) { + touchHitTargetElementStyle.bottom = `-${newBottom}px`; + } +} + // We can remove this once the event API is stable and out of a flag if (enableEventAPI) { setListenToResponderEventTypes(listenToEventResponderEventTypes); diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index e8b1de8b333..ba08a71bad3 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -26,6 +26,7 @@ import { warnForInsertedHydratedText, listenToEventResponderEventTypes, createTouchHitTargetElement, + updateTouchHitTargetElement, } from './ReactDOMComponent'; import {getSelectionInformation, restoreSelection} from './ReactInputSelection'; import setTextContent from './setTextContent'; @@ -58,10 +59,6 @@ export type Props = { style?: { display?: string, }, - bottom?: null | number, - left?: null | number, - right?: null | number, - top?: null | number, }; export type Container = Element | Document; export type Instance = Element; @@ -907,21 +904,25 @@ export function handleEventComponent( } export function createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, ): Instance { - const {bottom, left, right, top} = props; let parentNamespace: string; if (__DEV__) { const hostContextDev = ((hostContext: any): HostContextDev); parentNamespace = hostContextDev.namespace; - throw new Error( - ' was used in an unsupported DOM namespace. ' + - 'Ensure the is used in an HTML namespace.', - ); + if (parentNamespace !== HTML_NAMESPACE) { + throw new Error( + ' was used in an unsupported DOM namespace. ' + + 'Ensure the is used in an HTML namespace.', + ); + } } else { parentNamespace = ((hostContext: any): HostContextProd); } @@ -936,3 +937,27 @@ export function createTouchHitTargetInstance( precacheFiberNode(internalInstanceHandle, touchHitTargetInstance); return touchHitTargetInstance; } + +export function commitTouchHitTargetUpdate( + oldBottom: number, + oldLeft: number, + oldRight: number, + oldTop: number, + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + touchHitTargetInstance: Instance, +): void { + updateTouchHitTargetElement( + oldBottom, + oldLeft, + oldRight, + oldTop, + newBottom, + newLeft, + newRight, + newTop, + touchHitTargetInstance, + ); +} diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index dc3b82dbb54..7be9c997e9c 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -411,6 +411,17 @@ export function cloneHiddenTextInstance( throw new Error('Not yet implemented.'); } +export function cloneHiddenTouchHitTargetInstance( + instance: Instance, + bottom: number, + left: number, + right: number, + top: number, + internalInstanceHandle: Object, +) { + throw new Error('Not yet implemented.'); +} + export function createContainerChildSet(container: Container): ChildSet { return createChildNodeSet(container); } @@ -438,17 +449,33 @@ export function handleEventComponent( eventResponder: ReactEventResponder, rootContainerInstance: Container, internalInstanceHandle: Object, -) { - // TODO: add handleEventComponent implementation +): void { + throw new Error('Not yet implemented.'); } export function createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, -) { - // TODO: add createTouchHitTargetInstance implementation - return (null: any); +): Instance { + throw new Error('Not yet implemented.'); +} + +export function commitTouchHitTargetUpdate( + oldBottom: number, + oldLeft: number, + oldRight: number, + oldTop: number, + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + touchHitTargetInstance: Instance, +): void { + throw new Error('Not yet implemented.'); } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 2683031faab..0512b32daed 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -502,12 +502,28 @@ export function handleEventComponent( } export function createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, -) { - // TODO: add createTouchHitTargetInstance implementation - return (null: any); +): Instance { + throw new Error('Not yet implemented.'); +} + +export function commitTouchHitTargetUpdate( + oldBottom: number, + oldLeft: number, + oldRight: number, + oldTop: number, + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + touchHitTargetInstance: Instance, +): void { + throw new Error('Not yet implemented.'); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 44eb54b7f69..f5ee099313f 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -432,13 +432,16 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, - ) { - // TODO: add createTouchHitTargetInstance implementation + ): Instance { + throw new Error('Not yet implemented.'); }, }; @@ -588,6 +591,17 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }); return clone; }, + + cloneHiddenTouchHitTargetInstance( + instance: Instance, + bottom: number, + left: number, + right: number, + top: number, + internalInstanceHandle: Object, + ) { + throw new Error('Not yet implemented.'); + }, }; const NoopRenderer = reconciler(hostConfig); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index a3e479c20ad..9f4bda1631a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -28,6 +28,7 @@ import { enableSchedulerTracing, enableProfilerTimer, enableSuspenseServerRenderer, + enableEventAPI, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -43,6 +44,7 @@ import { IncompleteClassComponent, MemoComponent, SimpleMemoComponent, + EventTarget, } from 'shared/ReactWorkTags'; import { invokeGuardedCallback, @@ -90,6 +92,7 @@ import { hideTextInstance, unhideInstance, unhideTextInstance, + commitTouchHitTargetUpdate, } from './ReactFiberHostConfig'; import { captureCommitPhaseError, @@ -107,6 +110,7 @@ import { MountPassive, } from './ReactHookEffectTags'; import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; +import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -1195,6 +1199,45 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { commitTextUpdate(textInstance, oldText, newText); return; } + case EventTarget: { + if (enableEventAPI) { + const type = finishedWork.type.type; + + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + invariant( + finishedWork.stateNode !== null, + 'This should have a touch hit target element initialized. This error is likely ' + + 'caused by a bug in React. Please file an issue.', + ); + const instance: Instance = finishedWork.stateNode; + const newProps = finishedWork.memoizedProps; + // For hydration we reuse the update path but we treat the oldProps + // as the newProps. The updatePayload will contain the real change in + // this case. + const oldProps = current !== null ? current.memoizedProps : newProps; + const oldBottom = oldProps.bottom || 0; + const oldLeft = oldProps.left || 0; + const oldRight = oldProps.right || 0; + const oldTop = oldProps.top || 0; + const newBottom = newProps.bottom || 0; + const newLeft = newProps.left || 0; + const newRight = newProps.right || 0; + const newTop = newProps.top || 0; + commitTouchHitTargetUpdate( + oldBottom, + oldLeft, + oldRight, + oldTop, + newBottom, + newLeft, + newRight, + newTop, + instance, + ); + } + } + return; + } case HostRoot: { return; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 26999dc79d5..47de828251d 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -67,6 +67,7 @@ import { finalizeContainerChildren, handleEventComponent, createTouchHitTargetInstance, + cloneHiddenTouchHitTargetInstance, } from './ReactFiberHostConfig'; import { getRootHostContainer, @@ -106,6 +107,7 @@ let appendAllChildren; let updateHostContainer; let updateHostComponent; let updateHostText; +let updateTouchHitTarget; if (supportsMutation) { // Mutation mode @@ -119,7 +121,13 @@ if (supportsMutation) { // children to find all the terminal nodes. let node = workInProgress.child; while (node !== null) { - if (node.tag === HostComponent || node.tag === HostText) { + if ( + node.tag === HostComponent || + node.tag === HostText || + (enableEventAPI && + node.tag === EventTarget && + node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) + ) { appendInitialChild(parent, node.stateNode); } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse @@ -199,6 +207,33 @@ if (supportsMutation) { markUpdate(workInProgress); } }; + updateTouchHitTarget = function( + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + current: Fiber, + workInProgress: Fiber, + parentHostInstance: Container, + ) { + if (enableEventAPI) { + const oldProps = current.memoizedProps; + const oldBottom = oldProps.bottom || 0; + const oldLeft = oldProps.left || 0; + const oldRight = oldProps.right || 0; + const oldTop = oldProps.top || 0; + + // If the hit slop values differ, mark it as an update. All the work in done in commitWork. + if ( + oldBottom !== newBottom || + oldLeft !== newLeft || + oldRight !== newRight || + oldTop !== newTop + ) { + markUpdate(workInProgress); + } + } + }; } else if (supportsPersistence) { // Persistent host tree mode @@ -230,6 +265,29 @@ if (supportsMutation) { instance = cloneHiddenTextInstance(instance, text, node); } appendInitialChild(parent, instance); + } else if ( + enableEventAPI && + node.tag === EventTarget && + node.type.type === REACT_EVENT_TARGET_TOUCH_HIT + ) { + let instance = node.stateNode; + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. + const props = node.memoizedProps; + const bottom = props.bottom || 0; + const left = props.left || 0; + const right = props.right || 0; + const top = props.top || 0; + instance = cloneHiddenTouchHitTargetInstance( + instance, + bottom, + left, + right, + top, + node, + ); + } + appendInitialChild(parent, instance); } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -315,6 +373,29 @@ if (supportsMutation) { instance = cloneHiddenTextInstance(instance, text, node); } appendChildToContainerChildSet(containerChildSet, instance); + } else if ( + enableEventAPI && + node.tag === EventTarget && + node.type.type === REACT_EVENT_TARGET_TOUCH_HIT + ) { + let instance = node.stateNode; + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. + const props = node.memoizedProps; + const bottom = props.bottom || 0; + const left = props.left || 0; + const right = props.right || 0; + const top = props.top || 0; + instance = cloneHiddenTouchHitTargetInstance( + instance, + bottom, + left, + right, + top, + node, + ); + } + appendChildToContainerChildSet(containerChildSet, instance); } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -480,6 +561,46 @@ if (supportsMutation) { markUpdate(workInProgress); } }; + updateTouchHitTarget = function( + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + current: Fiber, + workInProgress: Fiber, + parentHostInstance: Container, + ) { + if (enableEventAPI) { + const oldProps = current.memoizedProps; + const oldBottom = oldProps.bottom || 0; + const oldLeft = oldProps.left || 0; + const oldRight = oldProps.right || 0; + const oldTop = oldProps.top || 0; + + if ( + oldBottom !== newBottom || + oldLeft !== newLeft || + oldRight !== newRight || + oldTop !== newTop + ) { + const currentHostContext = getHostContext(); + const rootContainerInstance = getRootHostContainer(); + workInProgress.stateNode = createTouchHitTargetInstance( + newBottom, + newLeft, + newRight, + newTop, + parentHostInstance, + rootContainerInstance, + currentHostContext, + workInProgress, + ); + // We'll have to mark it as having an effect, even though we won't use the effect for anything. + // This lets the parents know that at least one of their children has changed. + markUpdate(workInProgress); + } + } + }; } else { // No host operations updateHostContainer = function(workInProgress: Fiber) { @@ -502,6 +623,17 @@ if (supportsMutation) { ) { // Noop }; + updateTouchHitTarget = function( + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + current: Fiber, + workInProgress: Fiber, + parentHostInstance: Container, + ) { + // Noop + }; } function completeWork( @@ -802,22 +934,44 @@ function completeWork( 'A touch hit target was completed without a parent host component node. ' + 'This is probably a bug in React.', ); + const newBottom = newProps.bottom || 0; + const newLeft = newProps.left || 0; + const newRight = newProps.right || 0; + const newTop = newProps.top || 0; if (current !== null && workInProgress.stateNode != null) { - // Update + // If we have an alternate, that means this is an update and we need + // to schedule a side-effect to do the updates. + updateTouchHitTarget( + newBottom, + newLeft, + newRight, + newTop, + current, + workInProgress, + parentHostInstance, + ); } else { + if ( + newBottom === 0 && + newLeft === 0 && + newRight === 0 && + newTop === 0 + ) { + return null; + } const currentHostContext = getHostContext(); const rootContainerInstance = getRootHostContainer(); - const instance = createTouchHitTargetInstance( + workInProgress.stateNode = createTouchHitTargetInstance( + newBottom, + newLeft, + newRight, + newTop, parentHostInstance, - newProps, rootContainerInstance, currentHostContext, workInProgress, ); - - appendAllChildren(instance, workInProgress, false, false); - workInProgress.stateNode = instance; } } } diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index d4a4be27c3f..b6be6bf9831 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -85,6 +85,8 @@ export const hideInstance = $$$hostConfig.hideInstance; export const hideTextInstance = $$$hostConfig.hideTextInstance; export const unhideInstance = $$$hostConfig.unhideInstance; export const unhideTextInstance = $$$hostConfig.unhideTextInstance; +export const commitTouchHitTargetUpdate = + $$$hostConfig.commitTouchHitTargetUpdate; // ------------------- // Persistence @@ -99,6 +101,8 @@ export const finalizeContainerChildren = export const replaceContainerChildren = $$$hostConfig.replaceContainerChildren; export const cloneHiddenInstance = $$$hostConfig.cloneHiddenInstance; export const cloneHiddenTextInstance = $$$hostConfig.cloneHiddenTextInstance; +export const cloneHiddenTouchHitTargetInstance = + $$$hostConfig.cloneHiddenTouchHitTargetInstance; // ------------------- // Hydration diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 3d7054e79c4..9e4240fa155 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -329,17 +329,33 @@ export function handleEventComponent( eventResponder: ReactEventResponder, rootContainerInstance: Container, internalInstanceHandle: Object, -) { - // TODO: add handleEventComponent implementation +): void { + throw new Error('Not yet implemented.'); } export function createTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, parentInstance: Container, - props: Props, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, ): Instance { - // TODO: add createTouchHitTargetInstance implementation - return (null: any); + throw new Error('Not yet implemented.'); +} + +export function commitTouchHitTargetUpdate( + oldBottom: number, + oldLeft: number, + oldRight: number, + oldTop: number, + newBottom: number, + newLeft: number, + newRight: number, + newTop: number, + touchHitTargetInstance: Instance, +): void { + throw new Error('Not yet implemented.'); } diff --git a/packages/shared/HostConfigWithNoPersistence.js b/packages/shared/HostConfigWithNoPersistence.js index d5f84cf43fd..9646c6a11f4 100644 --- a/packages/shared/HostConfigWithNoPersistence.js +++ b/packages/shared/HostConfigWithNoPersistence.js @@ -30,3 +30,4 @@ export const finalizeContainerChildren = shim; export const replaceContainerChildren = shim; export const cloneHiddenInstance = shim; export const cloneHiddenTextInstance = shim; +export const cloneHiddenTouchHitTargetInstance = shim; From 4b05730303869d1b269f2eee7111f648a284ce1a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 3 Apr 2019 17:33:35 +0100 Subject: [PATCH 3/9] Adds hydration support --- .../src/client/ReactDOMHostConfig.js | 40 ++- .../src/events/DOMEventResponderSystem.js | 39 ++- .../src/server/ReactPartialRenderer.js | 21 ++ .../__tests__/TouchHitTarget-test.internal.js | 316 ++++++++++++++++++ .../src/ReactFabricHostConfig.js | 2 +- .../src/ReactNativeHostConfig.js | 2 +- .../src/createReactNoop.js | 13 +- .../src/ReactFiberBeginWork.js | 10 +- .../src/ReactFiberCommitWork.js | 61 +++- .../src/ReactFiberCompleteWork.js | 80 +++-- .../src/ReactFiberHydrationContext.js | 43 ++- .../src/forks/ReactFiberHostConfig.custom.js | 4 + .../src/ReactTestHostConfig.js | 54 ++- packages/shared/HostConfigWithNoHydration.js | 2 + 14 files changed, 618 insertions(+), 69 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index ba08a71bad3..3d0d02e1b84 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -48,6 +48,7 @@ import dangerousStyleValue from '../shared/dangerousStyleValue'; import type {DOMContainer} from './ReactDOM'; import type {ReactEventResponder} from 'shared/ReactTypes'; import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; +import {canUseDOM} from 'shared/ExecutionEnvironment'; export type Type = string; export type Props = { @@ -908,7 +909,6 @@ export function createTouchHitTargetInstance( left: number, right: number, top: number, - parentInstance: Container, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, @@ -948,7 +948,21 @@ export function commitTouchHitTargetUpdate( newRight: number, newTop: number, touchHitTargetInstance: Instance, + parentInstance: null | Container, ): void { + if (__DEV__ && canUseDOM && parentInstance !== null) { + // This is done at DEV time because getComputedStyle will + // typically force a style recalculation and force a layout, + // reflow -– both of which are sync are expensive. + const computedStyles = window.getComputedStyle(parentInstance); + warning( + computedStyles.getPropertyValue('position') !== 'static', + ' inserts an empty absolutely positioned
. ' + + 'This requires its parent DOM node to be positioned too, but the ' + + 'parent DOM node was found to have the style "position" set to ' + + 'value "static". Try using a "position" value of "relative".', + ); + } updateTouchHitTargetElement( oldBottom, oldLeft, @@ -961,3 +975,27 @@ export function commitTouchHitTargetUpdate( touchHitTargetInstance, ); } + +export function hydrateTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, + touchHitTargetInstance: Instance, +): boolean { + // Rather than do computed checks on the position styles, we always flag + // the hit target instance as needing to update from hydration. + return true; +} + +export function canHydrateTouchHitTargetInstance(instance: HydratableInstance) { + // The touch hit target element is always a
. + if ( + instance.nodeType !== ELEMENT_NODE || + instance.nodeName.toLowerCase() !== 'div' + ) { + return null; + } + // This has now been refined to an element node. + return ((instance: any): Instance); +} diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 45e6c95e463..7e08215956f 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -12,7 +12,10 @@ import { PASSIVE_NOT_SUPPORTED, } from 'events/EventSystemFlags'; import type {AnyNativeEvent} from 'events/PluginModuleType'; -import {EventComponent} from 'shared/ReactWorkTags'; +import { + EventComponent, + EventTarget as EventTargetWorkTag, +} from 'shared/ReactWorkTags'; import type { ReactEventResponder, ReactEventResponderEventType, @@ -274,8 +277,38 @@ DOMEventResponderContext.prototype.removeRootEventTypes = function( } }; -DOMEventResponderContext.prototype.isPositionWithinTouchHitTarget = function() { - // TODO +DOMEventResponderContext.prototype.isPositionWithinTouchHitTarget = function( + x: number, + y: number, +): boolean { + const doc = this.eventTarget.ownerDocument; + // This isn't available in some environments (JSDOM) + if (typeof doc.elementFromPoint !== 'function') { + return false; + } + const target = doc.elementFromPoint(x, y); + if (target === null) { + return false; + } + const childFiber = getClosestInstanceFromNode(target); + if (childFiber === null) { + return false; + } + if (childFiber.tag === EventTargetWorkTag) { + // TODO find another way to do this without using the + // expensive getBoundingClientRect. + const { + left, + top, + right, + bottom, + } = target.parentNode.getBoundingClientRect(); + if (x > left && y > top && x < right && y < bottom) { + return false; + } + return true; + } + return false; }; DOMEventResponderContext.prototype.isTargetOwned = function( diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index 7eecd8dd41b..7d5bd001e02 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -39,6 +39,7 @@ import { REACT_MEMO_TYPE, REACT_EVENT_COMPONENT_TYPE, REACT_EVENT_TARGET_TYPE, + REACT_EVENT_TARGET_TOUCH_HIT, } from 'shared/ReactSymbols'; import { @@ -1168,6 +1169,26 @@ class ReactDOMServerRenderer { case REACT_EVENT_COMPONENT_TYPE: case REACT_EVENT_TARGET_TYPE: { if (enableEventAPI) { + if ( + elementType.$$typeof === REACT_EVENT_TARGET_TYPE && + elementType.type === REACT_EVENT_TARGET_TOUCH_HIT + ) { + const props = nextElement.props; + const bottom = props.bottom || 0; + const left = props.left || 0; + const right = props.right || 0; + const top = props.top || 0; + + if (bottom === 0 && left === 0 && right === 0 && top === 0) { + return ''; + } + let topString = top ? `-${top}px;` : ''; + let leftString = left ? `-${left}px;` : ''; + let rightString = right ? `-${right}px;` : ''; + let bottomString = bottom ? `-${bottom}px;` : ''; + + return `
`; + } const nextChildren = toArray( ((nextChild: any): ReactElement).props.children, ); diff --git a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js index e47f7f3cb47..34e2c5d52f9 100644 --- a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js +++ b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js @@ -16,6 +16,7 @@ let ReactFeatureFlags; let EventComponent; let ReactTestRenderer; let ReactDOM; +let ReactDOMServer; let ReactSymbols; let ReactEvents; let TouchHitTarget; @@ -58,6 +59,11 @@ function initReactDOM() { ReactDOM = require('react-dom'); } +function initReactDOMServer() { + init(); + ReactDOMServer = require('react-dom/server'); +} + describe('TouchHitTarget', () => { describe('NoopRenderer', () => { beforeEach(() => { @@ -318,5 +324,315 @@ describe('TouchHitTarget', () => { 'Ensure is a direct child of a DOM element.', ); }); + + it('should render a conditional TouchHitTarget correctly (false -> true)', () => { + let cond = false; + + const Test = () => ( + +
+ {cond ? null : ( + + )} +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
', + ); + + cond = true; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe('
'); + }); + + it('should render a conditional TouchHitTarget correctly (true -> false)', () => { + let cond = true; + + const Test = () => ( + +
+ {cond ? null : ( + + )} +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe('
'); + + cond = false; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
', + ); + }); + + it('should render a conditional TouchHitTarget hit slop correctly (false -> true)', () => { + let cond = false; + + const Test = () => ( + +
+ {cond ? ( + + ) : ( + + )} +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
', + ); + + cond = true; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
', + ); + }); + + it('should render a conditional TouchHitTarget hit slop correctly (true -> false)', () => { + let cond = true; + + const Test = () => ( + +
+ Random span 1 + {cond ? ( + + ) : ( + + )} + Random span 2 +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1Random span 2
', + ); + + cond = false; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1
Random span 2
', + ); + }); + + it('should update TouchHitTarget hit slop values correctly (false -> true)', () => { + let cond = false; + + const Test = () => ( + +
+ Random span 1 + {cond ? ( + + ) : ( + + )} + Random span 2 +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1
Random span 2
', + ); + + cond = true; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1
Random span 2
', + ); + }); + + it('should update TouchHitTarget hit slop values correctly (true -> false)', () => { + let cond = true; + + const Test = () => ( + +
+ Random span 1 + {cond ? ( + + ) : ( + + )} + Random span 2 +
+
+ ); + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1
Random span 2
', + ); + + cond = false; + ReactDOM.render(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
Random span 1
Random span 2
', + ); + }); + + it('should hydrate TouchHitTarget hit slop elements correcty', () => { + const Test = () => ( + +
+ +
+
+ ); + + const container = document.createElement('div'); + container.innerHTML = '
'; + ReactDOM.hydrate(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe('
'); + + const Test2 = () => ( + +
+ +
+
+ ); + + const container2 = document.createElement('div'); + container2.innerHTML = + '
'; + ReactDOM.hydrate(, container2); + expect(Scheduler).toFlushWithoutYielding(); + expect(container2.innerHTML).toBe( + '
', + ); + }); + + it('should hydrate TouchHitTarget hit slop elements correcty and patch them', () => { + const Test = () => ( + +
+ +
+
+ ); + + const container = document.createElement('div'); + container.innerHTML = '
'; + ReactDOM.hydrate(, container); + expect(Scheduler).toFlushWithoutYielding(); + expect(container.innerHTML).toBe( + '
', + ); + }); + }); + + describe('ReactDOMServer', () => { + beforeEach(() => { + initReactDOMServer(); + EventComponent = createReactEventComponent(); + TouchHitTarget = ReactEvents.TouchHitTarget; + }); + + it('should not warn when a TouchHitTarget is used correctly', () => { + const Test = () => ( + +
+ +
+
+ ); + + const output = ReactDOMServer.renderToString(); + expect(output).toBe('
'); + }); + + it('should render a TouchHitTarget with hit slop values', () => { + const Test = () => ( + +
+ +
+
+ ); + + let output = ReactDOMServer.renderToString(); + expect(output).toBe( + '
', + ); + + const Test2 = () => ( + +
+ +
+
+ ); + + output = ReactDOMServer.renderToString(); + expect(output).toBe( + '
', + ); + + const Test3 = () => ( + +
+ +
+
+ ); + + output = ReactDOMServer.renderToString(); + expect(output).toBe( + '
', + ); + }); }); }); diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 7be9c997e9c..cf0d47b19a3 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -458,7 +458,6 @@ export function createTouchHitTargetInstance( left: number, right: number, top: number, - parentInstance: Container, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, @@ -476,6 +475,7 @@ export function commitTouchHitTargetUpdate( newRight: number, newTop: number, touchHitTargetInstance: Instance, + parentInstance: null | Container, ): void { throw new Error('Not yet implemented.'); } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 0512b32daed..a5bf9dcc86f 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -506,7 +506,6 @@ export function createTouchHitTargetInstance( left: number, right: number, top: number, - parentInstance: Container, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, @@ -524,6 +523,7 @@ export function commitTouchHitTargetUpdate( newRight: number, newTop: number, touchHitTargetInstance: Instance, + parentInstance: null | Container, ): void { throw new Error('Not yet implemented.'); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index f5ee099313f..05d95e8c8ac 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -431,17 +431,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // NO-OP }, - createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, - parentInstance: Container, - rootContainerInstance: Container, - hostContext: HostContext, - internalInstanceHandle: Object, - ): Instance { - throw new Error('Not yet implemented.'); + createTouchHitTargetInstance() { + // NO-OP }, }; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index e6b04911a70..8685db7b6be 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -61,7 +61,10 @@ import shallowEqual from 'shared/shallowEqual'; import getComponentName from 'shared/getComponentName'; import ReactStrictModeWarnings from './ReactStrictModeWarnings'; import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent'; -import {REACT_LAZY_TYPE} from 'shared/ReactSymbols'; +import { + REACT_LAZY_TYPE, + REACT_EVENT_TARGET_TOUCH_HIT, +} from 'shared/ReactSymbols'; import warning from 'shared/warning'; import warningWithoutStack from 'shared/warningWithoutStack'; import { @@ -1981,9 +1984,14 @@ function updateEventComponent(current, workInProgress, renderExpirationTime) { } function updateEventTarget(current, workInProgress, renderExpirationTime) { + const type = workInProgress.type.type; const nextProps = workInProgress.pendingProps; let nextChildren = nextProps.children; + if (type === REACT_EVENT_TARGET_TOUCH_HIT && current === null) { + tryToClaimNextHydratableInstance(workInProgress); + } + reconcileChildren( current, workInProgress, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 9f4bda1631a..bf439405fe7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -303,6 +303,7 @@ function commitBeforeMutationLifeCycles( case HostText: case HostPortal: case IncompleteClassComponent: + case EventTarget: // Nothing to do for these component types return; default: { @@ -589,6 +590,7 @@ function commitLifeCycles( } case SuspenseComponent: case IncompleteClassComponent: + case EventTarget: break; default: { invariant( @@ -821,7 +823,8 @@ function commitContainer(finishedWork: Fiber) { switch (finishedWork.tag) { case ClassComponent: case HostComponent: - case HostText: { + case HostText: + case EventTarget: { return; } case HostRoot: @@ -958,18 +961,27 @@ function commitPlacement(finishedWork: Fiber): void { // children to find all the terminal nodes. let node: Fiber = finishedWork; while (true) { - if (node.tag === HostComponent || node.tag === HostText) { - if (before) { - if (isContainer) { - insertInContainerBefore(parent, node.stateNode, before); - } else { - insertBefore(parent, node.stateNode, before); - } - } else { - if (isContainer) { - appendChildToContainer(parent, node.stateNode); + if ( + node.tag === HostComponent || + node.tag === HostText || + (enableEventAPI && + node.tag === EventTarget && + node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) + ) { + const stateNode = node.stateNode; + if (stateNode !== null) { + if (before) { + if (isContainer) { + insertInContainerBefore(parent, stateNode, before); + } else { + insertBefore(parent, stateNode, before); + } } else { - appendChild(parent, node.stateNode); + if (isContainer) { + appendChildToContainer(parent, stateNode); + } else { + appendChild(parent, stateNode); + } } } } else if (node.tag === HostPortal) { @@ -1036,7 +1048,13 @@ function unmountHostComponents(current): void { currentParentIsValid = true; } - if (node.tag === HostComponent || node.tag === HostText) { + if ( + node.tag === HostComponent || + node.tag === HostText || + (enableEventAPI && + node.tag === EventTarget && + node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) + ) { commitNestedUnmounts(node); // After all the children have unmounted, it is now safe to remove the // node from the tree. @@ -1223,6 +1241,22 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { const newLeft = newProps.left || 0; const newRight = newProps.right || 0; const newTop = newProps.top || 0; + let parentHostInstance = null; + + if (__DEV__) { + let node = finishedWork.return; + // For cases like ReactDOM, where we need to validate that the + // parent host component is styled correctly in relation to its + // "position" style. To do this we traverse the fiber tree upwards + // until we find the parent host component instance. + while (node !== null) { + if (node.tag === HostComponent) { + parentHostInstance = node.stateNode; + break; + } + node = node.return; + } + } commitTouchHitTargetUpdate( oldBottom, oldLeft, @@ -1233,6 +1267,7 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { newRight, newTop, instance, + parentHostInstance, ); } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 47de828251d..f2814eef17c 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -86,6 +86,7 @@ import { prepareToHydrateHostTextInstance, skipPastDehydratedSuspenseInstance, popHydrationState, + prepareToHydrateTouchHitTargetInstance, } from './ReactFiberHydrationContext'; import { enableSuspenseServerRenderer, @@ -128,7 +129,10 @@ if (supportsMutation) { node.tag === EventTarget && node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) ) { - appendInitialChild(parent, node.stateNode); + const instance = node.stateNode; + if (instance !== null) { + appendInitialChild(parent, instance); + } } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -214,7 +218,6 @@ if (supportsMutation) { newTop: number, current: Fiber, workInProgress: Fiber, - parentHostInstance: Container, ) { if (enableEventAPI) { const oldProps = current.memoizedProps; @@ -287,7 +290,9 @@ if (supportsMutation) { node, ); } - appendInitialChild(parent, instance); + if (instance !== null) { + appendInitialChild(parent, instance); + } } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -395,7 +400,9 @@ if (supportsMutation) { node, ); } - appendChildToContainerChildSet(containerChildSet, instance); + if (instance !== null) { + appendChildToContainerChildSet(containerChildSet, instance); + } } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -568,7 +575,6 @@ if (supportsMutation) { newTop: number, current: Fiber, workInProgress: Fiber, - parentHostInstance: Container, ) { if (enableEventAPI) { const oldProps = current.memoizedProps; @@ -590,7 +596,6 @@ if (supportsMutation) { newLeft, newRight, newTop, - parentHostInstance, rootContainerInstance, currentHostContext, workInProgress, @@ -630,7 +635,6 @@ if (supportsMutation) { newTop: number, current: Fiber, workInProgress: Fiber, - parentHostInstance: Container, ) { // Noop }; @@ -919,21 +923,6 @@ function completeWork( const type = workInProgress.type.type; if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - let node = workInProgress.return; - let parentHostInstance = null; - // Traverse up the fiber tree till we find a host component fiber - while (node !== null) { - if (node.tag === HostComponent) { - parentHostInstance = node.stateNode; - break; - } - node = node.return; - } - invariant( - parentHostInstance !== null, - 'A touch hit target was completed without a parent host component node. ' + - 'This is probably a bug in React.', - ); const newBottom = newProps.bottom || 0; const newLeft = newProps.left || 0; const newRight = newProps.right || 0; @@ -949,7 +938,6 @@ function completeWork( newTop, current, workInProgress, - parentHostInstance, ); } else { if ( @@ -962,16 +950,42 @@ function completeWork( } const currentHostContext = getHostContext(); const rootContainerInstance = getRootHostContainer(); - workInProgress.stateNode = createTouchHitTargetInstance( - newBottom, - newLeft, - newRight, - newTop, - parentHostInstance, - rootContainerInstance, - currentHostContext, - workInProgress, - ); + const wasHydrated = popHydrationState(workInProgress); + if (wasHydrated) { + if ( + prepareToHydrateTouchHitTargetInstance( + newBottom, + newLeft, + newRight, + newTop, + workInProgress, + ) + ) { + markUpdate(workInProgress); + } + } else { + workInProgress.stateNode = createTouchHitTargetInstance( + newBottom, + newLeft, + newRight, + newTop, + rootContainerInstance, + currentHostContext, + workInProgress, + ); + // If the previous hit slop dimensions were all 0, we never actually + // create a host instance. In this case, we need to mark the update + // as having a new placement to deal with. + if (current !== null) { + workInProgress.effectTag |= Placement; + } + // For cases like ReactDOM, where we need to validate that the + // parent host component is styled correctly in relation to its + // "position" style, we mark an update for the commit phase. + if (__DEV__) { + markUpdate(workInProgress); + } + } } } } diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 07647d523c4..b7b88fa853d 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -23,6 +23,7 @@ import { HostRoot, SuspenseComponent, DehydratedSuspenseComponent, + EventTarget, } from 'shared/ReactWorkTags'; import {Deletion, Placement} from 'shared/ReactSideEffectTags'; import invariant from 'shared/invariant'; @@ -49,8 +50,14 @@ import { didNotFindHydratableInstance, didNotFindHydratableTextInstance, didNotFindHydratableSuspenseInstance, + hydrateTouchHitTargetInstance, + canHydrateTouchHitTargetInstance, } from './ReactFiberHostConfig'; -import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import { + enableSuspenseServerRenderer, + enableEventAPI, +} from 'shared/ReactFeatureFlags'; +import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -221,6 +228,21 @@ function tryHydrate(fiber, nextInstance) { } return false; } + case EventTarget: { + if (enableEventAPI) { + const type = fiber.type.type; + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + const touchHitTargetInstance = canHydrateTouchHitTargetInstance( + nextInstance, + ); + if (touchHitTargetInstance !== null) { + fiber.stateNode = (touchHitTargetInstance: Instance); + return true; + } + } + } + return false; + } default: return false; } @@ -344,6 +366,24 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { return shouldUpdate; } +function prepareToHydrateTouchHitTargetInstance( + bottom: number, + left: number, + right: number, + top: number, + fiber: Fiber, +): boolean { + if (!supportsHydration) { + invariant( + false, + 'Expected prepareToHydrateTouchHitTargetInstance() to never be called. ' + + 'This error is likely caused by a bug in React. Please file an issue.', + ); + } + const instance: Instance = fiber.stateNode; + return hydrateTouchHitTargetInstance(bottom, left, right, top, instance); +} + function skipPastDehydratedSuspenseInstance(fiber: Fiber): void { if (!supportsHydration) { invariant( @@ -440,4 +480,5 @@ export { prepareToHydrateHostTextInstance, skipPastDehydratedSuspenseInstance, popHydrationState, + prepareToHydrateTouchHitTargetInstance, }; diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index b6be6bf9831..a8204e33692 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -146,3 +146,7 @@ export const didNotFindHydratableTextInstance = $$$hostConfig.didNotFindHydratableTextInstance; export const didNotFindHydratableSuspenseInstance = $$$hostConfig.didNotFindHydratableSuspenseInstance; +export const hydrateTouchHitTargetInstance = + $$$hostConfig.hydrateTouchHitTargetInstance; +export const canHydrateTouchHitTargetInstance = + $$$hostConfig.canHydrateTouchHitTargetInstance; diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 9e4240fa155..ebda2ffac6c 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -330,7 +330,7 @@ export function handleEventComponent( rootContainerInstance: Container, internalInstanceHandle: Object, ): void { - throw new Error('Not yet implemented.'); + // noop } export function createTouchHitTargetInstance( @@ -338,12 +338,44 @@ export function createTouchHitTargetInstance( left: number, right: number, top: number, - parentInstance: Container, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, ): Instance { - throw new Error('Not yet implemented.'); + let topStyle = ''; + let leftStyle = ''; + let rightStyle = ''; + let bottomStyle = ''; + + if (top !== 0) { + topStyle = `-${top}px`; + } + if (left !== 0) { + leftStyle = `-${left}px`; + } + if (right !== 0) { + rightStyle = `-${right}px`; + } + if (bottom !== 0) { + bottomStyle = `-${bottom}px`; + } + + return { + type: 'div', + props: { + style: { + position: 'absolute', + top: topStyle, + left: leftStyle, + right: rightStyle, + bottom: bottomStyle, + }, + }, + isHidden: false, + children: [], + rootContainerInstance, + tag: 'INSTANCE', + }; } export function commitTouchHitTargetUpdate( @@ -356,6 +388,20 @@ export function commitTouchHitTargetUpdate( newRight: number, newTop: number, touchHitTargetInstance: Instance, + parentInstance: null | Container, ): void { - throw new Error('Not yet implemented.'); + const style = touchHitTargetInstance.props.style; + + if (oldTop !== newTop) { + style.top = `-${newTop}px`; + } + if (oldLeft !== newLeft) { + style.left = `-${newLeft}px`; + } + if (oldRight !== newRight) { + style.right = `-${newRight}px`; + } + if (oldBottom !== newBottom) { + style.bottom = `-${newBottom}px`; + } } diff --git a/packages/shared/HostConfigWithNoHydration.js b/packages/shared/HostConfigWithNoHydration.js index 1be5f0b8a98..adc976a849b 100644 --- a/packages/shared/HostConfigWithNoHydration.js +++ b/packages/shared/HostConfigWithNoHydration.js @@ -47,3 +47,5 @@ export const didNotFindHydratableContainerSuspenseInstance = shim; export const didNotFindHydratableInstance = shim; export const didNotFindHydratableTextInstance = shim; export const didNotFindHydratableSuspenseInstance = shim; +export const canHydrateTouchHitTargetInstance = shim; +export const hydrateTouchHitTargetInstance = shim; From 1158792b7e3cb64ef48ed0d072965e7fe5a1292b Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 3 Apr 2019 18:05:42 +0100 Subject: [PATCH 4/9] Add ReactART exports Adds hydration support --- packages/react-art/src/ReactARTHostConfig.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index dded3ecd295..a15d81a5164 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -452,10 +452,22 @@ export function createTouchHitTargetInstance( left: number, right: number, top: number, - parentInstance: Container, rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, ): Instance { throw new Error('Not yet implemented.'); } + +export function commitTouchHitTargetUpdate( + bottom: number, + left: number, + right: number, + top: number, + parentInstance: Container, + rootContainerInstance: Container, + hostContext: HostContext, + internalInstanceHandle: Object, +): void { + throw new Error('Not yet implemented.'); +} From e7d19d70bd1c520dbd1a8289fc4b98122daa79f3 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 3 Apr 2019 18:37:51 +0100 Subject: [PATCH 5/9] Add enableEventAPI flag to more areas to improve DCE --- packages/react-dom/src/client/ReactDOMComponentTree.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index 52cd9602432..6f1e1840371 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -8,6 +8,8 @@ import {HostComponent, HostText, EventTarget} from 'shared/ReactWorkTags'; import invariant from 'shared/invariant'; +import {enableEventAPI} from 'shared/ReactFeatureFlags'; + const randomKey = Math.random() .toString(36) .slice(2); @@ -41,7 +43,7 @@ export function getClosestInstanceFromNode(node) { if ( inst.tag === HostComponent || inst.tag === HostText || - inst.tag === EventTarget + (enableEventAPI && inst.tag === EventTarget) ) { // In Fiber, this will always be the deepest root. return inst; @@ -60,7 +62,7 @@ export function getInstanceFromNode(node) { if ( inst.tag === HostComponent || inst.tag === HostText || - inst.tag === EventTarget + (enableEventAPI && inst.tag === EventTarget) ) { return inst; } else { @@ -78,7 +80,7 @@ export function getNodeFromInstance(inst) { if ( inst.tag === HostComponent || inst.tag === HostText || - inst.tag === EventTarget + (enableEventAPI && inst.tag === EventTarget) ) { // In Fiber this, is just the state node right now. We assume it will be // a host component or host text. From 263970719690490c6eb7dbd0ba202cdb2d01f826 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 4 Apr 2019 15:39:35 +0100 Subject: [PATCH 6/9] Changed to an alternative model of creating child fibers via helper method --- packages/react-art/src/ReactARTHostConfig.js | 32 +-- .../react-dom/src/client/ReactDOMComponent.js | 62 ----- .../src/client/ReactDOMComponentTree.js | 22 +- .../src/client/ReactDOMHostConfig.js | 204 ++++++-------- .../src/server/ReactPartialRenderer.js | 15 +- .../__tests__/TouchHitTarget-test.internal.js | 81 +++--- .../src/ReactFabricHostConfig.js | 34 ++- .../src/ReactNativeHostConfig.js | 36 ++- .../src/createReactNoop.js | 82 ++++-- .../src/ReactFiberBeginWork.js | 37 ++- .../src/ReactFiberCommitWork.js | 107 +++----- .../src/ReactFiberCompleteWork.js | 213 +-------------- .../src/ReactFiberHydrationContext.js | 43 +-- .../ReactFiberEvents-test-internal.js | 252 +++++++----------- .../src/forks/ReactFiberHostConfig.custom.js | 10 +- .../src/ReactTestHostConfig.js | 134 ++++------ 16 files changed, 468 insertions(+), 896 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index a15d81a5164..ef5b5914bfd 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -447,27 +447,27 @@ export function handleEventComponent( throw new Error('Not yet implemented.'); } -export function createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, - rootContainerInstance: Container, - hostContext: HostContext, - internalInstanceHandle: Object, -): Instance { +export function getEventTargetChildElement( + type: Symbol | number, + props: Props, +): null { throw new Error('Not yet implemented.'); } -export function commitTouchHitTargetUpdate( - bottom: number, - left: number, - right: number, - top: number, - parentInstance: Container, +export function handleEventTarget( + type: Symbol | number, + props: Props, rootContainerInstance: Container, - hostContext: HostContext, internalInstanceHandle: Object, +): boolean { + throw new Error('Not yet implemented.'); +} + +export function commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, ): void { throw new Error('Not yet implemented.'); } diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 3b2e16d7397..bf5508c72cb 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -98,7 +98,6 @@ const AUTOFOCUS = 'autoFocus'; const CHILDREN = 'children'; const STYLE = 'style'; const HTML = '__html'; -const EMPTY_OBJECT = {}; const {html: HTML_NAMESPACE} = Namespaces; @@ -1340,67 +1339,6 @@ export function listenToEventResponderEventTypes( } } -export function createTouchHitTargetElement( - bottom: number, - left: number, - right: number, - top: number, - rootContainerInstance: Element | Document, - parentNamespace: string, -): HTMLElement { - const touchHitTargetElement = ((createElement( - 'div', - EMPTY_OBJECT, - rootContainerInstance, - parentNamespace, - ): any): HTMLElement); - const touchHitTargetElementStyle = touchHitTargetElement.style; - - touchHitTargetElementStyle.position = 'absolute'; - if (top !== 0) { - touchHitTargetElementStyle.top = `-${top}px`; - } - if (left !== 0) { - touchHitTargetElementStyle.left = `-${left}px`; - } - if (right !== 0) { - touchHitTargetElementStyle.right = `-${right}px`; - } - if (bottom !== 0) { - touchHitTargetElementStyle.bottom = `-${bottom}px`; - } - - return touchHitTargetElement; -} - -export function updateTouchHitTargetElement( - oldBottom: number, - oldLeft: number, - oldRight: number, - oldTop: number, - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - touchHitTargetElement: Element, -) { - const touchHitTargetElementStyle = ((touchHitTargetElement: any): HTMLElement) - .style; - - if (oldTop !== newTop) { - touchHitTargetElementStyle.top = `-${newTop}px`; - } - if (oldLeft !== newLeft) { - touchHitTargetElementStyle.left = `-${newLeft}px`; - } - if (oldRight !== newRight) { - touchHitTargetElementStyle.right = `-${newRight}px`; - } - if (oldBottom !== newBottom) { - touchHitTargetElementStyle.bottom = `-${newBottom}px`; - } -} - // We can remove this once the event API is stable and out of a flag if (enableEventAPI) { setListenToResponderEventTypes(listenToEventResponderEventTypes); diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index 6f1e1840371..a3b61d6e49e 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -5,11 +5,9 @@ * LICENSE file in the root directory of this source tree. */ -import {HostComponent, HostText, EventTarget} from 'shared/ReactWorkTags'; +import {HostComponent, HostText} from 'shared/ReactWorkTags'; import invariant from 'shared/invariant'; -import {enableEventAPI} from 'shared/ReactFeatureFlags'; - const randomKey = Math.random() .toString(36) .slice(2); @@ -40,11 +38,7 @@ export function getClosestInstanceFromNode(node) { } let inst = node[internalInstanceKey]; - if ( - inst.tag === HostComponent || - inst.tag === HostText || - (enableEventAPI && inst.tag === EventTarget) - ) { + if (inst.tag === HostComponent || inst.tag === HostText) { // In Fiber, this will always be the deepest root. return inst; } @@ -59,11 +53,7 @@ export function getClosestInstanceFromNode(node) { export function getInstanceFromNode(node) { const inst = node[internalInstanceKey]; if (inst) { - if ( - inst.tag === HostComponent || - inst.tag === HostText || - (enableEventAPI && inst.tag === EventTarget) - ) { + if (inst.tag === HostComponent || inst.tag === HostText) { return inst; } else { return null; @@ -77,11 +67,7 @@ export function getInstanceFromNode(node) { * DOM node. */ export function getNodeFromInstance(inst) { - if ( - inst.tag === HostComponent || - inst.tag === HostText || - (enableEventAPI && inst.tag === EventTarget) - ) { + if (inst.tag === HostComponent || inst.tag === HostText) { // In Fiber this, is just the state node right now. We assume it will be // a host component or host text. return inst.stateNode; diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 3d0d02e1b84..9cc01d3a5d2 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -25,8 +25,6 @@ import { warnForInsertedHydratedElement, warnForInsertedHydratedText, listenToEventResponderEventTypes, - createTouchHitTargetElement, - updateTouchHitTargetElement, } from './ReactDOMComponent'; import {getSelectionInformation, restoreSelection} from './ReactInputSelection'; import setTextContent from './setTextContent'; @@ -60,6 +58,22 @@ export type Props = { style?: { display?: string, }, + bottom?: null | number, + left?: null | number, + right?: null | number, + top?: null | number, +}; +export type EventTargetChildElement = { + type: string, + props: null | { + style?: { + position?: string, + bottom?: string, + left?: string, + right?: string, + top?: string, + }, + }, }; export type Container = Element | Document; export type Instance = Element; @@ -73,7 +87,6 @@ type HostContextDev = { eventData: null | {| isEventComponent?: boolean, isEventTarget?: boolean, - eventTargetType?: null | Symbol | number, |}, }; type HostContextProd = string; @@ -195,7 +208,6 @@ export function getChildHostContextForEventComponent( const eventData = { isEventComponent: true, isEventTarget: false, - eventTargetType: null, }; return {namespace, ancestorInfo, eventData}; } @@ -209,17 +221,24 @@ export function getChildHostContextForEventTarget( if (__DEV__) { const parentHostContextDev = ((parentHostContext: any): HostContextDev); const {namespace, ancestorInfo} = parentHostContextDev; - warning( - parentHostContextDev.eventData === null || - !parentHostContextDev.eventData.isEventComponent || - type !== REACT_EVENT_TARGET_TOUCH_HIT, - 'validateDOMNesting: cannot not be a direct child of an event component. ' + - 'Ensure is a direct child of a DOM element.', - ); + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + warning( + parentHostContextDev.eventData === null || + !parentHostContextDev.eventData.isEventComponent, + 'validateDOMNesting: cannot not be a direct child of an event component. ' + + 'Ensure is a direct child of a DOM element.', + ); + const parentNamespace = parentHostContextDev.namespace; + if (parentNamespace !== HTML_NAMESPACE) { + throw new Error( + ' was used in an unsupported DOM namespace. ' + + 'Ensure the is used in an HTML namespace.', + ); + } + } const eventData = { isEventComponent: false, isEventTarget: true, - eventTargetType: type, }; return {namespace, ancestorInfo, eventData}; } @@ -254,16 +273,6 @@ export function createInstance( if (__DEV__) { // TODO: take namespace into account when validating. const hostContextDev = ((hostContext: any): HostContextDev); - if (enableEventAPI) { - const eventData = hostContextDev.eventData; - if (eventData !== null) { - warning( - !eventData.isEventTarget || - eventData.eventTargetType !== REACT_EVENT_TARGET_TOUCH_HIT, - 'Warning: validateDOMNesting: must not have any children.', - ); - } - } validateDOMNesting(type, null, hostContextDev.ancestorInfo); if ( typeof props.children === 'string' || @@ -370,25 +379,12 @@ export function createTextInstance( if (enableEventAPI) { const eventData = hostContextDev.eventData; if (eventData !== null) { - warning( - eventData === null || - !eventData.isEventTarget || - eventData.eventTargetType !== REACT_EVENT_TARGET_TOUCH_HIT, - 'Warning: validateDOMNesting: must not have any children.', - ); warning( !eventData.isEventComponent, 'validateDOMNesting: React event components cannot have text DOM nodes as children. ' + 'Wrap the child text "%s" in an element.', text, ); - warning( - !eventData.isEventTarget || - eventData.eventTargetType === REACT_EVENT_TARGET_TOUCH_HIT, - 'validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "%s" in an element.', - text, - ); } } } @@ -904,98 +900,64 @@ export function handleEventComponent( } } -export function createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, - rootContainerInstance: Container, - hostContext: HostContext, - internalInstanceHandle: Object, -): Instance { - let parentNamespace: string; - if (__DEV__) { - const hostContextDev = ((hostContext: any): HostContextDev); - parentNamespace = hostContextDev.namespace; - if (parentNamespace !== HTML_NAMESPACE) { - throw new Error( - ' was used in an unsupported DOM namespace. ' + - 'Ensure the is used in an HTML namespace.', - ); +export function getEventTargetChildElement( + type: Symbol | number, + props: Props, +): null | EventTargetChildElement { + if (enableEventAPI) { + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + const {bottom, left, right, top} = props; + + if (!bottom && !left && !right && !top) { + return null; + } + return { + type: 'div', + props: { + style: { + position: 'absolute', + bottom: bottom ? `-${bottom}px` : '0px', + left: left ? `-${left}px` : '0px', + right: right ? `-${right}px` : '0px', + top: top ? `-${top}px` : '0px', + }, + }, + }; } - } else { - parentNamespace = ((hostContext: any): HostContextProd); } - const touchHitTargetInstance = createTouchHitTargetElement( - bottom, - left, - right, - top, - rootContainerInstance, - parentNamespace, - ); - precacheFiberNode(internalInstanceHandle, touchHitTargetInstance); - return touchHitTargetInstance; -} - -export function commitTouchHitTargetUpdate( - oldBottom: number, - oldLeft: number, - oldRight: number, - oldTop: number, - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - touchHitTargetInstance: Instance, - parentInstance: null | Container, -): void { - if (__DEV__ && canUseDOM && parentInstance !== null) { - // This is done at DEV time because getComputedStyle will - // typically force a style recalculation and force a layout, - // reflow -– both of which are sync are expensive. - const computedStyles = window.getComputedStyle(parentInstance); - warning( - computedStyles.getPropertyValue('position') !== 'static', - ' inserts an empty absolutely positioned
. ' + - 'This requires its parent DOM node to be positioned too, but the ' + - 'parent DOM node was found to have the style "position" set to ' + - 'value "static". Try using a "position" value of "relative".', - ); - } - updateTouchHitTargetElement( - oldBottom, - oldLeft, - oldRight, - oldTop, - newBottom, - newLeft, - newRight, - newTop, - touchHitTargetInstance, - ); + return null; } -export function hydrateTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, - touchHitTargetInstance: Instance, +export function handleEventTarget( + type: Symbol | number, + props: Props, + rootContainerInstance: Container, + internalInstanceHandle: Object, ): boolean { - // Rather than do computed checks on the position styles, we always flag - // the hit target instance as needing to update from hydration. - return true; + return false; } -export function canHydrateTouchHitTargetInstance(instance: HydratableInstance) { - // The touch hit target element is always a
. - if ( - instance.nodeType !== ELEMENT_NODE || - instance.nodeName.toLowerCase() !== 'div' - ) { - return null; +export function commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, +): void { + if (enableEventAPI) { + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + if (__DEV__ && canUseDOM) { + // This is done at DEV time because getComputedStyle will + // typically force a style recalculation and force a layout, + // reflow -– both of which are sync are expensive. + const computedStyles = window.getComputedStyle(parentInstance); + warning( + computedStyles.getPropertyValue('position') !== 'static', + ' inserts an empty absolutely positioned
. ' + + 'This requires its parent DOM node to be positioned too, but the ' + + 'parent DOM node was found to have the style "position" set to ' + + 'value "static". Try using a "position" value of "relative".', + ); + } + } } - // This has now been refined to an element node. - return ((instance: any): Instance); } diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index 7d5bd001e02..a3015099526 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -1182,12 +1182,15 @@ class ReactDOMServerRenderer { if (bottom === 0 && left === 0 && right === 0 && top === 0) { return ''; } - let topString = top ? `-${top}px;` : ''; - let leftString = left ? `-${left}px;` : ''; - let rightString = right ? `-${right}px;` : ''; - let bottomString = bottom ? `-${bottom}px;` : ''; - - return `
`; + let topString = top ? `-${top}px` : '0px'; + let leftString = left ? `-${left}px` : '0px'; + let rightString = right ? `-${right}px` : '0x'; + let bottomString = bottom ? `-${bottom}px` : '0px'; + + return ( + `
` + ); } const nextChildren = toArray( ((nextChild: any): ReactElement).props.children, diff --git a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js index 34e2c5d52f9..e385137682b 100644 --- a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js +++ b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js @@ -100,9 +100,7 @@ describe('TouchHitTarget', () => { expect(() => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); const Test2 = () => ( @@ -115,9 +113,7 @@ describe('TouchHitTarget', () => { expect(() => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); // Should render without warnings const Test3 = () => ( @@ -187,9 +183,7 @@ describe('TouchHitTarget', () => { expect(() => { root.update(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); const Test2 = () => ( @@ -202,9 +196,7 @@ describe('TouchHitTarget', () => { expect(() => { root.update(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); // Should render without warnings const Test3 = () => ( @@ -275,9 +267,7 @@ describe('TouchHitTarget', () => { expect(() => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); const Test2 = () => ( @@ -290,9 +280,7 @@ describe('TouchHitTarget', () => { expect(() => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: must not have any children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); // Should render without warnings const Test3 = () => ( @@ -342,8 +330,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', + '
', ); cond = true; @@ -374,8 +362,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', + '
', ); }); @@ -398,17 +386,14 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', + '
', ); cond = true; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - expect(container.innerHTML).toBe( - '
', - ); + expect(container.innerHTML).toBe('
'); }); it('should render a conditional TouchHitTarget hit slop correctly (true -> false)', () => { @@ -439,8 +424,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', + '
Random span 1
Random span 2
', ); }); @@ -470,16 +455,16 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', + '
Random span 1
Random span 2
', ); cond = true; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', + '
Random span 1
Random span 2
', ); }); @@ -509,16 +494,16 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', + '
Random span 1
Random span 2
', ); cond = false; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', + '
Random span 1
Random span 2
', ); }); @@ -547,11 +532,11 @@ describe('TouchHitTarget', () => { const container2 = document.createElement('div'); container2.innerHTML = - '
'; + '
'; ReactDOM.hydrate(, container2); expect(Scheduler).toFlushWithoutYielding(); expect(container2.innerHTML).toBe( - '
', + '
', ); }); @@ -566,10 +551,16 @@ describe('TouchHitTarget', () => { const container = document.createElement('div'); container.innerHTML = '
'; - ReactDOM.hydrate(, container); + expect(() => { + ReactDOM.hydrate(, container); + expect(Scheduler).toFlushWithoutYielding(); + }).toWarnDev( + 'Warning: Expected server HTML to contain a matching
in
.', + {withoutStack: true}, + ); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', + '
', ); }); }); @@ -605,7 +596,7 @@ describe('TouchHitTarget', () => { let output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); const Test2 = () => ( @@ -618,7 +609,7 @@ describe('TouchHitTarget', () => { output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); const Test3 = () => ( @@ -631,7 +622,7 @@ describe('TouchHitTarget', () => { output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); }); }); diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index cf0d47b19a3..cce4ad950aa 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -453,29 +453,27 @@ export function handleEventComponent( throw new Error('Not yet implemented.'); } -export function createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, +export function getEventTargetChildElement( + type: Symbol | number, + props: Props, +): null { + throw new Error('Not yet implemented.'); +} + +export function handleEventTarget( + type: Symbol | number, + props: Props, rootContainerInstance: Container, - hostContext: HostContext, internalInstanceHandle: Object, -): Instance { +): boolean { throw new Error('Not yet implemented.'); } -export function commitTouchHitTargetUpdate( - oldBottom: number, - oldLeft: number, - oldRight: number, - oldTop: number, - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - touchHitTargetInstance: Instance, - parentInstance: null | Container, +export function commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, ): void { throw new Error('Not yet implemented.'); } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index a5bf9dcc86f..55fe1ba868b 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -498,32 +498,30 @@ export function handleEventComponent( rootContainerInstance: Container, internalInstanceHandle: Object, ) { - // TODO: add handleEventComponent implementation + throw new Error('Not yet implemented.'); +} + +export function getEventTargetChildElement( + type: Symbol | number, + props: Props, +): null { + throw new Error('Not yet implemented.'); } -export function createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, +export function handleEventTarget( + type: Symbol | number, + props: Props, rootContainerInstance: Container, - hostContext: HostContext, internalInstanceHandle: Object, -): Instance { +): boolean { throw new Error('Not yet implemented.'); } -export function commitTouchHitTargetUpdate( - oldBottom: number, - oldLeft: number, - oldRight: number, - oldTop: number, - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - touchHitTargetInstance: Instance, - parentInstance: null | Container, +export function commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, ): void { throw new Error('Not yet implemented.'); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 05d95e8c8ac..991adf47c3c 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -33,12 +33,32 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import warningWithoutStack from 'shared/warningWithoutStack'; import {enableEventAPI} from 'shared/ReactFeatureFlags'; +type EventTargetChildElement = { + type: string, + props: null | { + style?: { + position?: string, + bottom?: string, + left?: string, + right?: string, + top?: string, + }, + }, +}; type Container = { rootID: string, children: Array, pendingChildren: Array, }; -type Props = {prop: any, hidden: boolean, children?: mixed}; +type Props = { + prop: any, + hidden: boolean, + children?: mixed, + bottom?: null | number, + left?: null | number, + right?: null | number, + top?: null | number, +}; type Instance = {| type: string, id: number, @@ -299,12 +319,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { rootContainerInstance: Container, hostContext: HostContext, ): Instance { - if (__DEV__ && enableEventAPI) { - warning( - hostContext !== EVENT_TOUCH_HIT_TARGET_CONTEXT, - 'validateDOMNesting: must not have any children.', - ); - } if (type === 'errorInCompletePhase') { throw new Error('Error in host config.'); } @@ -379,22 +393,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { internalInstanceHandle: Object, ): TextInstance { if (__DEV__ && enableEventAPI) { - warning( - hostContext !== EVENT_TOUCH_HIT_TARGET_CONTEXT, - 'validateDOMNesting: must not have any children.', - ); warning( hostContext !== EVENT_COMPONENT_CONTEXT, 'validateDOMNesting: React event components cannot have text DOM nodes as children. ' + 'Wrap the child text "%s" in an element.', text, ); - warning( - hostContext !== EVENT_TARGET_CONTEXT, - 'validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "%s" in an element.', - text, - ); } if (hostContext === UPPERCASE_CONTEXT) { text = text.toUpperCase(); @@ -431,7 +435,49 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // NO-OP }, - createTouchHitTargetInstance() { + getEventTargetChildElement( + type: Symbol | number, + props: Props, + ): null | EventTargetChildElement { + if (enableEventAPI) { + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + const {bottom, left, right, top} = props; + + if (!bottom && !left && !right && !top) { + return null; + } + return { + type: 'div', + props: { + style: { + position: 'absolute', + bottom: bottom ? `-${bottom}px` : '0px', + left: left ? `-${left}px` : '0px', + right: right ? `-${right}px` : '0px', + top: top ? `-${top}px` : '0px', + }, + }, + }; + } + } + return null; + }, + + handleEventTarget( + type: Symbol | number, + props: Props, + rootContainerInstance: Container, + internalInstanceHandle: Object, + ): boolean { + return false; + }, + + commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, + ): void { // NO-OP }, }; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 8685db7b6be..4ed12bfbc21 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -61,10 +61,7 @@ import shallowEqual from 'shared/shallowEqual'; import getComponentName from 'shared/getComponentName'; import ReactStrictModeWarnings from './ReactStrictModeWarnings'; import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent'; -import { - REACT_LAZY_TYPE, - REACT_EVENT_TARGET_TOUCH_HIT, -} from 'shared/ReactSymbols'; +import {REACT_LAZY_TYPE} from 'shared/ReactSymbols'; import warning from 'shared/warning'; import warningWithoutStack from 'shared/warningWithoutStack'; import { @@ -99,6 +96,7 @@ import { registerSuspenseInstanceRetry, } from './ReactFiberHostConfig'; import type {SuspenseInstance} from './ReactFiberHostConfig'; +import {getEventTargetChildElement} from './ReactFiberHostConfig'; import { pushHostContext, pushHostContainer, @@ -1986,18 +1984,31 @@ function updateEventComponent(current, workInProgress, renderExpirationTime) { function updateEventTarget(current, workInProgress, renderExpirationTime) { const type = workInProgress.type.type; const nextProps = workInProgress.pendingProps; - let nextChildren = nextProps.children; + const eventTargetChild = getEventTargetChildElement(type, nextProps); - if (type === REACT_EVENT_TARGET_TOUCH_HIT && current === null) { - tryToClaimNextHydratableInstance(workInProgress); + if (__DEV__) { + warning( + nextProps.children == null, + 'Event targets should not have children.', + ); } + if (eventTargetChild !== null) { + const child = (workInProgress.child = createFiberFromTypeAndProps( + eventTargetChild.type, + null, + eventTargetChild.props, + null, + workInProgress.mode, + renderExpirationTime, + )); + child.return = workInProgress; - reconcileChildren( - current, - workInProgress, - nextChildren, - renderExpirationTime, - ); + if (current === null || current.child === null) { + child.effectTag = Placement; + } + } else { + reconcileChildren(current, workInProgress, null, renderExpirationTime); + } pushHostContextForEventTarget(workInProgress); return workInProgress.child; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index bf439405fe7..dfba0370219 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -92,7 +92,7 @@ import { hideTextInstance, unhideInstance, unhideTextInstance, - commitTouchHitTargetUpdate, + commitEventTarget, } from './ReactFiberHostConfig'; import { captureCommitPhaseError, @@ -110,7 +110,6 @@ import { MountPassive, } from './ReactHookEffectTags'; import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; -import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -961,27 +960,19 @@ function commitPlacement(finishedWork: Fiber): void { // children to find all the terminal nodes. let node: Fiber = finishedWork; while (true) { - if ( - node.tag === HostComponent || - node.tag === HostText || - (enableEventAPI && - node.tag === EventTarget && - node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) - ) { + if (node.tag === HostComponent || node.tag === HostText) { const stateNode = node.stateNode; - if (stateNode !== null) { - if (before) { - if (isContainer) { - insertInContainerBefore(parent, stateNode, before); - } else { - insertBefore(parent, stateNode, before); - } + if (before) { + if (isContainer) { + insertInContainerBefore(parent, stateNode, before); } else { - if (isContainer) { - appendChildToContainer(parent, stateNode); - } else { - appendChild(parent, stateNode); - } + insertBefore(parent, stateNode, before); + } + } else { + if (isContainer) { + appendChildToContainer(parent, stateNode); + } else { + appendChild(parent, stateNode); } } } else if (node.tag === HostPortal) { @@ -1048,13 +1039,7 @@ function unmountHostComponents(current): void { currentParentIsValid = true; } - if ( - node.tag === HostComponent || - node.tag === HostText || - (enableEventAPI && - node.tag === EventTarget && - node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) - ) { + if (node.tag === HostComponent || node.tag === HostText) { commitNestedUnmounts(node); // After all the children have unmounted, it is now safe to remove the // node from the tree. @@ -1220,56 +1205,28 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { case EventTarget: { if (enableEventAPI) { const type = finishedWork.type.type; + const props = finishedWork.memoizedProps; + const instance = finishedWork.stateNode; + let parentInstance = null; - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - invariant( - finishedWork.stateNode !== null, - 'This should have a touch hit target element initialized. This error is likely ' + - 'caused by a bug in React. Please file an issue.', - ); - const instance: Instance = finishedWork.stateNode; - const newProps = finishedWork.memoizedProps; - // For hydration we reuse the update path but we treat the oldProps - // as the newProps. The updatePayload will contain the real change in - // this case. - const oldProps = current !== null ? current.memoizedProps : newProps; - const oldBottom = oldProps.bottom || 0; - const oldLeft = oldProps.left || 0; - const oldRight = oldProps.right || 0; - const oldTop = oldProps.top || 0; - const newBottom = newProps.bottom || 0; - const newLeft = newProps.left || 0; - const newRight = newProps.right || 0; - const newTop = newProps.top || 0; - let parentHostInstance = null; - - if (__DEV__) { - let node = finishedWork.return; - // For cases like ReactDOM, where we need to validate that the - // parent host component is styled correctly in relation to its - // "position" style. To do this we traverse the fiber tree upwards - // until we find the parent host component instance. - while (node !== null) { - if (node.tag === HostComponent) { - parentHostInstance = node.stateNode; - break; - } - node = node.return; - } + let node = finishedWork.return; + // Traverse up the fiber tree until we find the parent host node. + while (node !== null) { + if (node.tag === HostComponent) { + parentInstance = node.stateNode; + break; + } else if (node.tag === HostRoot) { + parentInstance = node.stateNode.containerInfo; + break; } - commitTouchHitTargetUpdate( - oldBottom, - oldLeft, - oldRight, - oldTop, - newBottom, - newLeft, - newRight, - newTop, - instance, - parentHostInstance, - ); + node = node.return; } + invariant( + parentInstance !== null, + 'This should have a parent host component initialized. This error is likely ' + + 'caused by a bug in React. Please file an issue.', + ); + commitEventTarget(type, props, instance, parentInstance); } return; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index f2814eef17c..6015f9fd962 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -66,8 +66,7 @@ import { appendChildToContainerChildSet, finalizeContainerChildren, handleEventComponent, - createTouchHitTargetInstance, - cloneHiddenTouchHitTargetInstance, + handleEventTarget, } from './ReactFiberHostConfig'; import { getRootHostContainer, @@ -86,13 +85,11 @@ import { prepareToHydrateHostTextInstance, skipPastDehydratedSuspenseInstance, popHydrationState, - prepareToHydrateTouchHitTargetInstance, } from './ReactFiberHydrationContext'; import { enableSuspenseServerRenderer, enableEventAPI, } from 'shared/ReactFeatureFlags'; -import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -108,7 +105,6 @@ let appendAllChildren; let updateHostContainer; let updateHostComponent; let updateHostText; -let updateTouchHitTarget; if (supportsMutation) { // Mutation mode @@ -122,13 +118,7 @@ if (supportsMutation) { // children to find all the terminal nodes. let node = workInProgress.child; while (node !== null) { - if ( - node.tag === HostComponent || - node.tag === HostText || - (enableEventAPI && - node.tag === EventTarget && - node.type.type === REACT_EVENT_TARGET_TOUCH_HIT) - ) { + if (node.tag === HostComponent || node.tag === HostText) { const instance = node.stateNode; if (instance !== null) { appendInitialChild(parent, instance); @@ -211,32 +201,6 @@ if (supportsMutation) { markUpdate(workInProgress); } }; - updateTouchHitTarget = function( - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - current: Fiber, - workInProgress: Fiber, - ) { - if (enableEventAPI) { - const oldProps = current.memoizedProps; - const oldBottom = oldProps.bottom || 0; - const oldLeft = oldProps.left || 0; - const oldRight = oldProps.right || 0; - const oldTop = oldProps.top || 0; - - // If the hit slop values differ, mark it as an update. All the work in done in commitWork. - if ( - oldBottom !== newBottom || - oldLeft !== newLeft || - oldRight !== newRight || - oldTop !== newTop - ) { - markUpdate(workInProgress); - } - } - }; } else if (supportsPersistence) { // Persistent host tree mode @@ -268,31 +232,6 @@ if (supportsMutation) { instance = cloneHiddenTextInstance(instance, text, node); } appendInitialChild(parent, instance); - } else if ( - enableEventAPI && - node.tag === EventTarget && - node.type.type === REACT_EVENT_TARGET_TOUCH_HIT - ) { - let instance = node.stateNode; - if (needsVisibilityToggle && isHidden) { - // This child is inside a timed out tree. Hide it. - const props = node.memoizedProps; - const bottom = props.bottom || 0; - const left = props.left || 0; - const right = props.right || 0; - const top = props.top || 0; - instance = cloneHiddenTouchHitTargetInstance( - instance, - bottom, - left, - right, - top, - node, - ); - } - if (instance !== null) { - appendInitialChild(parent, instance); - } } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -378,31 +317,6 @@ if (supportsMutation) { instance = cloneHiddenTextInstance(instance, text, node); } appendChildToContainerChildSet(containerChildSet, instance); - } else if ( - enableEventAPI && - node.tag === EventTarget && - node.type.type === REACT_EVENT_TARGET_TOUCH_HIT - ) { - let instance = node.stateNode; - if (needsVisibilityToggle && isHidden) { - // This child is inside a timed out tree. Hide it. - const props = node.memoizedProps; - const bottom = props.bottom || 0; - const left = props.left || 0; - const right = props.right || 0; - const top = props.top || 0; - instance = cloneHiddenTouchHitTargetInstance( - instance, - bottom, - left, - right, - top, - node, - ); - } - if (instance !== null) { - appendChildToContainerChildSet(containerChildSet, instance); - } } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -568,44 +482,6 @@ if (supportsMutation) { markUpdate(workInProgress); } }; - updateTouchHitTarget = function( - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - current: Fiber, - workInProgress: Fiber, - ) { - if (enableEventAPI) { - const oldProps = current.memoizedProps; - const oldBottom = oldProps.bottom || 0; - const oldLeft = oldProps.left || 0; - const oldRight = oldProps.right || 0; - const oldTop = oldProps.top || 0; - - if ( - oldBottom !== newBottom || - oldLeft !== newLeft || - oldRight !== newRight || - oldTop !== newTop - ) { - const currentHostContext = getHostContext(); - const rootContainerInstance = getRootHostContainer(); - workInProgress.stateNode = createTouchHitTargetInstance( - newBottom, - newLeft, - newRight, - newTop, - rootContainerInstance, - currentHostContext, - workInProgress, - ); - // We'll have to mark it as having an effect, even though we won't use the effect for anything. - // This lets the parents know that at least one of their children has changed. - markUpdate(workInProgress); - } - } - }; } else { // No host operations updateHostContainer = function(workInProgress: Fiber) { @@ -628,16 +504,6 @@ if (supportsMutation) { ) { // Noop }; - updateTouchHitTarget = function( - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - current: Fiber, - workInProgress: Fiber, - ) { - // Noop - }; } function completeWork( @@ -921,72 +787,15 @@ function completeWork( if (enableEventAPI) { popHostContext(workInProgress); const type = workInProgress.type.type; - - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - const newBottom = newProps.bottom || 0; - const newLeft = newProps.left || 0; - const newRight = newProps.right || 0; - const newTop = newProps.top || 0; - - if (current !== null && workInProgress.stateNode != null) { - // If we have an alternate, that means this is an update and we need - // to schedule a side-effect to do the updates. - updateTouchHitTarget( - newBottom, - newLeft, - newRight, - newTop, - current, - workInProgress, - ); - } else { - if ( - newBottom === 0 && - newLeft === 0 && - newRight === 0 && - newTop === 0 - ) { - return null; - } - const currentHostContext = getHostContext(); - const rootContainerInstance = getRootHostContainer(); - const wasHydrated = popHydrationState(workInProgress); - if (wasHydrated) { - if ( - prepareToHydrateTouchHitTargetInstance( - newBottom, - newLeft, - newRight, - newTop, - workInProgress, - ) - ) { - markUpdate(workInProgress); - } - } else { - workInProgress.stateNode = createTouchHitTargetInstance( - newBottom, - newLeft, - newRight, - newTop, - rootContainerInstance, - currentHostContext, - workInProgress, - ); - // If the previous hit slop dimensions were all 0, we never actually - // create a host instance. In this case, we need to mark the update - // as having a new placement to deal with. - if (current !== null) { - workInProgress.effectTag |= Placement; - } - // For cases like ReactDOM, where we need to validate that the - // parent host component is styled correctly in relation to its - // "position" style, we mark an update for the commit phase. - if (__DEV__) { - markUpdate(workInProgress); - } - } - } + const rootContainerInstance = getRootHostContainer(); + const shouldUpdate = handleEventTarget( + type, + newProps, + rootContainerInstance, + workInProgress, + ); + if (shouldUpdate) { + markUpdate(workInProgress); } } break; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index b7b88fa853d..07647d523c4 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -23,7 +23,6 @@ import { HostRoot, SuspenseComponent, DehydratedSuspenseComponent, - EventTarget, } from 'shared/ReactWorkTags'; import {Deletion, Placement} from 'shared/ReactSideEffectTags'; import invariant from 'shared/invariant'; @@ -50,14 +49,8 @@ import { didNotFindHydratableInstance, didNotFindHydratableTextInstance, didNotFindHydratableSuspenseInstance, - hydrateTouchHitTargetInstance, - canHydrateTouchHitTargetInstance, } from './ReactFiberHostConfig'; -import { - enableSuspenseServerRenderer, - enableEventAPI, -} from 'shared/ReactFeatureFlags'; -import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; +import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -228,21 +221,6 @@ function tryHydrate(fiber, nextInstance) { } return false; } - case EventTarget: { - if (enableEventAPI) { - const type = fiber.type.type; - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - const touchHitTargetInstance = canHydrateTouchHitTargetInstance( - nextInstance, - ); - if (touchHitTargetInstance !== null) { - fiber.stateNode = (touchHitTargetInstance: Instance); - return true; - } - } - } - return false; - } default: return false; } @@ -366,24 +344,6 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { return shouldUpdate; } -function prepareToHydrateTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, - fiber: Fiber, -): boolean { - if (!supportsHydration) { - invariant( - false, - 'Expected prepareToHydrateTouchHitTargetInstance() to never be called. ' + - 'This error is likely caused by a bug in React. Please file an issue.', - ); - } - const instance: Instance = fiber.stateNode; - return hydrateTouchHitTargetInstance(bottom, left, right, top, instance); -} - function skipPastDehydratedSuspenseInstance(fiber: Fiber): void { if (!supportsHydration) { invariant( @@ -480,5 +440,4 @@ export { prepareToHydrateHostTextInstance, skipPastDehydratedSuspenseInstance, popHydrationState, - prepareToHydrateTouchHitTargetInstance, }; diff --git a/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js b/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js index 73dcc8d4a01..5516f783731 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js @@ -127,9 +127,9 @@ describe('ReactFiberEvents', () => { it('should render a simple event component with a single event target', () => { const Test = () => ( - -
Hello world
-
+
+ Hello world +
); @@ -148,10 +148,7 @@ describe('ReactFiberEvents', () => { expect(() => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should warn when an event target has a direct text child #2', () => { @@ -167,19 +164,15 @@ describe('ReactFiberEvents', () => { expect(() => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should not warn if an event target is not a direct child of an event component', () => { const Test = () => (
- - Child 1 - + + Child 1
); @@ -207,9 +200,7 @@ describe('ReactFiberEvents', () => { expect(() => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should handle event components correctly with error boundaries', () => { @@ -219,11 +210,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -268,11 +257,9 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - -
- -
-
+
+ +
); @@ -321,9 +308,7 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - - - + ); @@ -341,7 +326,7 @@ describe('ReactFiberEvents', () => { }); expect(Scheduler).toFlushWithoutYielding(); }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + + 'Warning: validateDOMNesting: React event components cannot have text DOM nodes as children. ' + 'Wrap the child text "Text!" in an element.', ); }); @@ -355,11 +340,7 @@ describe('ReactFiberEvents', () => { _updateCounter = updateCounter; if (counter === 1) { - return ( - -
Child
-
- ); + return 123; } return ( @@ -370,18 +351,20 @@ describe('ReactFiberEvents', () => { } const Parent = () => ( - - +
+ - - + +
); ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); expect(ReactNoop).toMatchRenderedOutput(
- Child - 0 +
+ Child - 0 +
, ); @@ -390,9 +373,7 @@ describe('ReactFiberEvents', () => { _updateCounter(counter => counter + 1); }); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should error with a component stack contains the names of the event components and event targets', () => { @@ -404,11 +385,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -437,7 +416,6 @@ describe('ReactFiberEvents', () => { expect(componentStackMessage.includes('ErrorComponent')).toBe(true); expect(componentStackMessage.includes('span')).toBe(true); - expect(componentStackMessage.includes('TestEventTarget')).toBe(true); expect(componentStackMessage.includes('TestEventComponent')).toBe(true); expect(componentStackMessage.includes('Test')).toBe(true); expect(componentStackMessage.includes('Wrapper')).toBe(true); @@ -498,9 +476,9 @@ describe('ReactFiberEvents', () => { it('should render a simple event component with a single event target', () => { const Test = () => ( - -
Hello world
-
+
+ Hello world +
); @@ -511,9 +489,8 @@ describe('ReactFiberEvents', () => { const Test2 = () => ( - - I am now a span - + + I am now a span ); @@ -533,10 +510,7 @@ describe('ReactFiberEvents', () => { expect(() => { root.update(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should warn when an event target has a direct text child #2', () => { @@ -553,19 +527,15 @@ describe('ReactFiberEvents', () => { expect(() => { root.update(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should not warn if an event target is not a direct child of an event component', () => { const Test = () => (
- - Child 1 - + + Child 1
); @@ -595,9 +565,7 @@ describe('ReactFiberEvents', () => { expect(() => { root.update(); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should handle event components correctly with error boundaries', () => { @@ -607,11 +575,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -620,7 +586,7 @@ describe('ReactFiberEvents', () => { error: null, }; - componentDidCatch(error, errStack) { + componentDidCatch(error) { this.setState({ error, }); @@ -657,11 +623,9 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - -
- -
-
+
+ +
); @@ -710,9 +674,7 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - - - + ); @@ -730,7 +692,7 @@ describe('ReactFiberEvents', () => { _updateCounter(counter => counter + 1); }); }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + + 'Warning: validateDOMNesting: React event components cannot have text DOM nodes as children. ' + 'Wrap the child text "Text!" in an element.', ); }); @@ -744,11 +706,7 @@ describe('ReactFiberEvents', () => { _updateCounter = updateCounter; if (counter === 1) { - return ( - -
Child
-
- ); + return 123; } return ( @@ -759,11 +717,11 @@ describe('ReactFiberEvents', () => { } const Parent = () => ( - - +
+ - - + +
); const root = ReactTestRenderer.create(null); @@ -771,7 +729,9 @@ describe('ReactFiberEvents', () => { expect(Scheduler).toFlushWithoutYielding(); expect(root).toMatchRenderedOutput(
- Child - 0 +
+ Child - 0 +
, ); @@ -780,9 +740,7 @@ describe('ReactFiberEvents', () => { _updateCounter(counter => counter + 1); }); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should error with a component stack contains the names of the event components and event targets', () => { @@ -794,11 +752,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -828,7 +784,6 @@ describe('ReactFiberEvents', () => { expect(componentStackMessage.includes('ErrorComponent')).toBe(true); expect(componentStackMessage.includes('span')).toBe(true); - expect(componentStackMessage.includes('TestEventTarget')).toBe(true); expect(componentStackMessage.includes('TestEventComponent')).toBe(true); expect(componentStackMessage.includes('Test')).toBe(true); expect(componentStackMessage.includes('Wrapper')).toBe(true); @@ -888,9 +843,9 @@ describe('ReactFiberEvents', () => { it('should render a simple event component with a single event target', () => { const Test = () => ( - -
Hello world
-
+
+ Hello world +
); @@ -901,9 +856,8 @@ describe('ReactFiberEvents', () => { const Test2 = () => ( - - I am now a span - + + I am now a span ); @@ -923,10 +877,7 @@ describe('ReactFiberEvents', () => { const container = document.createElement('div'); ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should warn when an event target has a direct text child #2', () => { @@ -943,19 +894,15 @@ describe('ReactFiberEvents', () => { const container = document.createElement('div'); ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + - 'Wrap the child text "Hello world" in an element.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should not warn if an event target is not a direct child of an event component', () => { const Test = () => (
- - Child 1 - + + Child 1
); @@ -981,9 +928,7 @@ describe('ReactFiberEvents', () => { const container = document.createElement('div'); ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should handle event components correctly with error boundaries', () => { @@ -993,11 +938,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -1043,11 +986,9 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - -
- -
-
+
+ +
); @@ -1087,9 +1028,7 @@ describe('ReactFiberEvents', () => { const Parent = () => ( - - - + ); @@ -1103,7 +1042,7 @@ describe('ReactFiberEvents', () => { }); expect(Scheduler).toFlushWithoutYielding(); }).toWarnDev( - 'Warning: validateDOMNesting: React event targets cannot have text DOM nodes as children. ' + + 'Warning: validateDOMNesting: React event components cannot have text DOM nodes as children. ' + 'Wrap the child text "Text!" in an element.', ); }); @@ -1117,11 +1056,7 @@ describe('ReactFiberEvents', () => { _updateCounter = updateCounter; if (counter === 1) { - return ( - -
Child
-
- ); + return 123; } return ( @@ -1132,25 +1067,25 @@ describe('ReactFiberEvents', () => { } const Parent = () => ( - - +
+ - - + +
); const container = document.createElement('div'); ReactDOM.render(, container); - expect(container.innerHTML).toBe('
Child - 0
'); + expect(container.innerHTML).toBe( + '
Child - 0
', + ); expect(() => { ReactTestUtils.act(() => { _updateCounter(counter => counter + 1); }); expect(Scheduler).toFlushWithoutYielding(); - }).toWarnDev( - 'Warning: validateDOMNesting: React event targets must not have event components as children.', - ); + }).toWarnDev('Warning: Event targets should not have children.'); }); it('should error with a component stack contains the names of the event components and event targets', () => { @@ -1162,11 +1097,9 @@ describe('ReactFiberEvents', () => { const Test = () => ( - - - - - + + + ); @@ -1195,7 +1128,6 @@ describe('ReactFiberEvents', () => { expect(componentStackMessage.includes('ErrorComponent')).toBe(true); expect(componentStackMessage.includes('span')).toBe(true); - expect(componentStackMessage.includes('TestEventTarget')).toBe(true); expect(componentStackMessage.includes('TestEventComponent')).toBe(true); expect(componentStackMessage.includes('Test')).toBe(true); expect(componentStackMessage.includes('Wrapper')).toBe(true); @@ -1222,9 +1154,9 @@ describe('ReactFiberEvents', () => { it('should render a simple event component with a single event target', () => { const Test = () => ( - -
Hello world
-
+
+ Hello world +
); diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index a8204e33692..f350dc80c45 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -64,8 +64,9 @@ export const supportsMutation = $$$hostConfig.supportsMutation; export const supportsPersistence = $$$hostConfig.supportsPersistence; export const supportsHydration = $$$hostConfig.supportsHydration; export const handleEventComponent = $$$hostConfig.handleEventComponent; -export const createTouchHitTargetInstance = - $$$hostConfig.createTouchHitTargetInstance; +export const handleEventTarget = $$$hostConfig.handleEventTarget; +export const getEventTargetChildElement = + $$$hostConfig.getEventTargetChildElement; // ------------------- // Mutation @@ -87,6 +88,7 @@ export const unhideInstance = $$$hostConfig.unhideInstance; export const unhideTextInstance = $$$hostConfig.unhideTextInstance; export const commitTouchHitTargetUpdate = $$$hostConfig.commitTouchHitTargetUpdate; +export const commitEventTarget = $$$hostConfig.commitEventTarget; // ------------------- // Persistence @@ -146,7 +148,3 @@ export const didNotFindHydratableTextInstance = $$$hostConfig.didNotFindHydratableTextInstance; export const didNotFindHydratableSuspenseInstance = $$$hostConfig.didNotFindHydratableSuspenseInstance; -export const hydrateTouchHitTargetInstance = - $$$hostConfig.hydrateTouchHitTargetInstance; -export const canHydrateTouchHitTargetInstance = - $$$hostConfig.canHydrateTouchHitTargetInstance; diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index ebda2ffac6c..414a2d7f53b 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -14,6 +14,18 @@ import {REACT_EVENT_TARGET_TOUCH_HIT} from 'shared/ReactSymbols'; import {enableEventAPI} from 'shared/ReactFeatureFlags'; +type EventTargetChildElement = { + type: string, + props: null | { + style?: { + position?: string, + bottom?: string, + left?: string, + right?: string, + top?: string, + }, + }, +}; export type Type = string; export type Props = Object; export type Container = {| @@ -170,12 +182,6 @@ export function createInstance( hostContext: Object, internalInstanceHandle: Object, ): Instance { - if (__DEV__ && enableEventAPI) { - warning( - hostContext !== EVENT_TOUCH_HIT_TARGET_CONTEXT, - 'validateDOMNesting: must not have any children.', - ); - } return { type, props, @@ -233,10 +239,6 @@ export function createTextInstance( internalInstanceHandle: Object, ): TextInstance { if (__DEV__ && enableEventAPI) { - warning( - hostContext !== EVENT_TOUCH_HIT_TARGET_CONTEXT, - 'validateDOMNesting: must not have any children.', - ); warning( hostContext !== EVENT_COMPONENT_CONTEXT, 'validateDOMNesting: React event components cannot have text DOM nodes as children. ' + @@ -333,75 +335,57 @@ export function handleEventComponent( // noop } -export function createTouchHitTargetInstance( - bottom: number, - left: number, - right: number, - top: number, +export function getEventTargetChildElement( + type: Symbol | number, + props: Props, +): null | EventTargetChildElement { + if (enableEventAPI) { + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + const {bottom, left, right, top} = props; + + if (!bottom && !left && !right && !top) { + return null; + } + return { + type: 'div', + props: { + style: { + position: 'absolute', + bottom: bottom ? `-${bottom}px` : '0px', + left: left ? `-${left}px` : '0px', + right: right ? `-${right}px` : '0px', + top: top ? `-${top}px` : '0px', + }, + }, + }; + } + } + return null; +} + +export function handleEventTarget( + type: Symbol | number, + props: Props, rootContainerInstance: Container, - hostContext: HostContext, internalInstanceHandle: Object, -): Instance { - let topStyle = ''; - let leftStyle = ''; - let rightStyle = ''; - let bottomStyle = ''; - - if (top !== 0) { - topStyle = `-${top}px`; - } - if (left !== 0) { - leftStyle = `-${left}px`; - } - if (right !== 0) { - rightStyle = `-${right}px`; - } - if (bottom !== 0) { - bottomStyle = `-${bottom}px`; +): boolean { + if (enableEventAPI) { + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + // In DEV we do a computed style check on the position to ensure + // the parent host component is correctly position in the document. + if (__DEV__) { + return true; + } + } } - - return { - type: 'div', - props: { - style: { - position: 'absolute', - top: topStyle, - left: leftStyle, - right: rightStyle, - bottom: bottomStyle, - }, - }, - isHidden: false, - children: [], - rootContainerInstance, - tag: 'INSTANCE', - }; + return false; } -export function commitTouchHitTargetUpdate( - oldBottom: number, - oldLeft: number, - oldRight: number, - oldTop: number, - newBottom: number, - newLeft: number, - newRight: number, - newTop: number, - touchHitTargetInstance: Instance, - parentInstance: null | Container, +export function commitEventTarget( + type: Symbol | number, + props: Props, + instance: Instance, + parentInstance: Instance, ): void { - const style = touchHitTargetInstance.props.style; - - if (oldTop !== newTop) { - style.top = `-${newTop}px`; - } - if (oldLeft !== newLeft) { - style.left = `-${newLeft}px`; - } - if (oldRight !== newRight) { - style.right = `-${newRight}px`; - } - if (oldBottom !== newBottom) { - style.bottom = `-${newBottom}px`; - } + // noop } From daf38821c525a07393282610666962a8c3c4b6bd Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 4 Apr 2019 15:45:52 +0100 Subject: [PATCH 7/9] Tidy up some pre-existing logic from older commits --- packages/react-dom/src/events/DOMEventResponderSystem.js | 3 ++- packages/react-reconciler/src/ReactFiberCompleteWork.js | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 7e08215956f..14372411647 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -294,7 +294,8 @@ DOMEventResponderContext.prototype.isPositionWithinTouchHitTarget = function( if (childFiber === null) { return false; } - if (childFiber.tag === EventTargetWorkTag) { + const parentFiber = childFiber.return; + if (parentFiber !== null && parentFiber.tag === EventTargetWorkTag) { // TODO find another way to do this without using the // expensive getBoundingClientRect. const { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 6015f9fd962..457a6b7289a 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -119,10 +119,7 @@ if (supportsMutation) { let node = workInProgress.child; while (node !== null) { if (node.tag === HostComponent || node.tag === HostText) { - const instance = node.stateNode; - if (instance !== null) { - appendInitialChild(parent, instance); - } + appendInitialChild(parent, node.stateNode); } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in From 46237ec59536c8fe369cd6b75f1ca383a3e6beee Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 4 Apr 2019 15:49:33 +0100 Subject: [PATCH 8/9] Remove unused code --- .../src/ReactFabricHostConfig.js | 11 ----------- packages/react-noop-renderer/src/createReactNoop.js | 11 ----------- .../src/forks/ReactFiberHostConfig.custom.js | 2 -- 3 files changed, 24 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index cce4ad950aa..5f55444bff4 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -411,17 +411,6 @@ export function cloneHiddenTextInstance( throw new Error('Not yet implemented.'); } -export function cloneHiddenTouchHitTargetInstance( - instance: Instance, - bottom: number, - left: number, - right: number, - top: number, - internalInstanceHandle: Object, -) { - throw new Error('Not yet implemented.'); -} - export function createContainerChildSet(container: Container): ChildSet { return createChildNodeSet(container); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 991adf47c3c..4065c870175 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -628,17 +628,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }); return clone; }, - - cloneHiddenTouchHitTargetInstance( - instance: Instance, - bottom: number, - left: number, - right: number, - top: number, - internalInstanceHandle: Object, - ) { - throw new Error('Not yet implemented.'); - }, }; const NoopRenderer = reconciler(hostConfig); diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index f350dc80c45..9a571b168f4 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -103,8 +103,6 @@ export const finalizeContainerChildren = export const replaceContainerChildren = $$$hostConfig.replaceContainerChildren; export const cloneHiddenInstance = $$$hostConfig.cloneHiddenInstance; export const cloneHiddenTextInstance = $$$hostConfig.cloneHiddenTextInstance; -export const cloneHiddenTouchHitTargetInstance = - $$$hostConfig.cloneHiddenTouchHitTargetInstance; // ------------------- // Hydration From 0e46aecf8466158716a503d3e75e931e56ac81e8 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 4 Apr 2019 23:52:08 +0100 Subject: [PATCH 9/9] Add zIndex support --- .../src/client/ReactDOMHostConfig.js | 15 ++++++++-- .../src/server/ReactPartialRenderer.js | 2 +- .../__tests__/TouchHitTarget-test.internal.js | 29 ++++++++++--------- .../src/createReactNoop.js | 1 + .../src/ReactTestHostConfig.js | 1 + 5 files changed, 31 insertions(+), 17 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 9cc01d3a5d2..27b00f7da91 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -68,6 +68,7 @@ export type EventTargetChildElement = { props: null | { style?: { position?: string, + zIndex?: number, bottom?: string, left?: string, right?: string, @@ -916,6 +917,7 @@ export function getEventTargetChildElement( props: { style: { position: 'absolute', + zIndex: -1, bottom: bottom ? `-${bottom}px` : '0px', left: left ? `-${left}px` : '0px', right: right ? `-${right}px` : '0px', @@ -950,12 +952,21 @@ export function commitEventTarget( // typically force a style recalculation and force a layout, // reflow -– both of which are sync are expensive. const computedStyles = window.getComputedStyle(parentInstance); + const position = computedStyles.getPropertyValue('position'); warning( - computedStyles.getPropertyValue('position') !== 'static', + position !== '' && position !== 'static', ' inserts an empty absolutely positioned
. ' + 'This requires its parent DOM node to be positioned too, but the ' + 'parent DOM node was found to have the style "position" set to ' + - 'value "static". Try using a "position" value of "relative".', + 'either no value, or a value of "static". Try using a "position" ' + + 'value of "relative".', + ); + warning( + computedStyles.getPropertyValue('zIndex') !== '', + ' inserts an empty
with "z-index" of "-1". ' + + 'This requires its parent DOM node to have a "z-index" great than "-1",' + + 'but the parent DOM node was found to no "z-index" value set.' + + ' Try using a "z-index" value of "0" or greater.', ); } } diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index a3015099526..c493c0398ce 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -1188,7 +1188,7 @@ class ReactDOMServerRenderer { let bottomString = bottom ? `-${bottom}px` : '0px'; return ( - `
` ); } diff --git a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js index 3211a0983e2..e0bb517518b 100644 --- a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js +++ b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js @@ -330,7 +330,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); @@ -362,7 +362,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); }); @@ -386,7 +386,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); @@ -424,7 +424,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', ); }); @@ -455,7 +455,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', ); @@ -463,7 +463,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', ); }); @@ -494,7 +494,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', ); @@ -502,7 +502,7 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 2
', ); }); @@ -532,11 +532,11 @@ describe('TouchHitTarget', () => { const container2 = document.createElement('div'); container2.innerHTML = - '
'; + '
'; ReactDOM.hydrate(, container2); expect(Scheduler).toFlushWithoutYielding(); expect(container2.innerHTML).toBe( - '
', + '
', ); }); @@ -560,7 +560,8 @@ describe('TouchHitTarget', () => { ); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', + '
', ); }); }); @@ -596,7 +597,7 @@ describe('TouchHitTarget', () => { let output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); const Test2 = () => ( @@ -609,7 +610,7 @@ describe('TouchHitTarget', () => { output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); const Test3 = () => ( @@ -622,7 +623,7 @@ describe('TouchHitTarget', () => { output = ReactDOMServer.renderToString(); expect(output).toBe( - '
', + '
', ); }); }); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 4065c870175..de42992a6bb 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -451,6 +451,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { props: { style: { position: 'absolute', + zIndex: -1, bottom: bottom ? `-${bottom}px` : '0px', left: left ? `-${left}px` : '0px', right: right ? `-${right}px` : '0px', diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 414a2d7f53b..cb9dceec361 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -351,6 +351,7 @@ export function getEventTargetChildElement( props: { style: { position: 'absolute', + zIndex: -1, bottom: bottom ? `-${bottom}px` : '0px', left: left ? `-${left}px` : '0px', right: right ? `-${right}px` : '0px',