@samitouri / QOS-React-2 / commits / ac9dd92272

Fix unmounting components mounted into doc element

If we are to unmount a component mounted into a document element we should unmount it from document.documentElement and not from document.firstChild which is a doctype element in this specific case.

Andrey Popp committed Oct 15, 2013 at 03:21 UTC ac9dd922726b5e886cada4788cf43906f18aedf5
3 files changed +79 -2
src/core/ReactMount.js
+6
@@ -430,6 +430,10 @@ var ReactMount = {
430 unmountComponentFromNode: function(instance, container) {
431 instance.unmountComponent();
432
433 + if (container.nodeType === DOC_NODE_TYPE) {
434 + container = container.documentElement;
435 + }
436 +
437 // http://jsperf.com/emptying-a-node
438 while (container.lastChild) {
439 container.removeChild(container.lastChild);
@@ -592,6 +596,8 @@ var ReactMount = {
596
597 ATTR_NAME: ATTR_NAME,
598
599 + getReactRootID: getReactRootID,
600 +
601 getID: getID,
602
603 setID: setID,
src/core/__tests__/ReactRenderDocument-test.js
+60
@@ -39,6 +39,66 @@ describe('rendering React components at document', function() {
39 testDocument = getTestDocument();
40 });
41
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 +
102 it('should be able to switch root constructors via state', function() {
103 if (!testDocument) {
104 // These tests are not applicable in jst, since jsdom is buggy.
src/core/getReactRootElementInContainer.js
+13 -2
@@ -18,12 +18,23 @@
18
19 "use strict";
20
21 +var DOC_NODE_TYPE = 9;
22 +
23 /**
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
26 * @return {?*} DOM element that may have the reactRoot ID, or null.
27 */
28 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 + }
38 }
39
40 module.exports = getReactRootElementInContainer;