Refs between fiber and stack (#8458)
I feel gross. Better ideas welcome.
Ben Alpert committed
Nov 30, 2016 at 12:17 UTC
705c9bcfd2745749a603b72e04528095021a673b
4 files changed
+204
-67
scripts/fiber/tests-passing-except-dev.txt
+4
@@ -177,3 +177,7 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
177
* should warn for childContextTypes on a functional component
178
* should warn when given a ref
179
* should use correct name in key warning
180
+
181
+src/renderers/shared/shared/__tests__/refs-test.js
182
+* attaches, detaches from fiber component with stack layer
183
+* attaches, detaches from stack component with fiber layer
src/renderers/shared/fiber/ReactChildFiber.js
+26
-13
@@ -15,6 +15,7 @@
15
import type { ReactCoroutine, ReactYield } from 'ReactCoroutine';
16
import type { ReactPortal } from 'ReactPortal';
17
import type { Fiber } from 'ReactFiber';
18
+import type { ReactInstance } from 'ReactInstanceType';
19
import type { PriorityLevel } from 'ReactPriorityLevel';
20
21
var REACT_ELEMENT_TYPE = require('ReactElementSymbol');
@@ -33,6 +34,7 @@ var ReactTypeOfWork = require('ReactTypeOfWork');
34
35
var emptyObject = require('emptyObject');
36
var getIteratorFn = require('getIteratorFn');
37
+var invariant = require('invariant');
38
39
const {
40
cloneFiber,
@@ -70,21 +72,32 @@ function coerceRef(current: ?Fiber, element: ReactElement<any>) {
72
let mixedRef = element.ref;
73
if (mixedRef != null && typeof mixedRef !== 'function') {
74
if (element._owner) {
73
- const ownerFiber : ?Fiber = (element._owner : any);
74
- if (ownerFiber && ownerFiber.tag === ClassComponent) {
75
- const stringRef = String(mixedRef);
76
- // Check if previous string ref matches new string ref
77
- if (current && current.ref && current.ref._stringRef === stringRef) {
78
- return current.ref;
75
+ const ownerFiber : ?(Fiber | ReactInstance) = (element._owner : any);
76
+ let inst;
77
+ if (ownerFiber) {
78
+ if ((ownerFiber : any).tag === ClassComponent) {
79
+ inst = (ownerFiber : any).stateNode;
80
+ } else {
81
+ // Stack
82
+ inst = (ownerFiber : any).getPublicInstance();
83
}
80
- const inst = ownerFiber.stateNode;
81
- const ref = function(value) {
82
- const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs;
83
- refs[stringRef] = value;
84
- };
85
- ref._stringRef = stringRef;
86
- return ref;
84
}
85
+ invariant(inst, 'Missing owner for string ref %s', mixedRef);
86
+ const stringRef = String(mixedRef);
87
+ // Check if previous string ref matches new string ref
88
+ if (current && current.ref && current.ref._stringRef === stringRef) {
89
+ return current.ref;
90
+ }
91
+ const ref = function(value) {
92
+ const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs;
93
+ if (value === null) {
94
+ delete refs[stringRef];
95
+ } else {
96
+ refs[stringRef] = value;
97
+ }
98
+ };
99
+ ref._stringRef = stringRef;
100
+ return ref;
101
}
102
}
103
return mixedRef;
src/renderers/shared/shared/__tests__/refs-test.js
+135
-32
@@ -111,6 +111,8 @@ var expectClickLogsLengthToBe = function(instance, length) {
111
describe('reactiverefs', () => {
112
beforeEach(() => {
113
jest.resetModuleRegistry();
114
+ React = require('React');
115
+ ReactTestUtils = require('ReactTestUtils');
116
});
117
118
/**
@@ -152,43 +154,46 @@ describe('reactiverefs', () => {
154
* Tests that when a ref hops around children, we can track that correctly.
155
*/
156
describe('ref swapping', () => {
157
+ let RefHopsAround;
158
beforeEach(() => {
159
jest.resetModuleRegistry();
157
- });
160
+ React = require('React');
161
+ ReactTestUtils = require('ReactTestUtils');
162
159
- class RefHopsAround extends React.Component {
160
- state = {count: 0};
163
+ RefHopsAround = class extends React.Component {
164
+ state = {count: 0};
165
162
- moveRef = () => {
163
- this.setState({count: this.state.count + 1});
164
- };
166
+ moveRef = () => {
167
+ this.setState({count: this.state.count + 1});
168
+ };
169
166
- render() {
167
- var count = this.state.count;
168
- /**
169
- * What we have here, is three divs with refs (div1/2/3), but a single
170
- * moving cursor ref `hopRef` that "hops" around the three. We'll call the
171
- * `moveRef()` function several times and make sure that the hop ref
172
- * points to the correct divs.
173
- */
174
- return (
175
- <div>
176
- <div
177
- className="first"
178
- ref={count % 3 === 0 ? 'hopRef' : 'divOneRef'}
179
- />
180
- <div
181
- className="second"
182
- ref={count % 3 === 1 ? 'hopRef' : 'divTwoRef'}
183
- />
184
- <div
185
- className="third"
186
- ref={count % 3 === 2 ? 'hopRef' : 'divThreeRef'}
187
- />
188
- </div>
189
- );
190
- }
191
- }
170
+ render() {
171
+ var count = this.state.count;
172
+ /**
173
+ * What we have here, is three divs with refs (div1/2/3), but a single
174
+ * moving cursor ref `hopRef` that "hops" around the three. We'll call the
175
+ * `moveRef()` function several times and make sure that the hop ref
176
+ * points to the correct divs.
177
+ */
178
+ return (
179
+ <div>
180
+ <div
181
+ className="first"
182
+ ref={count % 3 === 0 ? 'hopRef' : 'divOneRef'}
183
+ />
184
+ <div
185
+ className="second"
186
+ ref={count % 3 === 1 ? 'hopRef' : 'divTwoRef'}
187
+ />
188
+ <div
189
+ className="third"
190
+ ref={count % 3 === 2 ? 'hopRef' : 'divThreeRef'}
191
+ />
192
+ </div>
193
+ );
194
+ }
195
+ };
196
+ });
197
198
it('Allow refs to hop around children correctly', () => {
199
var refHopsAround = ReactTestUtils.renderIntoDocument(<RefHopsAround />);
@@ -284,3 +289,101 @@ describe('ref swapping', () => {
289
expect(a.refs[1].nodeName).toBe('DIV');
290
});
291
});
292
+
293
+describe('string refs between fiber and stack', () => {
294
+ beforeEach(() => {
295
+ jest.resetModuleRegistry();
296
+ React = require('React');
297
+ ReactTestUtils = require('ReactTestUtils');
298
+ });
299
+
300
+ it('attaches, detaches from fiber component with stack layer', () => {
301
+ spyOn(console, 'error');
302
+ const ReactCurrentOwner = require('ReactCurrentOwner');
303
+ const ReactDOM = require('ReactDOM');
304
+ const ReactDOMFiber = require('ReactDOMFiber');
305
+ const ReactInstanceMap = require('ReactInstanceMap');
306
+ let layerMounted = false;
307
+ class A extends React.Component {
308
+ render() {
309
+ return <div />;
310
+ }
311
+ componentDidMount() {
312
+ // ReactLayeredComponentMixin sets ReactCurrentOwner manually
313
+ ReactCurrentOwner.current = ReactInstanceMap.get(this);
314
+ const span = <span ref="span" />;
315
+ ReactCurrentOwner.current = null;
316
+
317
+ ReactDOM.unstable_renderSubtreeIntoContainer(
318
+ this,
319
+ span,
320
+ this._container = document.createElement('div'),
321
+ () => {
322
+ expect(this.refs.span.nodeName).toBe('SPAN');
323
+ layerMounted = true;
324
+ }
325
+ );
326
+ }
327
+ componentWillUnmount() {
328
+ ReactDOM.unmountComponentAtNode(this._container);
329
+ }
330
+ }
331
+ const container = document.createElement('div');
332
+ const a = ReactDOMFiber.render(<A />, container);
333
+ expect(a.refs.span).toBeTruthy();
334
+ ReactDOMFiber.unmountComponentAtNode(container);
335
+ expect(a.refs.span).toBe(undefined);
336
+ expect(layerMounted).toBe(true);
337
+ expectDev(console.error.calls.count()).toBe(1);
338
+ expectDev(console.error.calls.argsFor(0)[0]).toBe(
339
+ 'Warning: You are using React DOM Fiber which is an experimental ' +
340
+ 'renderer. It is likely to have bugs, breaking changes and is ' +
341
+ 'unsupported.'
342
+ );
343
+ });
344
+
345
+ it('attaches, detaches from stack component with fiber layer', () => {
346
+ spyOn(console, 'error');
347
+ const ReactCurrentOwner = require('ReactCurrentOwner');
348
+ const ReactDOM = require('ReactDOM');
349
+ const ReactDOMFiber = require('ReactDOMFiber');
350
+ const ReactInstanceMap = require('ReactInstanceMap');
351
+ let layerMounted = false;
352
+ class A extends React.Component {
353
+ render() {
354
+ return <div />;
355
+ }
356
+ componentDidMount() {
357
+ // ReactLayeredComponentMixin sets ReactCurrentOwner manually
358
+ ReactCurrentOwner.current = ReactInstanceMap.get(this);
359
+ const span = <span ref="span" />;
360
+ ReactCurrentOwner.current = null;
361
+
362
+ ReactDOMFiber.unstable_renderSubtreeIntoContainer(
363
+ this,
364
+ span,
365
+ this._container = document.createElement('div'),
366
+ () => {
367
+ expect(this.refs.span.nodeName).toBe('SPAN');
368
+ layerMounted = true;
369
+ }
370
+ );
371
+ }
372
+ componentWillUnmount() {
373
+ ReactDOMFiber.unmountComponentAtNode(this._container);
374
+ }
375
+ }
376
+ const container = document.createElement('div');
377
+ const a = ReactDOM.render(<A />, container);
378
+ expect(a.refs.span).toBeTruthy();
379
+ ReactDOM.unmountComponentAtNode(container);
380
+ expect(a.refs.span).toBe(undefined);
381
+ expect(layerMounted).toBe(true);
382
+ expectDev(console.error.calls.count()).toBe(1);
383
+ expectDev(console.error.calls.argsFor(0)[0]).toBe(
384
+ 'Warning: You are using React DOM Fiber which is an experimental ' +
385
+ 'renderer. It is likely to have bugs, breaking changes and is ' +
386
+ 'unsupported.'
387
+ );
388
+ });
389
+});
src/renderers/shared/stack/reconciler/ReactOwner.js
+39
-22
@@ -12,8 +12,12 @@
12
13
'use strict';
14
15
+var { ClassComponent } = require('ReactTypeOfWork');
16
+
17
+var emptyObject = require('emptyObject');
18
var invariant = require('invariant');
19
20
+import type { Fiber } from 'ReactFiber';
21
import type { ReactInstance } from 'ReactInstanceType';
22
23
/**
@@ -72,16 +76,22 @@ var ReactOwner = {
76
addComponentAsRefTo: function(
77
component: ReactInstance,
78
ref: string,
75
- owner: ReactInstance,
79
+ owner: ReactInstance | Fiber,
80
): void {
77
- invariant(
78
- isValidOwner(owner),
79
- 'addComponentAsRefTo(...): Only a ReactOwner can have refs. You might ' +
80
- 'be adding a ref to a component that was not created inside a component\'s ' +
81
- '`render` method, or you have multiple copies of React loaded ' +
82
- '(details: https://fb.me/react-refs-must-have-owner).'
83
- );
84
- owner.attachRef(ref, component);
81
+ if (owner && (owner : any).tag === ClassComponent) {
82
+ const inst = (owner : any).stateNode;
83
+ const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs;
84
+ refs[ref] = component.getPublicInstance();
85
+ } else {
86
+ invariant(
87
+ isValidOwner(owner),
88
+ 'addComponentAsRefTo(...): Only a ReactOwner can have refs. You might ' +
89
+ 'be adding a ref to a component that was not created inside a component\'s ' +
90
+ '`render` method, or you have multiple copies of React loaded ' +
91
+ '(details: https://fb.me/react-refs-must-have-owner).'
92
+ );
93
+ (owner : any).attachRef(ref, component);
94
+ }
95
},
96
97
/**
@@ -96,20 +106,27 @@ var ReactOwner = {
106
removeComponentAsRefFrom: function(
107
component: ReactInstance,
108
ref: string,
99
- owner: ReactInstance,
109
+ owner: ReactInstance | Fiber,
110
): void {
101
- invariant(
102
- isValidOwner(owner),
103
- 'removeComponentAsRefFrom(...): Only a ReactOwner can have refs. You might ' +
104
- 'be removing a ref to a component that was not created inside a component\'s ' +
105
- '`render` method, or you have multiple copies of React loaded ' +
106
- '(details: https://fb.me/react-refs-must-have-owner).'
107
- );
108
- var ownerPublicInstance = owner.getPublicInstance();
109
- // Check that `component`'s owner is still alive and that `component` is still the current ref
110
- // because we do not want to detach the ref if another component stole it.
111
- if (ownerPublicInstance && ownerPublicInstance.refs[ref] === component.getPublicInstance()) {
112
- owner.detachRef(ref);
111
+ if (owner && (owner : any).tag === ClassComponent) {
112
+ const inst = (owner : any).stateNode;
113
+ if (inst && inst.refs[ref] === component.getPublicInstance()) {
114
+ delete inst.refs[ref];
115
+ }
116
+ } else {
117
+ invariant(
118
+ isValidOwner(owner),
119
+ 'removeComponentAsRefFrom(...): Only a ReactOwner can have refs. You might ' +
120
+ 'be removing a ref to a component that was not created inside a component\'s ' +
121
+ '`render` method, or you have multiple copies of React loaded ' +
122
+ '(details: https://fb.me/react-refs-must-have-owner).'
123
+ );
124
+ var ownerPublicInstance = (owner : any).getPublicInstance();
125
+ // Check that `component`'s owner is still alive and that `component` is still the current ref
126
+ // because we do not want to detach the ref if another component stole it.
127
+ if (ownerPublicInstance && ownerPublicInstance.refs[ref] === component.getPublicInstance()) {
128
+ (owner : any).detachRef(ref);
129
+ }
130
}
131
},
132