Extract Fabric event handlers from canonical props (#13024)

sebmarkbage · web-flow · commit 051637da615a · 2018-06-13T16:20:48.000-07:00
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. :)
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -17,19 +17,16 @@
 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';
 
 /**
  * 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
@@ -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(<View onTouchStart={touchStart} />, 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(<View onTouchStart={touchStart2} />, 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();
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -293,6 +293,8 @@ export function prepareUpdate(`
`293`	`293`	`);`
`294`	`294`	`// TODO: If the event handlers have changed, we need to update the current props`
`295`	`295`	`// in the commit phase but there is no host config hook to do it yet.`
	`296`	`+ // So instead we hack it by updating it in the render phase.`
	`297`	`+ instance.canonical.currentProps = newProps;`
`296`	`298`	`return updatePayload;`
`297`	`299`	`}`
`298`	`300`
Original file line number	Diff line number	Diff line change
`@@ -20,12 +20,7 @@ export function uncacheFiberNode(tag) {`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`function getInstanceFromTag(tag) {`
`23`		`- if (typeof tag === 'number') {`
`24`		`- return instanceCache[tag] \|\| null;`
`25`		`- } else {`
`26`		`- // Fabric will invoke event emitters on a direct fiber reference`
`27`		`- return tag;`
`28`		`- }`
	`23`	`+ return instanceCache[tag] \|\| null;`
`29`	`24`	`}`
`30`	`25`
`31`	`26`	`function getTagFromInstance(inst) {`