From 051637da615a4e8d0cbf8f147a75e129c1866138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 13 Jun 2018 16:20:48 -0700 Subject: [PATCH] Extract Fabric event handlers from canonical props (#13024) We need a different "component tree" thingy for Fabric. A lot of this doesn't really make much sense in a persistent world but currently we can't dispatch events to memoizedProps on a Fiber since they're pooled. Also, it's unclear what the semantics should be when we dispatch an event that happened when the old props were in effect but now we have new props already. This implementation tries to use the last committed props but also fails at that because we don't have a commit hook in the persistent mode. However, at least it doesn't crash when dispatching. :) --- .../react-native-renderer/src/ReactFabric.js | 4 +- .../src/ReactFabricComponentTree.js | 28 ++++++++++++ .../src/ReactFabricHostConfig.js | 2 + .../src/ReactFabricInjection.js | 14 ++---- .../src/ReactNativeComponentTree.js | 7 +-- .../src/ReactNativeInjection.js | 4 ++ .../src/ReactNativeInjectionShared.js | 3 -- .../__tests__/ReactFabric-test.internal.js | 45 +++++++++++++++++++ 8 files changed, 85 insertions(+), 22 deletions(-) create mode 100644 packages/react-native-renderer/src/ReactFabricComponentTree.js diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index e4e663de72..0634921b68 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -20,7 +20,7 @@ import ReactVersion from 'shared/ReactVersion'; import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; -import * as ReactNativeComponentTree from './ReactNativeComponentTree'; +import * as ReactFabricComponentTree from './ReactFabricComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; @@ -120,7 +120,7 @@ const ReactFabric: ReactFabricType = { }; ReactFabricRenderer.injectIntoDevTools({ - findFiberByHostInstance: ReactNativeComponentTree.getClosestInstanceFromNode, + findFiberByHostInstance: ReactFabricComponentTree.getClosestInstanceFromNode, getInspectorDataForViewTag: getInspectorDataForViewTag, bundleType: __DEV__ ? 1 : 0, version: ReactVersion, diff --git a/packages/react-native-renderer/src/ReactFabricComponentTree.js b/packages/react-native-renderer/src/ReactFabricComponentTree.js new file mode 100644 index 0000000000..23f5ce9b95 --- /dev/null +++ b/packages/react-native-renderer/src/ReactFabricComponentTree.js @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import invariant from 'fbjs/lib/invariant'; + +function getInstanceFromInstance(instanceHandle) { + return instanceHandle; +} + +function getTagFromInstance(inst) { + let tag = inst.stateNode.canonical._nativeTag; + invariant(tag, 'All native instances should have a tag.'); + return tag; +} + +export { + getInstanceFromInstance as getClosestInstanceFromNode, + getInstanceFromInstance as getInstanceFromNode, + getTagFromInstance as getNodeFromInstance, +}; + +export function getFiberCurrentPropsFromNode(inst) { + return inst.canonical.currentProps; +} diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 321b86753e..ef4525d8e5 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -293,6 +293,8 @@ export function prepareUpdate( ); // TODO: If the event handlers have changed, we need to update the current props // in the commit phase but there is no host config hook to do it yet. + // So instead we hack it by updating it in the render phase. + instance.canonical.currentProps = newProps; return updatePayload; } diff --git a/packages/react-native-renderer/src/ReactFabricInjection.js b/packages/react-native-renderer/src/ReactFabricInjection.js index eb4c2bca3f..a3e766b47e 100644 --- a/packages/react-native-renderer/src/ReactFabricInjection.js +++ b/packages/react-native-renderer/src/ReactFabricInjection.js @@ -9,15 +9,7 @@ import './ReactNativeInjectionShared'; -// TODO: The event emitter registration is interfering with the existing -// ReactNative renderer. So disable it for Fabric for now. +import * as ReactFabricComponentTree from './ReactFabricComponentTree'; +import * as EventPluginUtils from 'events/EventPluginUtils'; -// import * as ReactNativeEventEmitter from './ReactNativeEventEmitter'; - -// Module provided by RN: -// import RCTEventEmitter from 'RCTEventEmitter'; - -/** - * Register the event emitter with the native bridge - */ -// RCTEventEmitter.register(ReactNativeEventEmitter); +EventPluginUtils.injection.injectComponentTree(ReactFabricComponentTree); diff --git a/packages/react-native-renderer/src/ReactNativeComponentTree.js b/packages/react-native-renderer/src/ReactNativeComponentTree.js index 6123dc1928..32554cd0bc 100644 --- a/packages/react-native-renderer/src/ReactNativeComponentTree.js +++ b/packages/react-native-renderer/src/ReactNativeComponentTree.js @@ -20,12 +20,7 @@ export function uncacheFiberNode(tag) { } function getInstanceFromTag(tag) { - if (typeof tag === 'number') { - return instanceCache[tag] || null; - } else { - // Fabric will invoke event emitters on a direct fiber reference - return tag; - } + return instanceCache[tag] || null; } function getTagFromInstance(inst) { diff --git a/packages/react-native-renderer/src/ReactNativeInjection.js b/packages/react-native-renderer/src/ReactNativeInjection.js index df5a468cd1..801304cf0d 100644 --- a/packages/react-native-renderer/src/ReactNativeInjection.js +++ b/packages/react-native-renderer/src/ReactNativeInjection.js @@ -9,6 +9,8 @@ import './ReactNativeInjectionShared'; +import * as ReactNativeComponentTree from './ReactNativeComponentTree'; +import * as EventPluginUtils from 'events/EventPluginUtils'; import * as ReactNativeEventEmitter from './ReactNativeEventEmitter'; // Module provided by RN: @@ -18,3 +20,5 @@ import RCTEventEmitter from 'RCTEventEmitter'; * Register the event emitter with the native bridge */ RCTEventEmitter.register(ReactNativeEventEmitter); + +EventPluginUtils.injection.injectComponentTree(ReactNativeComponentTree); diff --git a/packages/react-native-renderer/src/ReactNativeInjectionShared.js b/packages/react-native-renderer/src/ReactNativeInjectionShared.js index cc35fdebe9..be0f25a4d3 100644 --- a/packages/react-native-renderer/src/ReactNativeInjectionShared.js +++ b/packages/react-native-renderer/src/ReactNativeInjectionShared.js @@ -17,11 +17,9 @@ import 'InitializeCore'; import * as EventPluginHub from 'events/EventPluginHub'; -import * as EventPluginUtils from 'events/EventPluginUtils'; import ResponderEventPlugin from 'events/ResponderEventPlugin'; import ReactNativeBridgeEventPlugin from './ReactNativeBridgeEventPlugin'; -import * as ReactNativeComponentTree from './ReactNativeComponentTree'; import ReactNativeEventPluginOrder from './ReactNativeEventPluginOrder'; import ReactNativeGlobalResponderHandler from './ReactNativeGlobalResponderHandler'; @@ -29,7 +27,6 @@ import ReactNativeGlobalResponderHandler from './ReactNativeGlobalResponderHandl * Inject module for resolving DOM hierarchy and plugin ordering. */ EventPluginHub.injection.injectEventPluginOrder(ReactNativeEventPluginOrder); -EventPluginUtils.injection.injectComponentTree(ReactNativeComponentTree); ResponderEventPlugin.injection.injectGlobalResponderHandler( ReactNativeGlobalResponderHandler, 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 d01a5a26f3..fda50ecb78 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -352,4 +352,49 @@ describe('ReactFabric', () => { 11, ); }); + + it('dispatches events to the last committed props', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {}, + uiViewClassName: 'RCTView', + directEventTypes: { + topTouchStart: { + registrationName: 'onTouchStart', + }, + }, + })); + + const touchStart = jest.fn(); + const touchStart2 = jest.fn(); + + ReactFabric.render(, 11); + + expect(FabricUIManager.createNode.mock.calls.length).toBe(1); + expect(FabricUIManager.registerEventHandler.mock.calls.length).toBe(1); + + let [, , , , instanceHandle] = FabricUIManager.createNode.mock.calls[0]; + let [dispatchEvent] = FabricUIManager.registerEventHandler.mock.calls[0]; + + let touchEvent = { + touches: [], + changedTouches: [], + }; + + expect(touchStart).not.toBeCalled(); + + dispatchEvent(instanceHandle, 'topTouchStart', touchEvent); + + expect(touchStart).toBeCalled(); + expect(touchStart2).not.toBeCalled(); + + ReactFabric.render(, 11); + + // Intentionally dispatch to the same instanceHandle again. + dispatchEvent(instanceHandle, 'topTouchStart', touchEvent); + + // The current semantics dictate that we always dispatch to the last committed + // props even though the actual scheduling of the event could have happened earlier. + // This could change in the future. + expect(touchStart2).toBeCalled(); + }); });