Fixed ReactNativeFiber event handling regression (#8959)
* Fixed ReactNativeFiber event system regression introduced in b354db2 Also added test coverage to ReactNativeEvents-test for Fiber * ReactNative tests now run against fiber renderer when env flag set Updated the test-framework-setup file so that record-tests runs native tests against ReactNativeFiber. ReactComponentTreeHook native tests all now fail but that's expected. * Avoid calling setChildren() if no children
Brian Vaughn committed
Feb 8, 2017 at 21:28 UTC
ab757e905cf4a42ad1503126eec427870de60387
7 files changed
+77
-59
scripts/fiber/tests-passing-except-dev.txt
+49
@@ -61,6 +61,55 @@ src/renderers/__tests__/ReactComponentTreeHook-test.js
61
* registers inlined text nodes
62
* works
63
64
+src/renderers/__tests__/ReactComponentTreeHook-test.native.js
65
+* uses displayName or Unknown for classic components
66
+* uses displayName, name, or ReactComponent for modern components
67
+* uses displayName, name, or Object for factory components
68
+* uses displayName, name, or StatelessComponent for functional components
69
+* reports a host tree correctly
70
+* reports a simple tree with composites correctly
71
+* reports a tree with composites correctly
72
+* ignores null children
73
+* ignores false children
74
+* reports text nodes as children
75
+* reports a single text node as a child
76
+* reports a single number node as a child
77
+* reports a zero as a child
78
+* skips empty nodes for multiple children
79
+* updates text of a single text child
80
+* updates from no children to a single text child
81
+* updates from a single text child to no children
82
+* updates from no children to multiple text children
83
+* updates from multiple text children to no children
84
+* updates from one text child to multiple text children
85
+* updates from multiple text children to one text child
86
+* updates text nodes when reordering
87
+* updates host nodes when reordering with keys
88
+* updates host nodes when reordering with keys
89
+* updates a single composite child of a different type
90
+* updates a single composite child of the same type
91
+* updates from no children to a single composite child
92
+* updates from a single composite child to no children
93
+* updates mixed children
94
+* updates with a host child
95
+* updates from null to a host child
96
+* updates from a host child to null
97
+* updates from a host child to a composite child
98
+* updates from a composite child to a host child
99
+* updates from null to a composite child
100
+* updates from a composite child to null
101
+* updates with a host child
102
+* updates from null to a host child
103
+* updates from a host child to null
104
+* updates from a host child to a composite child
105
+* updates from a composite child to a host child
106
+* updates from null to a composite child
107
+* updates from a composite child to null
108
+* tracks owner correctly
109
+* purges unmounted components automatically
110
+* reports update counts
111
+* does not report top-level wrapper as a root
112
+
113
src/renderers/__tests__/ReactCompositeComponent-test.js
114
* should warn about `forceUpdate` on unmounted components
115
* should warn about `setState` on unmounted components
scripts/fiber/tests-passing.txt
-49
@@ -558,55 +558,6 @@ src/renderers/__tests__/ReactComponentTreeHook-test.js
558
* is created during mounting
559
* is created when calling renderToString during render
560
561
-src/renderers/__tests__/ReactComponentTreeHook-test.native.js
562
-* uses displayName or Unknown for classic components
563
-* uses displayName, name, or ReactComponent for modern components
564
-* uses displayName, name, or Object for factory components
565
-* uses displayName, name, or StatelessComponent for functional components
566
-* reports a host tree correctly
567
-* reports a simple tree with composites correctly
568
-* reports a tree with composites correctly
569
-* ignores null children
570
-* ignores false children
571
-* reports text nodes as children
572
-* reports a single text node as a child
573
-* reports a single number node as a child
574
-* reports a zero as a child
575
-* skips empty nodes for multiple children
576
-* updates text of a single text child
577
-* updates from no children to a single text child
578
-* updates from a single text child to no children
579
-* updates from no children to multiple text children
580
-* updates from multiple text children to no children
581
-* updates from one text child to multiple text children
582
-* updates from multiple text children to one text child
583
-* updates text nodes when reordering
584
-* updates host nodes when reordering with keys
585
-* updates host nodes when reordering with keys
586
-* updates a single composite child of a different type
587
-* updates a single composite child of the same type
588
-* updates from no children to a single composite child
589
-* updates from a single composite child to no children
590
-* updates mixed children
591
-* updates with a host child
592
-* updates from null to a host child
593
-* updates from a host child to null
594
-* updates from a host child to a composite child
595
-* updates from a composite child to a host child
596
-* updates from null to a composite child
597
-* updates from a composite child to null
598
-* updates with a host child
599
-* updates from null to a host child
600
-* updates from a host child to null
601
-* updates from a host child to a composite child
602
-* updates from a composite child to a host child
603
-* updates from null to a composite child
604
-* updates from a composite child to null
605
-* tracks owner correctly
606
-* purges unmounted components automatically
607
-* reports update counts
608
-* does not report top-level wrapper as a root
609
-
561
src/renderers/__tests__/ReactCompositeComponent-test.js
562
* should support module pattern components
563
* should support rendering to different child types over time
scripts/jest/test-framework-setup.js
+7
@@ -3,6 +3,7 @@
3
// We want to globally mock this but jest doesn't let us do that by default
4
// for a file that already exists. So we have to explicitly mock it.
5
jest.mock('ReactDOM');
6
+jest.mock('ReactNative');
7
jest.mock('ReactDOMFeatureFlags', () => {
8
const flags = require.requireActual('ReactDOMFeatureFlags');
9
return Object.assign({}, flags, {
@@ -15,6 +16,12 @@ jest.mock('ReactFeatureFlags', () => {
16
disableNewFiberFeatures: true,
17
});
18
});
19
+jest.mock('ReactNativeFeatureFlags', () => {
20
+ const flags = require.requireActual('ReactNativeFeatureFlags');
21
+ return Object.assign({}, flags, {
22
+ useFiber: flags.useFiber || !!process.env.REACT_DOM_JEST_USE_FIBER,
23
+ });
24
+});
25
26
// Error logging varies between Fiber and Stack;
27
// Rather than fork dozens of tests, mock the error-logging file by default.
src/renderers/native/ReactNativeComponentTree.js
+3
-3
@@ -66,8 +66,8 @@ function getTagFromInstance(inst) {
66
return tag;
67
}
68
69
-function getFiberEventHandlersFromTag(tag) {
70
- return instanceProps[tag] || null;
69
+function getFiberCurrentPropsFromNode(stateNode) {
70
+ return instanceProps[stateNode._nativeTag] || null;
71
}
72
73
function updateFiberProps(tag, props) {
@@ -82,7 +82,7 @@ var ReactNativeComponentTree = {
82
precacheNode,
83
uncacheFiberNode,
84
uncacheNode,
85
- getFiberCurrentPropsFromNode: getFiberEventHandlersFromTag,
85
+ getFiberCurrentPropsFromNode,
86
updateFiberProps,
87
};
88
src/renderers/native/ReactNativeFiber.js
+5
@@ -213,6 +213,11 @@ const NativeRenderer = ReactFiberReconciler({
213
props : Props,
214
rootContainerInstance : Container,
215
) : boolean {
216
+ // Don't send a no-op message over the bridge.
217
+ if (parentInstance._children.length === 0) {
218
+ return false;
219
+ }
220
+
221
// Map from child objects to native tags.
222
// Either way we need to pass a copy of the Array to prevent it from being frozen.
223
const nativeTags = parentInstance._children.map(
src/renderers/native/__tests__/ReactNativeEvents-test.js
+6
-2
@@ -42,7 +42,6 @@ it('handles events', () => {
42
uiViewClassName: 'View',
43
});
44
45
-
45
var log = [];
46
ReactNative.render(
47
<View
@@ -63,7 +62,12 @@ it('handles events', () => {
62
);
63
64
expect(UIManager.createView.mock.calls.length).toBe(2);
66
- var innerTag = UIManager.createView.mock.calls[1][0];
65
+
66
+ // Don't depend on the order of createView() calls.
67
+ // Stack creates views outside-in; fiber creates them inside-out.
68
+ var innerTag = UIManager.createView.mock.calls.find(
69
+ args => args[3].foo === 'inner'
70
+ )[0];
71
72
EventEmitter.receiveTouches(
73
'topTouchStart',
src/renderers/native/__tests__/ReactNativeMount-test.js
+7
-5
@@ -18,6 +18,8 @@ var UIManager;
18
19
describe('ReactNative', () => {
20
beforeEach(() => {
21
+ jest.resetModules();
22
+
23
React = require('React');
24
ReactNative = require('ReactNative');
25
UIManager = require('UIManager');
@@ -45,17 +47,17 @@ describe('ReactNative', () => {
47
48
ReactNative.render(<View foo="foo" />, 11);
49
48
- expect(UIManager.createView.mock.calls.length).toBe(2);
49
- expect(UIManager.setChildren.mock.calls.length).toBe(2);
50
+ expect(UIManager.createView.mock.calls.length).toBe(1);
51
+ expect(UIManager.setChildren.mock.calls.length).toBe(1);
52
expect(UIManager.manageChildren).not.toBeCalled();
53
expect(UIManager.updateView).not.toBeCalled();
54
55
ReactNative.render(<View foo="bar" />, 11);
56
55
- expect(UIManager.createView.mock.calls.length).toBe(2);
56
- expect(UIManager.setChildren.mock.calls.length).toBe(2);
57
+ expect(UIManager.createView.mock.calls.length).toBe(1);
58
+ expect(UIManager.setChildren.mock.calls.length).toBe(1);
59
expect(UIManager.manageChildren).not.toBeCalled();
58
- expect(UIManager.updateView).toBeCalledWith(3, 'View', { foo: 'bar' });
60
+ expect(UIManager.updateView).toBeCalledWith(2, 'View', { foo: 'bar' });
61
});
62
63
it('returns the correct instance and calls it in the callback', () => {