Fix memory leak with renderComponentToString()
This is leaking memory. Move event registration to mount time.
Pete Hunt committed
Jan 24, 2014 at 17:10 UTC
526be1570e1c08c381d88e7c54703b9f8356cb1e
7 files changed
+159
-11
src/core/ReactDOMComponent.js
+14
-9
@@ -64,7 +64,7 @@ function assertValidProps(props) {
64
);
65
}
66
67
-function putListener(id, registrationName, listener) {
67
+function putListener(id, registrationName, listener, transaction) {
68
var container = ReactMount.findReactContainerForID(id);
69
if (container) {
70
var doc = container.nodeType === ELEMENT_NODE_TYPE ?
@@ -72,8 +72,11 @@ function putListener(id, registrationName, listener) {
72
container;
73
listenTo(registrationName, doc);
74
}
75
-
76
- ReactEventEmitter.putListener(id, registrationName, listener);
75
+ transaction.getPutListenerQueue().enqueuePutListener(
76
+ id,
77
+ registrationName,
78
+ listener
79
+ );
80
}
81
82
@@ -112,7 +115,7 @@ ReactDOMComponent.Mixin = {
115
);
116
assertValidProps(this.props);
117
return (
115
- this._createOpenTagMarkup() +
118
+ this._createOpenTagMarkupAndPutListeners(transaction) +
119
this._createContentMarkup(transaction) +
120
this._tagClose
121
);
@@ -128,9 +131,10 @@ ReactDOMComponent.Mixin = {
131
* @see http://jsperf.com/obj-vs-arr-iteration
132
*
133
* @private
134
+ * @param {ReactReconcileTransaction} transaction
135
* @return {string} Markup of opening tag.
136
*/
133
- _createOpenTagMarkup: function() {
137
+ _createOpenTagMarkupAndPutListeners: function(transaction) {
138
var props = this.props;
139
var ret = this._tagOpen;
140
@@ -143,7 +147,7 @@ ReactDOMComponent.Mixin = {
147
continue;
148
}
149
if (registrationNameModules[propKey]) {
146
- putListener(this._rootNodeID, propKey, propValue);
150
+ putListener(this._rootNodeID, propKey, propValue, transaction);
151
} else {
152
if (propKey === STYLE) {
153
if (propValue) {
@@ -222,7 +226,7 @@ ReactDOMComponent.Mixin = {
226
prevProps,
227
prevOwner
228
);
225
- this._updateDOMProperties(prevProps);
229
+ this._updateDOMProperties(prevProps, transaction);
230
this._updateDOMChildren(prevProps, transaction);
231
}
232
),
@@ -240,8 +244,9 @@ ReactDOMComponent.Mixin = {
244
*
245
* @private
246
* @param {object} lastProps
247
+ * @param {ReactReconcileTransaction} transaction
248
*/
244
- _updateDOMProperties: function(lastProps) {
249
+ _updateDOMProperties: function(lastProps, transaction) {
250
var nextProps = this.props;
251
var propKey;
252
var styleName;
@@ -302,7 +307,7 @@ ReactDOMComponent.Mixin = {
307
styleUpdates = nextProp;
308
}
309
} else if (registrationNameModules[propKey]) {
305
- putListener(this._rootNodeID, propKey, nextProp);
310
+ putListener(this._rootNodeID, propKey, nextProp, transaction);
311
} else if (
312
DOMProperty.isStandardName[propKey] ||
313
DOMProperty.isCustomAttribute(propKey)) {
src/core/ReactPutListenerQueue.js
new
+61
@@ -0,0 +1,61 @@
1
+/**
2
+ * Copyright 2013 Facebook, Inc.
3
+ *
4
+ * Licensed under the Apache License, Version 2.0 (the "License");
5
+ * you may not use this file except in compliance with the License.
6
+ * You may obtain a copy of the License at
7
+ *
8
+ * http://www.apache.org/licenses/LICENSE-2.0
9
+ *
10
+ * Unless required by applicable law or agreed to in writing, software
11
+ * distributed under the License is distributed on an "AS IS" BASIS,
12
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13
+ * See the License for the specific language governing permissions and
14
+ * limitations under the License.
15
+ *
16
+ * @providesModule ReactPutListenerQueue
17
+ */
18
+
19
+"use strict";
20
+
21
+var PooledClass = require('PooledClass');
22
+var ReactEventEmitter = require('ReactEventEmitter');
23
+
24
+var mixInto = require('mixInto');
25
+
26
+function ReactPutListenerQueue() {
27
+ this.listenersToPut = [];
28
+}
29
+
30
+mixInto(ReactPutListenerQueue, {
31
+ enqueuePutListener: function(rootNodeID, propKey, propValue) {
32
+ this.listenersToPut.push({
33
+ rootNodeID: rootNodeID,
34
+ propKey: propKey,
35
+ propValue: propValue
36
+ });
37
+ },
38
+
39
+ putListeners: function() {
40
+ for (var i = 0; i < this.listenersToPut.length; i++) {
41
+ var listenerToPut = this.listenersToPut[i];
42
+ ReactEventEmitter.putListener(
43
+ listenerToPut.rootNodeID,
44
+ listenerToPut.propKey,
45
+ listenerToPut.propValue
46
+ );
47
+ }
48
+ },
49
+
50
+ reset: function() {
51
+ this.listenersToPut.length = 0;
52
+ },
53
+
54
+ destructor: function() {
55
+ this.reset();
56
+ }
57
+});
58
+
59
+PooledClass.addPoolingTo(ReactPutListenerQueue);
60
+
61
+module.exports = ReactPutListenerQueue;
src/core/ReactReconcileTransaction.js
+20
@@ -24,6 +24,7 @@ var PooledClass = require('PooledClass');
24
var ReactEventEmitter = require('ReactEventEmitter');
25
var ReactInputSelection = require('ReactInputSelection');
26
var ReactMountReady = require('ReactMountReady');
27
+var ReactPutListenerQueue = require('ReactPutListenerQueue');
28
var Transaction = require('Transaction');
29
30
var mixInto = require('mixInto');
@@ -88,12 +89,23 @@ var ON_DOM_READY_QUEUEING = {
89
}
90
};
91
92
+var PUT_LISTENER_QUEUEING = {
93
+ initialize: function() {
94
+ this.putListenerQueue.reset();
95
+ },
96
+
97
+ close: function() {
98
+ this.putListenerQueue.putListeners();
99
+ }
100
+};
101
+
102
/**
103
* Executed within the scope of the `Transaction` instance. Consider these as
104
* being member methods, but with an implied ordering while being isolated from
105
* each other.
106
*/
107
var TRANSACTION_WRAPPERS = [
108
+ PUT_LISTENER_QUEUEING,
109
SELECTION_RESTORATION,
110
EVENT_SUPPRESSION,
111
ON_DOM_READY_QUEUEING
@@ -116,6 +128,7 @@ var TRANSACTION_WRAPPERS = [
128
function ReactReconcileTransaction() {
129
this.reinitializeTransaction();
130
this.reactMountReady = ReactMountReady.getPooled(null);
131
+ this.putListenerQueue = ReactPutListenerQueue.getPooled();
132
}
133
134
var Mixin = {
@@ -142,6 +155,10 @@ var Mixin = {
155
return this.reactMountReady;
156
},
157
158
+ getPutListenerQueue: function() {
159
+ return this.putListenerQueue;
160
+ },
161
+
162
/**
163
* `PooledClass` looks for this, and will invoke this before allowing this
164
* instance to be resused.
@@ -149,6 +166,9 @@ var Mixin = {
166
destructor: function() {
167
ReactMountReady.release(this.reactMountReady);
168
this.reactMountReady = null;
169
+
170
+ ReactPutListenerQueue.release(this.putListenerQueue);
171
+ this.putListenerQueue = null;
172
}
173
};
174
src/core/__tests__/ReactDOMComponent-test.js
+1
-1
@@ -229,7 +229,7 @@ describe('ReactDOMComponent', function() {
229
mixInto(NodeStub, ReactDOMComponent.Mixin);
230
231
genMarkup = function(props) {
232
- return (new NodeStub(props))._createOpenTagMarkup();
232
+ return (new NodeStub(props))._createOpenTagMarkupAndPutListeners();
233
};
234
235
this.addMatchers({
src/core/putDOMComponentListener.js
new
+40
@@ -0,0 +1,40 @@
1
+/**
2
+ * Copyright 2013 Facebook, Inc.
3
+ *
4
+ * Licensed under the Apache License, Version 2.0 (the "License");
5
+ * you may not use this file except in compliance with the License.
6
+ * You may obtain a copy of the License at
7
+ *
8
+ * http://www.apache.org/licenses/LICENSE-2.0
9
+ *
10
+ * Unless required by applicable law or agreed to in writing, software
11
+ * distributed under the License is distributed on an "AS IS" BASIS,
12
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13
+ * See the License for the specific language governing permissions and
14
+ * limitations under the License.
15
+ *
16
+ * @providesModule putDOMComponentListener
17
+ */
18
+
19
+"use strict";
20
+
21
+var ReactEventEmitter = require('ReactEventEmitter');
22
+var ReactMount = require('ReactMount');
23
+
24
+var listenTo = ReactEventEmitter.listenTo;
25
+
26
+var ELEMENT_NODE_TYPE = 1;
27
+
28
+function putDOMComponentListener(id, registrationName, listener) {
29
+ var container = ReactMount.findReactContainerForID(id);
30
+ if (container) {
31
+ var doc = container.nodeType === ELEMENT_NODE_TYPE ?
32
+ container.ownerDocument :
33
+ container;
34
+ listenTo(registrationName, doc);
35
+ }
36
+
37
+ ReactEventEmitter.putListener(id, registrationName, listener);
38
+}
39
+
40
+module.exports = putDOMComponentListener;
src/environment/__tests__/ReactServerRendering-test.js
+12
@@ -69,6 +69,18 @@ describe('ReactServerRendering', function() {
69
);
70
});
71
72
+ it('should not register event listeners', function() {
73
+ var EventPluginHub = require('EventPluginHub');
74
+ var cb = mocks.getMockFunction();
75
+
76
+ ReactServerRendering.renderComponentToString(
77
+ <span onClick={cb}>hello world</span>,
78
+ function() {
79
+ expect(EventPluginHub.__getListenerBank()).toEqual({});
80
+ }
81
+ );
82
+ });
83
+
84
it('should render composite components', function() {
85
var Parent = React.createClass({
86
render: function() {
src/event/EventPluginHub.js
+11
-1
@@ -20,6 +20,7 @@
20
21
var EventPluginRegistry = require('EventPluginRegistry');
22
var EventPluginUtils = require('EventPluginUtils');
23
+var ExecutionEnvironment = require('ExecutionEnvironment');
24
25
var accumulate = require('accumulate');
26
var forEachAccumulated = require('forEachAccumulated');
@@ -150,6 +151,11 @@ var EventPluginHub = {
151
* @param {?function} listener The callback to store.
152
*/
153
putListener: function(id, registrationName, listener) {
154
+ invariant(
155
+ ExecutionEnvironment.canUseDOM,
156
+ 'Cannot call putListener() in a non-DOM environment.'
157
+ );
158
+
159
if (__DEV__) {
160
// IE8 has no API for event capturing and the `onScroll` event doesn't
161
// bubble.
@@ -265,10 +271,14 @@ var EventPluginHub = {
271
},
272
273
/**
268
- * This is needed for tests only. Do not use!
274
+ * These are needed for tests only. Do not use!
275
*/
276
__purge: function() {
277
listenerBank = {};
278
+ },
279
+
280
+ __getListenerBank: function() {
281
+ return listenerBank;
282
}
283
284
};