@samitouri / QOS-React / commits / 264bed13ba

[Fiber] Miscellaneous fixes to get more tests passing (#8151)

* Reset current owner when handling an error * Make string ref deletion tests accept null * Relax requirements on ref and render interleaving

Dan Abramov committed Oct 30, 2016 at 22:15 UTC 264bed13ba638f14bf8f0b3af6ca8dfe0915dcc8
3 files changed +39 -10
src/renderers/shared/fiber/ReactFiberScheduler.js
+2
@@ -555,6 +555,8 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
555 });
556 }
557
558 + ReactCurrentOwner.current = null;
559 +
560 // Surface the first error uncaught by the boundaries to the user.
561 if (firstUncaughtError) {
562 // We need to make sure any future root can get scheduled despite these errors.
src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js
+17 -6
@@ -13,12 +13,14 @@
13
14 var React;
15 var ReactDOM;
16 +var ReactDOMFeatureFlags;
17 var ReactTestUtils;
18
19 describe('ReactComponent', () => {
20 beforeEach(() => {
21 React = require('React');
22 ReactDOM = require('ReactDOM');
23 + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
24 ReactTestUtils = require('ReactTestUtils');
25 });
26
@@ -195,7 +197,7 @@ describe('ReactComponent', () => {
197 getInner = () => {
198 // (With old-style refs, it's impossible to get a ref to this div
199 // because Wrapper is the current owner when this function is called.)
198 - return <div title="inner" ref={(c) => this.innerRef = c} />;
200 + return <div className="inner" ref={(c) => this.innerRef = c} />;
201 };
202
203 render() {
@@ -211,7 +213,7 @@ describe('ReactComponent', () => {
213 componentDidMount() {
214 // Check .props.title to make sure we got the right elements back
215 expect(this.wrapperRef.getTitle()).toBe('wrapper');
214 - expect(ReactDOM.findDOMNode(this.innerRef).title).toBe('inner');
216 + expect(ReactDOM.findDOMNode(this.innerRef).className).toBe('inner');
217 mounted = true;
218 }
219 }
@@ -291,10 +293,19 @@ describe('ReactComponent', () => {
293 'outer componentDidMount',
294 'start update',
295 // Previous (equivalent) refs get cleared
294 - 'ref 1 got null',
295 - 'inner 1 render',
296 - 'ref 2 got null',
297 - 'inner 2 render',
296 + ...(ReactDOMFeatureFlags.useFiber ? [
297 + // Fiber renders first, resets refs later
298 + 'inner 1 render',
299 + 'inner 2 render',
300 + 'ref 1 got null',
301 + 'ref 2 got null',
302 + ] : [
303 + // Stack resets refs before rendering
304 + 'ref 1 got null',
305 + 'inner 1 render',
306 + 'ref 2 got null',
307 + 'inner 2 render',
308 + ]),
309 'inner 1 componentDidUpdate',
310 'ref 1 got instance 1',
311 'inner 2 componentDidUpdate',
src/renderers/shared/stack/reconciler/__tests__/refs-destruction-test.js
+20 -4
@@ -45,9 +45,17 @@ describe('refs-destruction', () => {
45 var testInstance = ReactDOM.render(<TestComponent />, container);
46 expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv))
47 .toBe(true);
48 - expect(Object.keys(testInstance.refs || {}).length).toEqual(1);
48 + expect(
49 + Object.keys(testInstance.refs || {})
50 + .filter(key => testInstance.refs[key])
51 + .length
52 + ).toEqual(1);
53 ReactDOM.unmountComponentAtNode(container);
50 - expect(Object.keys(testInstance.refs || {}).length).toEqual(0);
54 + expect(
55 + Object.keys(testInstance.refs || {})
56 + .filter(key => testInstance.refs[key])
57 + .length
58 + ).toEqual(0);
59 });
60
61 it('should remove refs when destroying the child', () => {
@@ -55,9 +63,17 @@ describe('refs-destruction', () => {
63 var testInstance = ReactDOM.render(<TestComponent />, container);
64 expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv))
65 .toBe(true);
58 - expect(Object.keys(testInstance.refs || {}).length).toEqual(1);
66 + expect(
67 + Object.keys(testInstance.refs || {})
68 + .filter(key => testInstance.refs[key])
69 + .length
70 + ).toEqual(1);
71 ReactDOM.render(<TestComponent destroy={true} />, container);
60 - expect(Object.keys(testInstance.refs || {}).length).toEqual(0);
72 + expect(
73 + Object.keys(testInstance.refs || {})
74 + .filter(key => testInstance.refs[key])
75 + .length
76 + ).toEqual(0);
77 });
78
79 it('should not error when destroying child with ref asynchronously', () => {