Fixed descrepancy between host and class component refs (#12178)
When a ref is removed from a class component, React now calls the previous ref-setter (if there was one) with null. Previously this was the case only for host component refs. A new test has been added.
Brian Vaughn committed
Feb 7, 2018 at 12:13 UTC
d529d2035e3fe741acdf1f01619326588fcca697
2 files changed
+46
-10
packages/react-dom/src/__tests__/refs-destruction-test.js
+42
-9
@@ -23,15 +23,31 @@ describe('refs-destruction', () => {
23
ReactDOM = require('react-dom');
24
ReactTestUtils = require('react-dom/test-utils');
25
26
+ class ClassComponent extends React.Component {
27
+ render() {
28
+ return null;
29
+ }
30
+ }
31
+
32
TestComponent = class extends React.Component {
33
render() {
28
- return (
29
- <div>
30
- {this.props.destroy ? null : (
31
- <div ref="theInnerDiv">Lets try to destroy this.</div>
32
- )}
33
- </div>
34
- );
34
+ if (this.props.destroy) {
35
+ return <div />;
36
+ } else if (this.props.removeRef) {
37
+ return (
38
+ <div>
39
+ <div />
40
+ <ClassComponent />
41
+ </div>
42
+ );
43
+ } else {
44
+ return (
45
+ <div>
46
+ <div ref="theInnerDiv" />
47
+ <ClassComponent ref="theInnerClassComponent" />
48
+ </div>
49
+ );
50
+ }
51
}
52
};
53
});
@@ -45,7 +61,7 @@ describe('refs-destruction', () => {
61
expect(
62
Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
63
.length,
48
- ).toEqual(1);
64
+ ).toEqual(2);
65
ReactDOM.unmountComponentAtNode(container);
66
expect(
67
Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
@@ -62,7 +78,7 @@ describe('refs-destruction', () => {
78
expect(
79
Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
80
.length,
65
- ).toEqual(1);
81
+ ).toEqual(2);
82
ReactDOM.render(<TestComponent destroy={true} />, container);
83
expect(
84
Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
@@ -70,6 +86,23 @@ describe('refs-destruction', () => {
86
).toEqual(0);
87
});
88
89
+ it('should remove refs when removing the child ref attribute', () => {
90
+ const container = document.createElement('div');
91
+ const testInstance = ReactDOM.render(<TestComponent />, container);
92
+ expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)).toBe(
93
+ true,
94
+ );
95
+ expect(
96
+ Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
97
+ .length,
98
+ ).toEqual(2);
99
+ ReactDOM.render(<TestComponent removeRef={true} />, container);
100
+ expect(
101
+ Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key])
102
+ .length,
103
+ ).toEqual(0);
104
+ });
105
+
106
it('should not error when destroying child with ref asynchronously', () => {
107
class Modal extends React.Component {
108
componentDidMount() {
packages/react-reconciler/src/ReactFiberBeginWork.js
+4
-1
@@ -184,7 +184,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
184
185
function markRef(current: Fiber | null, workInProgress: Fiber) {
186
const ref = workInProgress.ref;
187
- if (ref !== null && (!current || current.ref !== ref)) {
187
+ if (
188
+ (current === null && ref !== null) ||
189
+ (current !== null && current.ref !== ref)
190
+ ) {
191
// Schedule a Ref effect
192
workInProgress.effectTag |= Ref;
193
}