Drop the _owner and _lifeCycle field on internal instances
The _owner field is unnecessary since it's reachable from _currentElement. The _lifeCycle field is unnecessary because an internal component should not even need to exist at all if it's unmounted. It should be dereferenced internally, and never exposed externally. The only case where it's important is for batching updates where we currently avoid calling performUpdateIfNecessary if it's mounted. However, this function is already only executed "if necessary" so we just make sure that it's not necessary after unmount by resetting all the pending fields.
Sebastian Markbage committed
Nov 16, 2014 at 16:58 UTC
dac59d10323e35b52d379a1ae7e9815cc5a55aa5
6 files changed
+93
-104
src/core/ReactComponent.js
+4
-63
@@ -20,21 +20,6 @@ var assign = require('Object.assign');
20
var invariant = require('invariant');
21
var keyMirror = require('keyMirror');
22
23
-/**
24
- * Every React component is in one of these life cycles.
25
- */
26
-var ComponentLifeCycle = keyMirror({
27
- /**
28
- * Mounted components have a DOM node representation and are capable of
29
- * receiving new props.
30
- */
31
- MOUNTED: null,
32
- /**
33
- * Unmounted components are inactive and cannot receive new props.
34
- */
35
- UNMOUNTED: null
36
-});
37
-
23
var injected = false;
24
25
/**
@@ -115,11 +100,6 @@ var ReactComponent = {
100
}
101
},
102
118
- /**
119
- * @internal
120
- */
121
- LifeCycle: ComponentLifeCycle,
122
-
103
/**
104
* Injected module that provides ability to mutate individual properties.
105
* Injected into the base class because many different subclasses need access
@@ -137,17 +117,6 @@ var ReactComponent = {
117
*/
118
Mixin: {
119
140
- /**
141
- * Checks whether or not this component is mounted.
142
- *
143
- * @return {boolean} True if mounted, false otherwise.
144
- * @final
145
- * @protected
146
- */
147
- isMounted: function() {
148
- return this._lifeCycleState === ComponentLifeCycle.MOUNTED;
149
- },
150
-
120
/**
121
* Sets a subset of the props.
122
*
@@ -175,10 +144,6 @@ var ReactComponent = {
144
* @public
145
*/
146
replaceProps: function(props, callback) {
178
- invariant(
179
- this.isMounted(),
180
- 'replaceProps(...): Can only update a mounted component.'
181
- );
147
invariant(
148
this._mountDepth === 0,
149
'replaceProps(...): You called `setProps` or `replaceProps` on a ' +
@@ -225,15 +190,6 @@ var ReactComponent = {
190
* @internal
191
*/
192
construct: function(element) {
228
- // Record the component responsible for creating this component.
229
- // This is accessible through the element but we maintain an extra
230
- // field for compatibility with devtools and as a way to make an
231
- // incremental update. TODO: Consider deprecating this field.
232
- this._owner = element._owner;
233
-
234
- // All components start unmounted.
235
- this._lifeCycleState = ComponentLifeCycle.UNMOUNTED;
236
-
193
// See ReactUpdates.
194
this._pendingCallbacks = null;
195
@@ -258,20 +214,12 @@ var ReactComponent = {
214
* @internal
215
*/
216
mountComponent: function(rootID, transaction, mountDepth) {
261
- invariant(
262
- !this.isMounted(),
263
- 'mountComponent(%s, ...): Can only mount an unmounted component. ' +
264
- 'Make sure to avoid storing components between renders or reusing a ' +
265
- 'single component instance in multiple places.',
266
- rootID
267
- );
217
var ref = this._currentElement.ref;
218
if (ref != null) {
219
var owner = this._currentElement._owner;
220
attachRef(ref, this, owner);
221
}
222
this._rootNodeID = rootID;
274
- this._lifeCycleState = ComponentLifeCycle.MOUNTED;
223
this._mountDepth = mountDepth;
224
// Effectively: return '';
225
},
@@ -287,17 +235,15 @@ var ReactComponent = {
235
* @internal
236
*/
237
unmountComponent: function() {
290
- invariant(
291
- this.isMounted(),
292
- 'unmountComponent(): Can only unmount a mounted component.'
293
- );
238
var ref = this._currentElement.ref;
239
if (ref != null) {
296
- detachRef(ref, this, this._owner);
240
+ detachRef(ref, this, this._currentElement._owner);
241
}
242
unmountIDFromEnvironment(this._rootNodeID);
243
+ // Reset all fields
244
this._rootNodeID = null;
300
- this._lifeCycleState = ComponentLifeCycle.UNMOUNTED;
245
+ this._pendingCallbacks = null;
246
+ this._pendingElement = null;
247
},
248
249
/**
@@ -312,10 +258,6 @@ var ReactComponent = {
258
* @internal
259
*/
260
receiveComponent: function(nextElement, transaction) {
315
- invariant(
316
- this.isMounted(),
317
- 'receiveComponent(...): Can only update a mounted component.'
318
- );
261
this._pendingElement = nextElement;
262
this.performUpdateIfNecessary(transaction);
263
},
@@ -333,7 +275,6 @@ var ReactComponent = {
275
var prevElement = this._currentElement;
276
var nextElement = this._pendingElement;
277
this._currentElement = nextElement;
336
- this._owner = nextElement._owner;
278
this._pendingElement = null;
279
this.updateComponent(transaction, prevElement, nextElement);
280
},
src/core/ReactCompositeComponent.js
+7
-14
@@ -28,7 +28,7 @@ var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
28
var warning = require('warning');
29
30
function getDeclarationErrorAddendum(component) {
31
- var owner = component._owner || null;
31
+ var owner = component._currentElement._owner || null;
32
if (owner) {
33
var constructor = owner._instance.constructor;
34
if (constructor && constructor.displayName) {
@@ -56,8 +56,8 @@ function validateLifeCycleOnReplaceState(instance) {
56
* `ReactCompositeComponent` maintains an auxiliary life cycle state in
57
* `this._compositeLifeCycleState` (which can be null).
58
*
59
- * This is different from the life cycle state maintained by `ReactComponent` in
60
- * `this._lifeCycleState`. The following diagram shows how the states overlap in
59
+ * This is different from the life cycle state maintained by `ReactComponent`.
60
+ * The following diagram shows how the states overlap in
61
* time. There are times when the CompositeLifeCycle is null - at those times it
62
* is only meaningful to look at ComponentLifeCycle alone.
63
*
@@ -129,8 +129,7 @@ var ReactCompositeComponentMixin = assign({},
129
* @final
130
*/
131
isMounted: function() {
132
- return ReactComponent.Mixin.isMounted.call(this) &&
133
- this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING;
132
+ return this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING;
133
},
134
135
/**
@@ -232,6 +231,9 @@ var ReactCompositeComponentMixin = assign({},
231
this._renderedComponent.unmountComponent();
232
this._renderedComponent = null;
233
234
+ // Reset pending fields
235
+ this._pendingState = null;
236
+ this._pendingForceUpdate = false;
237
ReactComponent.Mixin.unmountComponent.call(this);
238
239
// Delete the reference from the instance to this internal representation
@@ -554,11 +556,6 @@ var ReactCompositeComponentMixin = assign({},
556
inst.props = nextProps;
557
inst.state = nextState;
558
inst.context = nextContext;
557
-
558
- // Owner cannot change because shouldUpdateReactComponent doesn't allow
559
- // it. TODO: Remove this._owner completely.
560
- this._owner = nextParentElement._owner;
561
-
559
return;
560
}
561
@@ -606,10 +603,6 @@ var ReactCompositeComponentMixin = assign({},
603
inst.state = nextState;
604
inst.context = nextContext;
605
609
- // Owner cannot change because shouldUpdateReactComponent doesn't allow
610
- // it. TODO: Remove this._owner completely.
611
- this._owner = nextElement._owner;
612
-
606
this._updateRenderedComponent(transaction);
607
608
if (inst.componentDidUpdate) {
src/core/ReactUpdates.js
+16
-17
@@ -136,24 +136,23 @@ function runBatchedUpdates(transaction) {
136
dirtyComponents.sort(mountDepthComparator);
137
138
for (var i = 0; i < len; i++) {
139
- // If a component is unmounted before pending changes apply, ignore them
140
- // TODO: Queue unmounts in the same list to avoid this happening at all
139
+ // If a component is unmounted before pending changes apply, it will still
140
+ // be here, but we assume that it has cleared its _pendingCallbacks and
141
+ // that performUpdateIfNecessary is a noop.
142
var component = dirtyComponents[i];
142
- if (component.isMounted()) {
143
- // If performUpdateIfNecessary happens to enqueue any new updates, we
144
- // shouldn't execute the callbacks until the next render happens, so
145
- // stash the callbacks first
146
- var callbacks = component._pendingCallbacks;
147
- component._pendingCallbacks = null;
148
- component.performUpdateIfNecessary(transaction.reconcileTransaction);
149
-
150
- if (callbacks) {
151
- for (var j = 0; j < callbacks.length; j++) {
152
- transaction.callbackQueue.enqueue(
153
- callbacks[j],
154
- component
155
- );
156
- }
143
+ // If performUpdateIfNecessary happens to enqueue any new updates, we
144
+ // shouldn't execute the callbacks until the next render happens, so
145
+ // stash the callbacks first
146
+ var callbacks = component._pendingCallbacks;
147
+ component._pendingCallbacks = null;
148
+ component.performUpdateIfNecessary(transaction.reconcileTransaction);
149
+
150
+ if (callbacks) {
151
+ for (var j = 0; j < callbacks.length; j++) {
152
+ transaction.callbackQueue.enqueue(
153
+ callbacks[j],
154
+ component
155
+ );
156
}
157
}
158
}
src/core/__tests__/ReactComponentLifeCycle-test.js
+22
-9
@@ -11,12 +11,13 @@
11
12
"use strict";
13
14
+var keyMirror = require('keyMirror');
15
+
16
var React;
17
var ReactTestUtils;
18
var ReactComponent;
19
var ReactCompositeComponent;
20
var ReactInstanceMap;
19
-var ComponentLifeCycle;
21
var CompositeComponentLifeCycle;
22
23
var getCompositeLifeCycle;
@@ -69,6 +70,21 @@ var POST_WILL_UNMOUNT_STATE = {
70
hasWillUnmountCompleted: true
71
};
72
73
+/**
74
+ * Every React component is in one of these life cycles.
75
+ */
76
+var ComponentLifeCycle = keyMirror({
77
+ /**
78
+ * Mounted components have a DOM node representation and are capable of
79
+ * receiving new props.
80
+ */
81
+ MOUNTED: null,
82
+ /**
83
+ * Unmounted components are inactive and cannot receive new props.
84
+ */
85
+ UNMOUNTED: null
86
+});
87
+
88
/**
89
* TODO: We should make any setState calls fail in
90
* `getInitialState` and `componentWillMount`. They will usually fail
@@ -83,7 +99,6 @@ describe('ReactComponentLifeCycle', function() {
99
ReactTestUtils = require('ReactTestUtils');
100
ReactComponent = require('ReactComponent');
101
ReactCompositeComponent = require('ReactCompositeComponent');
86
- ComponentLifeCycle = ReactComponent.LifeCycle;
102
CompositeComponentLifeCycle = ReactCompositeComponent.LifeCycle;
103
104
ReactInstanceMap = require('ReactInstanceMap');
@@ -94,13 +109,11 @@ describe('ReactComponentLifeCycle', function() {
109
110
getLifeCycleState = function(instance) {
111
var internalInstance = ReactInstanceMap.get(instance);
97
- if (!internalInstance) {
98
- // Once a component is fully unmounted, we cannot actually get to the
99
- // internal instance. It's already dereferenced and possibly GC:ed.
100
- // So the unmounted life cycle hook doesn't exist anymore.
101
- return ComponentLifeCycle.UNMOUNTED;
102
- }
103
- return internalInstance._lifeCycleState;
112
+ // Once a component gets mounted, it has an internal instance, once it
113
+ // gets unmounted, it loses that internal instance.
114
+ return internalInstance ?
115
+ ComponentLifeCycle.MOUNTED :
116
+ ComponentLifeCycle.UNMOUNTED;
117
};
118
});
119
src/core/__tests__/ReactUpdates-test.js
+42
@@ -815,4 +815,46 @@ describe('ReactUpdates', function() {
815
'asap-1.2'
816
]);
817
});
818
+
819
+ it('does not call render after a component as been deleted', function() {
820
+
821
+ var renderCount = 0;
822
+ var componentB = null;
823
+
824
+ var B = React.createClass({
825
+ getInitialState: function() {
826
+ return { updates: 0 };
827
+ },
828
+ componentDidMount: function() {
829
+ componentB = this;
830
+ },
831
+ render: function() {
832
+ renderCount++;
833
+ return <div />;
834
+ }
835
+ });
836
+
837
+ var A = React.createClass({
838
+ getInitialState: function() {
839
+ return { showB: true };
840
+ },
841
+ render: function() {
842
+ return this.state.showB ? <B /> : <div />;
843
+ }
844
+ });
845
+
846
+ var container = document.createElement('div');
847
+
848
+ var component = React.render(<A />, container);
849
+
850
+ ReactUpdates.batchedUpdates(function() {
851
+ // B will have scheduled an update but the batching should ensure that its
852
+ // update never fires.
853
+ componentB.setState({ updates: 1 });
854
+ component.setState({ showB: false });
855
+ });
856
+
857
+ expect(renderCount).toBe(1);
858
+ });
859
+
860
});
src/test/ReactDefaultPerf.js
+2
-1
@@ -244,7 +244,8 @@ var ReactDefaultPerf = {
244
245
entry.displayNames[rootNodeID] = {
246
current: this.constructor.displayName,
247
- owner: this._owner ? this._owner.constructor.displayName : '<root>'
247
+ owner: this._currentElement._owner ?
248
+ this._currentElement._owner.constructor.displayName : '<root>'
249
};
250
251
return rv;