From 2c6ea0b3ffffd1a110845327262ecea59ee48dab Mon Sep 17 00:00:00 2001 From: Eli White Date: Mon, 11 Nov 2019 11:35:29 -0800 Subject: [PATCH] [Native] Add FeatureFlag to dispatch events with instance targets (#17323) * [Native] Add FeatureFlag to dispatch events with instance targets * Prettier --- .../src/ReactFabricEventEmitter.js | 20 +- .../src/ReactNativeEventEmitter.js | 15 +- .../__tests__/ReactFabric-test.internal.js | 194 ++++++++++++++++++ .../ReactNativeEvents-test.internal.js | 140 +++++++++++++ packages/shared/ReactFeatureFlags.js | 3 + .../forks/ReactFeatureFlags.native-fb.js | 4 +- .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.persistent.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 2 + scripts/flow/react-native-host-hooks.js | 1 + .../shims/react-native/ReactFeatureFlags.js | 1 + 13 files changed, 380 insertions(+), 4 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricEventEmitter.js b/packages/react-native-renderer/src/ReactFabricEventEmitter.js index cdc00c4da5..52d5136c0c 100644 --- a/packages/react-native-renderer/src/ReactFabricEventEmitter.js +++ b/packages/react-native-renderer/src/ReactFabricEventEmitter.js @@ -18,7 +18,10 @@ import {registrationNameModules} from 'legacy-events/EventPluginRegistry'; import {batchedUpdates} from 'legacy-events/ReactGenericBatching'; import type {AnyNativeEvent} from 'legacy-events/PluginModuleType'; -import {enableFlareAPI} from 'shared/ReactFeatureFlags'; +import { + enableFlareAPI, + enableNativeTargetAsInstance, +} from 'shared/ReactFeatureFlags'; import type {TopLevelType} from 'legacy-events/TopLevelEventTypes'; import {dispatchEventForResponderEventSystem} from './ReactFabricEventResponderSystem'; @@ -30,6 +33,7 @@ export function dispatchEvent( nativeEvent: AnyNativeEvent, ) { const targetFiber = (target: null | Fiber); + if (enableFlareAPI) { // React Flare event system dispatchEventForResponderEventSystem( @@ -38,13 +42,25 @@ export function dispatchEvent( (nativeEvent: any), ); } + + let eventTarget; + if (enableNativeTargetAsInstance) { + if (targetFiber == null) { + eventTarget = null; + } else { + eventTarget = targetFiber.stateNode.canonical; + } + } else { + eventTarget = nativeEvent.target; + } + batchedUpdates(function() { // Heritage plugin event system runExtractedPluginEventsInBatch( topLevelType, targetFiber, nativeEvent, - nativeEvent.target, + eventTarget, PLUGIN_EVENT_SYSTEM, ); }); diff --git a/packages/react-native-renderer/src/ReactNativeEventEmitter.js b/packages/react-native-renderer/src/ReactNativeEventEmitter.js index 072d5106a2..3bb9fee82c 100644 --- a/packages/react-native-renderer/src/ReactNativeEventEmitter.js +++ b/packages/react-native-renderer/src/ReactNativeEventEmitter.js @@ -15,6 +15,7 @@ import { import {registrationNameModules} from 'legacy-events/EventPluginRegistry'; import {batchedUpdates} from 'legacy-events/ReactGenericBatching'; import warningWithoutStack from 'shared/warningWithoutStack'; +import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags'; import {getInstanceFromNode} from './ReactNativeComponentTree'; @@ -98,12 +99,24 @@ function _receiveRootNodeIDEvent( ) { const nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT; const inst = getInstanceFromNode(rootNodeID); + + let target; + if (enableNativeTargetAsInstance) { + if (inst == null) { + target = null; + } else { + target = inst.stateNode; + } + } else { + target = nativeEvent.target; + } + batchedUpdates(function() { runExtractedPluginEventsInBatch( topLevelType, inst, nativeEvent, - nativeEvent.target, + target, PLUGIN_EVENT_SYSTEM, ); }); diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 5a502edd67..207f2d0afe 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -12,6 +12,7 @@ let React; let ReactFabric; +let ReactFeatureFlags; let createReactClass; let createReactNativeComponentClass; let UIManager; @@ -38,6 +39,7 @@ describe('ReactFabric', () => { React = require('react'); StrictMode = React.StrictMode; ReactFabric = require('react-native-renderer/fabric'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); UIManager = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface') .UIManager; createReactClass = require('create-react-class/factory')( @@ -779,6 +781,198 @@ describe('ReactFabric', () => { expect(touchStart2).toBeCalled(); }); + it('dispatches event with target as reactTag', () => { + ReactFeatureFlags.enableNativeTargetAsInstance = false; + + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: { + id: true, + }, + uiViewClassName: 'RCTView', + directEventTypes: { + topTouchStart: { + registrationName: 'onTouchStart', + }, + topTouchEnd: { + registrationName: 'onTouchEnd', + }, + }, + })); + + function getViewById(id) { + const [ + reactTag, + , + , + , + instanceHandle, + ] = nativeFabricUIManager.createNode.mock.calls.find( + args => args[3] && args[3].id === id, + ); + + return {reactTag, instanceHandle}; + } + + const ref1 = React.createRef(); + const ref2 = React.createRef(); + + ReactFabric.render( + + { + expect(ref1.current).not.toBeNull(); + expect(ReactFabric.findNodeHandle(ref1.current)).toEqual( + event.target, + ); + }} + onStartShouldSetResponder={() => true} + /> + { + expect(ref2.current).not.toBeNull(); + expect(ReactFabric.findNodeHandle(ref2.current)).toEqual( + event.target, + ); + }} + onStartShouldSetResponder={() => true} + /> + , + 1, + ); + + let [ + dispatchEvent, + ] = nativeFabricUIManager.registerEventHandler.mock.calls[0]; + + dispatchEvent(getViewById('one').instanceHandle, 'topTouchStart', { + target: getViewById('one').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + dispatchEvent(getViewById('one').instanceHandle, 'topTouchEnd', { + target: getViewById('one').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + dispatchEvent(getViewById('two').instanceHandle, 'topTouchStart', { + target: getViewById('two').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + dispatchEvent(getViewById('two').instanceHandle, 'topTouchEnd', { + target: getViewById('two').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + expect.assertions(4); + }); + + it('dispatches event with target as instance', () => { + ReactFeatureFlags.enableNativeTargetAsInstance = true; + + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: { + id: true, + }, + uiViewClassName: 'RCTView', + directEventTypes: { + topTouchStart: { + registrationName: 'onTouchStart', + }, + topTouchEnd: { + registrationName: 'onTouchEnd', + }, + }, + })); + + function getViewById(id) { + const [ + reactTag, + , + , + , + instanceHandle, + ] = nativeFabricUIManager.createNode.mock.calls.find( + args => args[3] && args[3].id === id, + ); + + return {reactTag, instanceHandle}; + } + + const ref1 = React.createRef(); + const ref2 = React.createRef(); + + ReactFabric.render( + + { + expect(ref1.current).not.toBeNull(); + // Check for referential equality + expect(ref1.current).toBe(event.target); + }} + onStartShouldSetResponder={() => true} + /> + { + expect(ref2.current).not.toBeNull(); + // Check for referential equality + expect(ref2.current).toBe(event.target); + }} + onStartShouldSetResponder={() => true} + /> + , + 1, + ); + + let [ + dispatchEvent, + ] = nativeFabricUIManager.registerEventHandler.mock.calls[0]; + + dispatchEvent(getViewById('one').instanceHandle, 'topTouchStart', { + target: getViewById('one').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + dispatchEvent(getViewById('one').instanceHandle, 'topTouchEnd', { + target: getViewById('one').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + dispatchEvent(getViewById('two').instanceHandle, 'topTouchStart', { + target: getViewById('two').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + dispatchEvent(getViewById('two').instanceHandle, 'topTouchEnd', { + target: getViewById('two').reactTag, + identifier: 17, + touches: [], + changedTouches: [], + }); + + expect.assertions(4); + }); + it('findHostInstance_DEPRECATED should warn if used to find a host component inside StrictMode', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js index 31b913129d..064da92758 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js @@ -14,6 +14,7 @@ let PropTypes; let RCTEventEmitter; let React; let ReactNative; +let ReactFeatureFlags; let ResponderEventPlugin; let UIManager; let createReactNativeComponentClass; @@ -68,6 +69,7 @@ beforeEach(() => { .RCTEventEmitter; React = require('react'); ReactNative = require('react-native-renderer'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ResponderEventPlugin = require('legacy-events/ResponderEventPlugin').default; UIManager = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface') .UIManager; @@ -456,3 +458,141 @@ it('handles events without target', () => { 'two responder end', ]); }); + +it('dispatches event with target as reactTag', () => { + ReactFeatureFlags.enableNativeTargetAsInstance = false; + const EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; + + const View = fakeRequireNativeComponent('View', {id: true}); + + function getViewById(id) { + return UIManager.createView.mock.calls.find( + args => args[3] && args[3].id === id, + )[0]; + } + + const ref1 = React.createRef(); + const ref2 = React.createRef(); + + ReactNative.render( + + { + expect(ref1.current).not.toBeNull(); + expect(ReactNative.findNodeHandle(ref1.current)).toEqual( + event.target, + ); + }} + onStartShouldSetResponder={() => true} + /> + { + expect(ref2.current).not.toBeNull(); + expect(ReactNative.findNodeHandle(ref2.current)).toEqual( + event.target, + ); + }} + onStartShouldSetResponder={() => true} + /> + , + 1, + ); + + EventEmitter.receiveTouches( + 'topTouchStart', + [{target: getViewById('one'), identifier: 17}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchEnd', + [{target: getViewById('one'), identifier: 17}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchStart', + [{target: getViewById('two'), identifier: 18}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchEnd', + [{target: getViewById('two'), identifier: 18}], + [0], + ); + + expect.assertions(4); +}); + +it('dispatches event with target as instance', () => { + ReactFeatureFlags.enableNativeTargetAsInstance = true; + const EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; + + const View = fakeRequireNativeComponent('View', {id: true}); + + function getViewById(id) { + return UIManager.createView.mock.calls.find( + args => args[3] && args[3].id === id, + )[0]; + } + + const ref1 = React.createRef(); + const ref2 = React.createRef(); + + ReactNative.render( + + { + expect(ref1.current).not.toBeNull(); + // Check for referential equality + expect(ref1.current).toBe(event.target); + }} + onStartShouldSetResponder={() => true} + /> + { + expect(ref2.current).not.toBeNull(); + // Check for referential equality + expect(ref2.current).toBe(event.target); + }} + onStartShouldSetResponder={() => true} + /> + , + 1, + ); + + EventEmitter.receiveTouches( + 'topTouchStart', + [{target: getViewById('one'), identifier: 17}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchEnd', + [{target: getViewById('one'), identifier: 17}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchStart', + [{target: getViewById('two'), identifier: 18}], + [0], + ); + + EventEmitter.receiveTouches( + 'topTouchEnd', + [{target: getViewById('two'), identifier: 18}], + [0], + ); + + expect.assertions(4); +}); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index b74aa2c30f..fc30d6f9a3 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -89,3 +89,6 @@ export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; + +// Flag to turn event.target and event.currentTarget in ReactNative from a reactTag to a component instance +export const enableNativeTargetAsInstance = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 88f15f0115..4eb32dbaaf 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -13,7 +13,9 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.native-fb'; // Uncomment to re-export dynamic flags from the fbsource version. -// export const {} = require('../shims/ReactFeatureFlags'); +export const { + enableNativeTargetAsInstance, +} = require('../shims/ReactFeatureFlags'); // The rest of the flags are static for better dead code elimination. export const enableUserTimingAPI = __DEV__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 5c264ca405..1006296f65 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -37,6 +37,7 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; +export const enableNativeTargetAsInstance = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index 55f62f3390..ab0a07ee9d 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -37,6 +37,7 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; +export const enableNativeTargetAsInstance = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index bf83c976aa..1d5049ffff 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -37,6 +37,7 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; +export const enableNativeTargetAsInstance = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 930a2077d8..20d16e23ab 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -35,6 +35,7 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; +export const enableNativeTargetAsInstance = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index b672259c20..ba025eefd3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -85,6 +85,8 @@ export const enableSuspenseCallback = true; export const flushSuspenseFallbacksInTests = true; +export const enableNativeTargetAsInstance = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/scripts/flow/react-native-host-hooks.js b/scripts/flow/react-native-host-hooks.js index 460f6170cd..a0aaa4ebc5 100644 --- a/scripts/flow/react-native-host-hooks.js +++ b/scripts/flow/react-native-host-hooks.js @@ -181,4 +181,5 @@ declare module 'RTManager' { // shims/ReactFeatureFlags is generated by the packaging script declare module '../shims/ReactFeatureFlags' { declare export var debugRenderPhaseSideEffects: boolean; + declare export var enableNativeTargetAsInstance: boolean; } diff --git a/scripts/rollup/shims/react-native/ReactFeatureFlags.js b/scripts/rollup/shims/react-native/ReactFeatureFlags.js index 86feb9c022..b52380c760 100644 --- a/scripts/rollup/shims/react-native/ReactFeatureFlags.js +++ b/scripts/rollup/shims/react-native/ReactFeatureFlags.js @@ -12,6 +12,7 @@ const ReactFeatureFlags = { debugRenderPhaseSideEffects: false, + enableNativeTargetAsInstance: false, }; module.exports = ReactFeatureFlags;