From 26aa1987ce823f54ebc90b2538184fefbc16b99a Mon Sep 17 00:00:00 2001 From: Eli White Date: Fri, 28 Feb 2020 13:45:42 -0800 Subject: [PATCH] [Native] Enable and remove targetAsInstance feature flag. (#18182) --- .../src/ReactFabricComponentTree.js | 20 +--- .../src/ReactFabricEventEmitter.js | 15 +-- .../src/ReactNativeComponentTree.js | 25 ++-- .../src/ReactNativeEventEmitter.js | 9 +- .../__tests__/ReactFabric-test.internal.js | 107 ------------------ .../ReactNativeEvents-test.internal.js | 79 ------------- packages/shared/ReactFeatureFlags.js | 3 - .../forks/ReactFeatureFlags.native-fb.js | 5 - .../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.testing.js | 1 - .../forks/ReactFeatureFlags.testing.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 2 - scripts/flow/react-native-host-hooks.js | 1 - .../shims/react-native/ReactFeatureFlags.js | 1 - 17 files changed, 20 insertions(+), 253 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricComponentTree.js b/packages/react-native-renderer/src/ReactFabricComponentTree.js index 9def0894f8cfa..1031ffe457a6a 100644 --- a/packages/react-native-renderer/src/ReactFabricComponentTree.js +++ b/packages/react-native-renderer/src/ReactFabricComponentTree.js @@ -7,25 +7,17 @@ import invariant from 'shared/invariant'; -import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags'; - function getInstanceFromInstance(instanceHandle) { return instanceHandle; } function getTagFromInstance(inst) { - if (enableNativeTargetAsInstance) { - let nativeInstance = inst.stateNode.canonical; - invariant( - nativeInstance._nativeTag, - 'All native instances should have a tag.', - ); - return nativeInstance; - } else { - let tag = inst.stateNode.canonical._nativeTag; - invariant(tag, 'All native instances should have a tag.'); - return tag; - } + let nativeInstance = inst.stateNode.canonical; + invariant( + nativeInstance._nativeTag, + 'All native instances should have a tag.', + ); + return nativeInstance; } export { diff --git a/packages/react-native-renderer/src/ReactFabricEventEmitter.js b/packages/react-native-renderer/src/ReactFabricEventEmitter.js index 49abda74eb362..0804f37c4bd78 100644 --- a/packages/react-native-renderer/src/ReactFabricEventEmitter.js +++ b/packages/react-native-renderer/src/ReactFabricEventEmitter.js @@ -19,7 +19,6 @@ import {registrationNameModules} from 'legacy-events/EventPluginRegistry'; import {batchedUpdates} from 'legacy-events/ReactGenericBatching'; import accumulateInto from 'legacy-events/accumulateInto'; -import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags'; import {plugins} from 'legacy-events/EventPluginRegistry'; import getListener from 'legacy-events/getListener'; import {runEventsInBatch} from 'legacy-events/EventBatching'; @@ -85,16 +84,12 @@ export function dispatchEvent( const targetFiber = (target: null | Fiber); let eventTarget = null; - if (enableNativeTargetAsInstance) { - if (targetFiber != null) { - const stateNode = targetFiber.stateNode; - // Guard against Fiber being unmounted - if (stateNode != null) { - eventTarget = stateNode.canonical; - } + if (targetFiber != null) { + const stateNode = targetFiber.stateNode; + // Guard against Fiber being unmounted + if (stateNode != null) { + eventTarget = stateNode.canonical; } - } else { - eventTarget = nativeEvent.target; } batchedUpdates(function() { diff --git a/packages/react-native-renderer/src/ReactNativeComponentTree.js b/packages/react-native-renderer/src/ReactNativeComponentTree.js index 6ba871b7c083b..23748937e2ace 100644 --- a/packages/react-native-renderer/src/ReactNativeComponentTree.js +++ b/packages/react-native-renderer/src/ReactNativeComponentTree.js @@ -7,8 +7,6 @@ import invariant from 'shared/invariant'; -import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags'; - const instanceCache = new Map(); const instanceProps = new Map(); @@ -26,23 +24,14 @@ function getInstanceFromTag(tag) { } function getTagFromInstance(inst) { - if (enableNativeTargetAsInstance) { - let nativeInstance = inst.stateNode; - let tag = nativeInstance._nativeTag; - if (tag === undefined) { - nativeInstance = nativeInstance.canonical; - tag = nativeInstance._nativeTag; - } - invariant(tag, 'All native instances should have a tag.'); - return nativeInstance; - } else { - let tag = inst.stateNode._nativeTag; - if (tag === undefined) { - tag = inst.stateNode.canonical._nativeTag; - } - invariant(tag, 'All native instances should have a tag.'); - return tag; + let nativeInstance = inst.stateNode; + let tag = nativeInstance._nativeTag; + if (tag === undefined) { + nativeInstance = nativeInstance.canonical; + tag = nativeInstance._nativeTag; } + invariant(tag, 'All native instances should have a tag.'); + return nativeInstance; } export { diff --git a/packages/react-native-renderer/src/ReactNativeEventEmitter.js b/packages/react-native-renderer/src/ReactNativeEventEmitter.js index fe69998e9db45..ae7f8e8927acd 100644 --- a/packages/react-native-renderer/src/ReactNativeEventEmitter.js +++ b/packages/react-native-renderer/src/ReactNativeEventEmitter.js @@ -18,7 +18,6 @@ import {PLUGIN_EVENT_SYSTEM} from 'legacy-events/EventSystemFlags'; import {registrationNameModules} from 'legacy-events/EventPluginRegistry'; import {batchedUpdates} from 'legacy-events/ReactGenericBatching'; import {runEventsInBatch} from 'legacy-events/EventBatching'; -import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags'; import {plugins} from 'legacy-events/EventPluginRegistry'; import getListener from 'legacy-events/getListener'; import accumulateInto from 'legacy-events/accumulateInto'; @@ -104,12 +103,8 @@ function _receiveRootNodeIDEvent( const inst = getInstanceFromNode(rootNodeID); let target = null; - if (enableNativeTargetAsInstance) { - if (inst != null) { - target = inst.stateNode; - } - } else { - target = nativeEvent.target; + if (inst != null) { + target = inst.stateNode; } batchedUpdates(function() { 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 2143957d19e5b..5ef0ad91f67cb 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -12,7 +12,6 @@ let React; let ReactFabric; -let ReactFeatureFlags; let createReactNativeComponentClass; let UIManager; let StrictMode; @@ -38,7 +37,6 @@ 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; createReactNativeComponentClass = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface') @@ -649,112 +647,7 @@ 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, - ); - expect(ReactFabric.findNodeHandle(ref1.current)).toEqual( - event.currentTarget, - ); - }} - onStartShouldSetResponder={() => true} - /> - { - expect(ref2.current).not.toBeNull(); - expect(ReactFabric.findNodeHandle(ref2.current)).toEqual( - event.target, - ); - expect(ReactFabric.findNodeHandle(ref2.current)).toEqual( - event.currentTarget, - ); - }} - 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(6); - }); - it('dispatches event with target as instance', () => { - ReactFeatureFlags.enableNativeTargetAsInstance = true; - const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: { id: 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 3c49c62983ed3..474592ab7793e 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js @@ -14,7 +14,6 @@ let PropTypes; let RCTEventEmitter; let React; let ReactNative; -let ReactFeatureFlags; let ResponderEventPlugin; let UIManager; let createReactNativeComponentClass; @@ -69,7 +68,6 @@ 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; @@ -459,84 +457,7 @@ it('handles events without target', () => { ]); }); -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, - ); - expect(ReactNative.findNodeHandle(ref1.current)).toEqual( - event.currentTarget, - ); - }} - onStartShouldSetResponder={() => true} - /> - { - expect(ref2.current).not.toBeNull(); - expect(ReactNative.findNodeHandle(ref2.current)).toEqual( - event.target, - ); - expect(ReactNative.findNodeHandle(ref2.current)).toEqual( - event.currentTarget, - ); - }} - 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(6); -}); - it('dispatches event with target as instance', () => { - ReactFeatureFlags.enableNativeTargetAsInstance = true; const EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; const View = fakeRequireNativeComponent('View', {id: true}); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 24d62ac2b06d7..412ec5f9eef73 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -83,9 +83,6 @@ 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; - // Controls sequence of passive effect destroy and create functions. // If this flag is off, destroy and create functions may be interleaved. // When the flag is on, all destroy functions will be run (for all fibers) diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 5b50b15fc6700..bce5af20b237d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -12,11 +12,6 @@ import invariant from 'shared/invariant'; import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as ExportsType from './ReactFeatureFlags.native-fb'; -// Uncomment to re-export dynamic flags from the fbsource version. -export const { - enableNativeTargetAsInstance, -} = require('../shims/ReactFeatureFlags'); - // The rest of the flags are static for better dead code elimination. export const enableUserTimingAPI = __DEV__; export const enableProfilerTimer = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 2ef64264e4243..c30e273b75e3b 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = false; export const disableMapsAsChildren = false; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index cd0a7955b0d6a..5bd1551a9cedf 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = false; export const disableMapsAsChildren = false; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 61c5b861c11ef..a785da9ec1251 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = false; export const disableMapsAsChildren = false; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 76a747ef7cc8f..c00f54a5e8985 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = false; export const disableMapsAsChildren = false; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 6dde781472ead..c8e7e7248415b 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = false; export const disableMapsAsChildren = false; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 3a2fff266c2ab..4b4db20654698 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = __EXPERIMENTAL__; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; -export const enableNativeTargetAsInstance = false; export const disableTextareaChildren = __EXPERIMENTAL__; export const disableMapsAsChildren = __EXPERIMENTAL__; export const warnUnstableRenderSubtreeIntoContainer = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index d352e0b662247..aea6f29596145 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -89,8 +89,6 @@ export const enableSuspenseCallback = true; export const flushSuspenseFallbacksInTests = true; -export const enableNativeTargetAsInstance = false; - export const disableTextareaChildren = __EXPERIMENTAL__; export const disableMapsAsChildren = __EXPERIMENTAL__; diff --git a/scripts/flow/react-native-host-hooks.js b/scripts/flow/react-native-host-hooks.js index 38625c31e291c..85ea68f160ce2 100644 --- a/scripts/flow/react-native-host-hooks.js +++ b/scripts/flow/react-native-host-hooks.js @@ -187,5 +187,4 @@ 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 9b6ec97a6234c..13c548b4bb1e8 100644 --- a/scripts/rollup/shims/react-native/ReactFeatureFlags.js +++ b/scripts/rollup/shims/react-native/ReactFeatureFlags.js @@ -12,7 +12,6 @@ const ReactFeatureFlags = { debugRenderPhaseSideEffects: false, - enableNativeTargetAsInstance: false, }; module.exports = ReactFeatureFlags;