[RT] More predictable things (#11139)
* Inject the right event emitter Previously this was injecting the ReactNativeEventEmitter even though we want the ReactNativeRTEventEmitter. RCTEventEmitter is also just an abstraction around BatchedBridge that registers a single name. We can't register more than one with it. Removed that abstraction for now so we don't have to add it back into the RN repo. * Unify appendChildToDetachedParent and appendChild, separate root Merge appendChildToDetachedParent and appendChild. We don't need the distinction. We do however need a separate notion for the root container. Calling this `appendChildToContext` since Context has a meaning here. * Add a simple shallow comparison so that we don't send updates for equal props This still sends all props if any differs. Not sure we'll want that but I think so. * Add BatchedBridge to bundle externals * Lint
Sebastian Markbåge committed
Oct 6, 2017 at 19:08 UTC
574457114097aabe77fe4cd7b5ed93b8cd3d6795
8 files changed
+78
-23
flow/react-native-host-hooks.js
+11
-4
@@ -93,13 +93,13 @@ declare module 'RTManager' {
93
classType : string,
94
props : ?Object,
95
) : void;
96
- declare function appendChildToDetachedParent(
97
- parentTag : number,
98
- childTag : number,
99
- ) : void;
96
97
declare function beginUpdates() : void;
98
99
+ declare function appendChildToContext(
100
+ contextTag : number,
101
+ childTag : number,
102
+ ) : void;
103
declare function appendChild(
104
parentTag : number,
105
childTag : number,
@@ -118,3 +118,10 @@ declare module 'RTManager' {
118
119
declare function completeUpdates() : void;
120
}
121
+
122
+declare module 'BatchedBridge' {
123
+ declare function registerCallableModule(
124
+ name : string,
125
+ module : Object,
126
+ ) : void;
127
+}
scripts/rollup/bundles.js
+1
-1
@@ -302,7 +302,7 @@ const bundles = [
302
'ExceptionsManager',
303
'InitializeCore',
304
'Platform',
305
- 'RCTEventEmitter',
305
+ 'BatchedBridge',
306
'RTManager',
307
'prop-types/checkPropTypes',
308
],
src/__mocks__/BatchedBridge.js
new
+14
@@ -0,0 +1,14 @@
1
+/**
2
+ * Copyright (c) 2013-present, Facebook, Inc.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ */
7
+
8
+'use strict';
9
+
10
+var BatchedBridge = {
11
+ registerCallableModule: jest.fn(),
12
+};
13
+
14
+module.exports = BatchedBridge;
src/__mocks__/RTManager.js
+3
-3
@@ -11,11 +11,11 @@
11
12
var RCTRTManager = {
13
createNode: jest.fn(function createView(tag, classType, props) {}),
14
- appendChildToDetachedParent: jest.fn(function appendChildToDetachedParent(
15
- parentTag,
14
+ beginUpdates: jest.fn(function beginUpdates() {}),
15
+ appendChildToContext: jest.fn(function appendChildToContext(
16
+ contextTag,
17
childTag,
18
) {}),
18
- beginUpdates: jest.fn(function beginUpdates() {}),
19
appendChild: jest.fn(function appendChild(parentTag, childTag) {}),
20
prependChild: jest.fn(function prependChild(childTag, beforeTag) {}),
21
deleteChild: jest.fn(function deleteChild(childTag) {}),
src/renderers/native-rt/ReactNativeRTEventEmitter.js
+5
-1
@@ -11,6 +11,7 @@
11
12
var ReactNativeRTComponentTree = require('ReactNativeRTComponentTree');
13
var ReactGenericBatching = require('ReactGenericBatching');
14
+var BatchedBridge = require('BatchedBridge');
15
16
var ReactNativeRTEventEmitter = {
17
/**
@@ -40,4 +41,7 @@ var ReactNativeRTEventEmitter = {
41
},
42
};
43
43
-module.exports = ReactNativeRTEventEmitter;
44
+BatchedBridge.registerCallableModule(
45
+ 'RTEventEmitter',
46
+ ReactNativeRTEventEmitter,
47
+);
src/renderers/native-rt/ReactNativeRTFiberEntry.js
+1
-7
@@ -32,13 +32,7 @@ import type {ReactNodeList} from 'ReactTypes';
32
*/
33
require('InitializeCore');
34
35
-var RCTEventEmitter = require('RCTEventEmitter');
36
-var ReactNativeEventEmitter = require('ReactNativeEventEmitter');
37
-
38
-/**
39
- * Register the event emitter with the native bridge
40
- */
41
-RCTEventEmitter.register(ReactNativeEventEmitter);
35
+require('ReactNativeRTEventEmitter');
36
37
ReactGenericBatching.injection.injectFiberBatchedUpdates(
38
ReactNativeRTFiberRenderer.batchedUpdates,
src/renderers/native-rt/ReactNativeRTFiberRenderer.js
+30
-4
@@ -45,6 +45,29 @@ function processProps(instance: number, props: Props): Object {
45
return propsPayload;
46
}
47
48
+function arePropsEqual(oldProps: Props, newProps: Props): boolean {
49
+ var key;
50
+ for (key in newProps) {
51
+ if (key === 'children') {
52
+ // Skip special case.
53
+ continue;
54
+ }
55
+ if (newProps[key] !== oldProps[key]) {
56
+ return false;
57
+ }
58
+ }
59
+ for (key in oldProps) {
60
+ if (key === 'children') {
61
+ // Skip special case.
62
+ continue;
63
+ }
64
+ if (!(key in newProps)) {
65
+ return false;
66
+ }
67
+ }
68
+ return true;
69
+}
70
+
71
const NativeRTRenderer = ReactFiberReconciler({
72
appendChild(parentInstance: Instance, child: Instance | TextInstance): void {
73
RTManager.appendChild(parentInstance, child);
@@ -54,14 +77,14 @@ const NativeRTRenderer = ReactFiberReconciler({
77
parentInstance: Container,
78
child: Instance | TextInstance,
79
): void {
57
- RTManager.appendChild(parentInstance, child);
80
+ RTManager.appendChildToContext(parentInstance, child);
81
},
82
83
appendInitialChild(
84
parentInstance: Instance,
85
child: Instance | TextInstance,
86
): void {
64
- RTManager.appendChildToDetachedParent(parentInstance, child);
87
+ RTManager.appendChild(parentInstance, child);
88
},
89
90
commitTextUpdate(
@@ -90,7 +113,7 @@ const NativeRTRenderer = ReactFiberReconciler({
113
internalInstanceHandle: Object,
114
): void {
115
updateFiberProps(instance, newProps);
93
- RTManager.updateNode(instance, processProps(instance, newProps));
116
+ RTManager.updateNode(instance, updatePayload);
117
},
118
119
createInstance(
@@ -165,7 +188,10 @@ const NativeRTRenderer = ReactFiberReconciler({
188
rootContainerInstance: Container,
189
hostContext: {},
190
): null | Object {
168
- return emptyObject;
191
+ if (arePropsEqual(oldProps, newProps)) {
192
+ return null;
193
+ }
194
+ return processProps(instance, newProps);
195
},
196
197
removeChild(parentInstance: Instance, child: Instance | TextInstance): void {
src/renderers/native-rt/__tests__/ReactNativeRT-test.js
+13
-3
@@ -25,7 +25,8 @@ describe('ReactNativeRT', () => {
25
it('should be able to create and render a native component', () => {
26
ReactNativeRT.render(<rt-box foo="test" />, 1);
27
expect(RTManager.createNode).toBeCalled();
28
- expect(RTManager.appendChild).toBeCalled();
28
+ expect(RTManager.appendChildToContext).toBeCalled();
29
+ expect(RTManager.appendChild).not.toBeCalled();
30
expect(RTManager.updateNode).not.toBeCalled();
31
});
32
@@ -34,13 +35,22 @@ describe('ReactNativeRT', () => {
35
36
expect(RTManager.createNode.mock.calls.length).toBe(1);
37
expect(RTManager.createNode).toBeCalledWith(1, 'rt-box', {foo: 'foo'});
37
- expect(RTManager.appendChild.mock.calls.length).toBe(1);
38
+ expect(RTManager.appendChildToContext.mock.calls.length).toBe(1);
39
+ expect(RTManager.appendChild).not.toBeCalled();
40
expect(RTManager.updateNode).not.toBeCalled();
41
42
ReactNativeRT.render(<rt-box foo="bar" />, 11);
43
44
expect(RTManager.createNode.mock.calls.length).toBe(1);
43
- expect(RTManager.appendChild.mock.calls.length).toBe(1);
45
+ expect(RTManager.appendChildToContext.mock.calls.length).toBe(1);
46
+ expect(RTManager.appendChild).not.toBeCalled();
47
expect(RTManager.updateNode).toBeCalledWith(1, {foo: 'bar'});
48
+
49
+ ReactNativeRT.render(<rt-box foo="bar"><rt-box /></rt-box>, 11);
50
+
51
+ expect(RTManager.createNode.mock.calls.length).toBe(2);
52
+ expect(RTManager.appendChildToContext.mock.calls.length).toBe(1);
53
+ expect(RTManager.appendChildToContext.mock.calls.length).toBe(1);
54
+ expect(RTManager.updateNode.mock.calls.length).toBe(1);
55
});
56
});