Skip to content

Commit 214e910

Browse files
committed
Merge pull request #423 from andreypopp/master
Re-rendering components into a document leaks components
2 parents 25ba629 + ac9dd92 commit 214e910

3 files changed

Lines changed: 79 additions & 2 deletions

File tree

src/core/ReactMount.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,10 @@ var ReactMount = {
430430
unmountComponentFromNode: function(instance, container) {
431431
instance.unmountComponent();
432432

433+
if (container.nodeType === DOC_NODE_TYPE) {
434+
container = container.documentElement;
435+
}
436+
433437
// http://jsperf.com/emptying-a-node
434438
while (container.lastChild) {
435439
container.removeChild(container.lastChild);
@@ -592,6 +596,8 @@ var ReactMount = {
592596

593597
ATTR_NAME: ATTR_NAME,
594598

599+
getReactRootID: getReactRootID,
600+
595601
getID: getID,
596602

597603
setID: setID,

src/core/__tests__/ReactRenderDocument-test.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,66 @@ describe('rendering React components at document', function() {
3939
testDocument = getTestDocument();
4040
});
4141

42+
it('should be able to get root component id for document node', function() {
43+
if (!testDocument) {
44+
// These tests are not applicable in jst, since jsdom is buggy.
45+
return;
46+
}
47+
48+
var Root = React.createClass({
49+
render: function() {
50+
return (
51+
<html>
52+
<head>
53+
<title>Hello World</title>
54+
</head>
55+
<body>
56+
Hello world
57+
</body>
58+
</html>
59+
);
60+
}
61+
});
62+
63+
ReactMount.allowFullPageRender = true;
64+
var component = React.renderComponent(<Root />, testDocument);
65+
expect(testDocument.body.innerHTML).toBe(' Hello world ');
66+
67+
var componentID = ReactMount.getReactRootID(testDocument);
68+
expect(componentID).toBe(component._rootNodeID);
69+
});
70+
71+
it('should be able to unmount component from document node', function() {
72+
if (!testDocument) {
73+
// These tests are not applicable in jst, since jsdom is buggy.
74+
return;
75+
}
76+
77+
var Root = React.createClass({
78+
render: function() {
79+
return (
80+
<html>
81+
<head>
82+
<title>Hello World</title>
83+
</head>
84+
<body>
85+
Hello world
86+
</body>
87+
</html>
88+
);
89+
}
90+
});
91+
92+
ReactMount.allowFullPageRender = true;
93+
React.renderComponent(<Root />, testDocument);
94+
expect(testDocument.body.innerHTML).toBe(' Hello world ');
95+
96+
var unmounted = React.unmountComponentAtNode(testDocument);
97+
expect(unmounted).toBe(true);
98+
expect(testDocument.documentElement).not.toBe(null);
99+
expect(testDocument.documentElement.innerHTML).toBe('');
100+
});
101+
42102
it('should be able to switch root constructors via state', function() {
43103
if (!testDocument) {
44104
// These tests are not applicable in jst, since jsdom is buggy.

src/core/getReactRootElementInContainer.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,23 @@
1818

1919
"use strict";
2020

21+
var DOC_NODE_TYPE = 9;
22+
2123
/**
22-
* @param {DOMElement} container DOM element that may contain a React component
24+
* @param {DOMElement|DOMDocument} container DOM element that may contain
25+
* a React component
2326
* @return {?*} DOM element that may have the reactRoot ID, or null.
2427
*/
2528
function getReactRootElementInContainer(container) {
26-
return container && container.firstChild;
29+
if (!container) {
30+
return null;
31+
}
32+
33+
if (container.nodeType === DOC_NODE_TYPE) {
34+
return container.documentElement;
35+
} else {
36+
return container.firstChild;
37+
}
2738
}
2839

2940
module.exports = getReactRootElementInContainer;

0 commit comments

Comments
 (0)