Cache native node on native components, return it when unmounting
This is probably slightly slower for unmounts in the case that no updates were ever performed, but caching the node on the instance should make updates faster. In any case, the more important consequence of this change is that we can fix the current memory leak that happens when swapping composite types.
Ben Alpert committed
Sep 26, 2015 at 13:59 UTC
743ccf090fcf1d769df7b146f17a6e6726483051
5 files changed
+50
-36
src/renderers/dom/shared/ReactDOMComponent.js
+35
-32
@@ -357,7 +357,7 @@ function trapBubbledEventsLocal() {
357
// If a component renders to null or if another component fatals and causes
358
// the state of the tree to be corrupted, `node` here can be null.
359
invariant(inst._rootNodeID, 'Must be mounted to trap events');
360
- var node = ReactMount.getNode(inst._rootNodeID);
360
+ var node = getNode(inst);
361
invariant(
362
node,
363
'trapBubbledEvent(...): Requires node to be rendered.'
@@ -493,6 +493,14 @@ function isCustomComponent(tagName, props) {
493
return tagName.indexOf('-') >= 0 || props.is != null;
494
}
495
496
+function getNode(inst) {
497
+ if (inst._nativeNode) {
498
+ return inst._nativeNode;
499
+ } else {
500
+ return inst._nativeNode = ReactMount.getNode(inst._rootNodeID);
501
+ }
502
+}
503
+
504
/**
505
* Creates a new React class that is idempotent and capable of containing other
506
* React components. It accepts event listeners and DOM properties that are
@@ -513,10 +521,11 @@ function ReactDOMComponent(tag) {
521
this._renderedChildren = null;
522
this._previousStyle = null;
523
this._previousStyleCopy = null;
524
+ this._nativeNode = null;
525
this._rootNodeID = null;
526
this._wrapperState = null;
527
this._topLevelWrapper = null;
519
- this._nodeWithLegacyProperties = null;
528
+ this._nodeHasLegacyProperties = false;
529
if (__DEV__) {
530
this._unprocessedContextDev = null;
531
this._processedContextDev = null;
@@ -600,10 +609,11 @@ ReactDOMComponent.Mixin = {
609
if (transaction.useCreateElement) {
610
var ownerDocument = context[ReactMount.ownerDocumentContextKey];
611
var el = ownerDocument.createElement(this._currentElement.type);
612
+ this._nativeNode = el;
613
DOMPropertyOperations.setAttributeForID(el, this._rootNodeID);
614
// Populate node cache
615
ReactMount.getID(el);
606
- this._updateDOMProperties({}, props, transaction, el);
616
+ this._updateDOMProperties({}, props, transaction);
617
this._createInitialChildren(transaction, props, context, el);
618
mountImage = el;
619
} else {
@@ -844,7 +854,7 @@ ReactDOMComponent.Mixin = {
854
855
856
assertValidProps(this, nextProps);
847
- this._updateDOMProperties(lastProps, nextProps, transaction, null);
857
+ this._updateDOMProperties(lastProps, nextProps, transaction);
858
this._updateDOMChildren(
859
lastProps,
860
nextProps,
@@ -852,8 +862,8 @@ ReactDOMComponent.Mixin = {
862
context
863
);
864
855
- if (!canDefineProperty && this._nodeWithLegacyProperties) {
856
- this._nodeWithLegacyProperties.props = nextProps;
865
+ if (!canDefineProperty && this._nodeHasLegacyProperties) {
866
+ this._nativeNode.props = nextProps;
867
}
868
869
if (this._tag === 'select') {
@@ -877,10 +887,9 @@ ReactDOMComponent.Mixin = {
887
* @private
888
* @param {object} lastProps
889
* @param {object} nextProps
880
- * @param {ReactReconcileTransaction} transaction
890
* @param {?DOMElement} node
891
*/
883
- _updateDOMProperties: function(lastProps, nextProps, transaction, node) {
892
+ _updateDOMProperties: function(lastProps, nextProps, transaction) {
893
var propKey;
894
var styleName;
895
var styleUpdates;
@@ -908,10 +917,7 @@ ReactDOMComponent.Mixin = {
917
} else if (
918
DOMProperty.properties[propKey] ||
919
DOMProperty.isCustomAttribute(propKey)) {
911
- if (!node) {
912
- node = ReactMount.getNode(this._rootNodeID);
913
- }
914
- DOMPropertyOperations.deleteValueForProperty(node, propKey);
920
+ DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey);
921
}
922
}
923
for (propKey in nextProps) {
@@ -964,20 +970,15 @@ ReactDOMComponent.Mixin = {
970
deleteListener(this._rootNodeID, propKey);
971
}
972
} else if (isCustomComponent(this._tag, nextProps)) {
967
- if (!node) {
968
- node = ReactMount.getNode(this._rootNodeID);
969
- }
973
DOMPropertyOperations.setValueForAttribute(
971
- node,
974
+ getNode(this),
975
propKey,
976
nextProp
977
);
978
} else if (
979
DOMProperty.properties[propKey] ||
980
DOMProperty.isCustomAttribute(propKey)) {
978
- if (!node) {
979
- node = ReactMount.getNode(this._rootNodeID);
980
- }
981
+ var node = getNode(this);
982
// If we're updating to null or undefined, we should remove the property
983
// from the DOM node instead of inadvertantly setting to a string. This
984
// brings us in line with the same behavior we have on initial render.
@@ -989,10 +990,7 @@ ReactDOMComponent.Mixin = {
990
}
991
}
992
if (styleUpdates) {
992
- if (!node) {
993
- node = ReactMount.getNode(this._rootNodeID);
994
- }
995
- CSSPropertyOperations.setValueForStyles(node, styleUpdates);
993
+ CSSPropertyOperations.setValueForStyles(getNode(this), styleUpdates);
994
}
995
},
996
@@ -1089,21 +1087,26 @@ ReactDOMComponent.Mixin = {
1087
break;
1088
}
1089
1090
+ var nativeNode = getNode(this);
1091
+ this._nativeNode = null;
1092
+ if (this._nodeHasLegacyProperties) {
1093
+ nativeNode._reactInternalComponent = null;
1094
+ }
1095
+
1096
this.unmountChildren();
1097
ReactBrowserEventEmitter.deleteAllListeners(this._rootNodeID);
1098
ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);
1099
this._rootNodeID = null;
1100
this._wrapperState = null;
1097
- if (this._nodeWithLegacyProperties) {
1098
- var node = this._nodeWithLegacyProperties;
1099
- node._reactInternalComponent = null;
1100
- this._nodeWithLegacyProperties = null;
1101
- }
1101
+
1102
+ return nativeNode;
1103
},
1104
1105
getPublicInstance: function() {
1105
- if (!this._nodeWithLegacyProperties) {
1106
- var node = ReactMount.getNode(this._rootNodeID);
1106
+ if (this._nodeHasLegacyProperties) {
1107
+ return this._nativeNode;
1108
+ } else {
1109
+ var node = getNode(this);
1110
1111
node._reactInternalComponent = this;
1112
node.getDOMNode = legacyGetDOMNode;
@@ -1126,9 +1129,9 @@ ReactDOMComponent.Mixin = {
1129
node.props = this._currentElement.props;
1130
}
1131
1129
- this._nodeWithLegacyProperties = node;
1132
+ this._nodeHasLegacyProperties = true;
1133
+ return node;
1134
}
1131
- return this._nodeWithLegacyProperties;
1135
},
1136
1137
};
src/renderers/dom/shared/ReactDOMTextComponent.js
+9
-1
@@ -52,6 +52,7 @@ assign(ReactDOMTextComponent.prototype, {
52
// TODO: This is really a ReactText (ReactNode), not a ReactElement
53
this._currentElement = text;
54
this._stringText = '' + text;
55
+ this._nativeNode = null;
56
57
// Properties
58
this._rootNodeID = null;
@@ -82,6 +83,7 @@ assign(ReactDOMTextComponent.prototype, {
83
if (transaction.useCreateElement) {
84
var ownerDocument = context[ReactMount.ownerDocumentContextKey];
85
var el = ownerDocument.createElement('span');
86
+ this._nativeNode = el;
87
DOMPropertyOperations.setAttributeForID(el, rootID);
88
// Populate node cache
89
ReactMount.getID(el);
@@ -121,14 +123,20 @@ assign(ReactDOMTextComponent.prototype, {
123
// and/or updateComponent to do the actual update for consistency with
124
// other component types?
125
this._stringText = nextStringText;
124
- var node = ReactMount.getNode(this._rootNodeID);
126
+ var node = this._nativeNode;
127
+ if (!node) {
128
+ node = this._nativeNode = ReactMount.getNode(this._rootNodeID);
129
+ }
130
DOMChildrenOperations.updateTextContent(node, nextStringText);
131
}
132
}
133
},
134
135
unmountComponent: function() {
136
+ var node = this._nativeNode || ReactMount.getNode(this._rootNodeID);
137
+ this._nativeNode = null;
138
ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);
139
+ return node;
140
},
141
142
});
src/renderers/shared/reconciler/ReactCompositeComponent.js
+3
-1
@@ -310,7 +310,8 @@ var ReactCompositeComponentMixin = {
310
inst.componentWillUnmount();
311
}
312
313
- ReactReconciler.unmountComponent(this._renderedComponent);
313
+ var unmountedNativeNode =
314
+ ReactReconciler.unmountComponent(this._renderedComponent);
315
this._renderedComponent = null;
316
this._instance = null;
317
@@ -339,6 +340,7 @@ var ReactCompositeComponentMixin = {
340
// TODO: inst.props = null;
341
// TODO: inst.state = null;
342
// TODO: inst.context = null;
343
+ return unmountedNativeNode;
344
},
345
346
/**
src/renderers/shared/reconciler/ReactEmptyComponent.js
+2
-1
@@ -46,10 +46,11 @@ assign(ReactEmptyComponent.prototype, {
46
receiveComponent: function() {
47
},
48
unmountComponent: function(rootID, transaction, context) {
49
- ReactReconciler.unmountComponent(this._renderedComponent);
49
+ var nativeNode = ReactReconciler.unmountComponent(this._renderedComponent);
50
ReactEmptyComponentRegistry.deregisterNullComponentID(this._rootNodeID);
51
this._rootNodeID = null;
52
this._renderedComponent = null;
53
+ return nativeNode;
54
},
55
});
56
src/renderers/shared/reconciler/ReactReconciler.js
+1
-1
@@ -50,7 +50,7 @@ var ReactReconciler = {
50
*/
51
unmountComponent: function(internalInstance) {
52
ReactRef.detachRefs(internalInstance, internalInstance._currentElement);
53
- internalInstance.unmountComponent();
53
+ return internalInstance.unmountComponent();
54
},
55
56
/**