@samitouri / QOS-React-2 / commits / 9b3342ed34

Throw when unmounting <html>, <head>, <title> or <body>

These are dangerous from a cross-browser perspective. I think supporting efficient reactive updates will be prohibitively expensive and we'll never prioritize it. Instead of killing this capability entirely, let's just throw on the dangerous cases. There will be a few follow-up diffs

Pete Hunt committed Jan 6, 2014 at 09:36 UTC 9b3342ed3425ae89007811dd8b5b2b20918ad6d4
3 files changed +92 -67
src/core/ReactDefaultInjection.js
+8 -1
@@ -51,6 +51,8 @@ var SimpleEventPlugin = require('SimpleEventPlugin');
51 var ReactDefaultBatchingStrategy = require('ReactDefaultBatchingStrategy');
52 var ReactUpdates = require('ReactUpdates');
53
54 +var createFullPageComponent = require('createFullPageComponent');
55 +
56 function inject() {
57 ReactEventEmitter.TopLevelCallbackCreator = ReactEventTopLevelCallback;
58 /**
@@ -79,7 +81,12 @@ function inject() {
81 input: ReactDOMInput,
82 option: ReactDOMOption,
83 select: ReactDOMSelect,
82 - textarea: ReactDOMTextarea
84 + textarea: ReactDOMTextarea,
85 +
86 + html: createFullPageComponent(ReactDOM.html),
87 + head: createFullPageComponent(ReactDOM.head),
88 + title: createFullPageComponent(ReactDOM.title),
89 + body: createFullPageComponent(ReactDOM.body)
90 });
91
92 DOMProperty.injection.injectDOMPropertyConfig(DefaultDOMPropertyConfig);
src/core/__tests__/ReactRenderDocument-test.js
+21 -66
@@ -65,7 +65,7 @@ describe('rendering React components at document', function() {
65 expect(componentID).toBe(component._rootNodeID);
66 });
67
68 - it('should be able to unmount component from document node', function() {
68 + it('should not be able to unmount component from document node', function() {
69 expect(testDocument).not.toBeUndefined();
70
71 var Root = React.createClass({
@@ -87,72 +87,19 @@ describe('rendering React components at document', function() {
87 React.renderComponent(<Root />, testDocument);
88 expect(testDocument.body.innerHTML).toBe(' Hello world ');
89
90 - var unmounted = React.unmountComponentAtNode(testDocument);
91 - expect(unmounted).toBe(true);
92 - expect(testDocument.documentElement).not.toBe(null);
93 - expect(testDocument.documentElement.innerHTML).toBe('');
94 - });
95 -
96 - it('should be able to switch root constructors via state', function() {
97 - expect(testDocument).not.toBeUndefined();
98 -
99 - var Component = React.createClass({
100 - render: function() {
101 - return (
102 - <html>
103 - <head>
104 - <title>Hello World</title>
105 - </head>
106 - <body>
107 - Hello world
108 - </body>
109 - </html>
110 - );
111 - }
112 - });
113 -
114 - var Component2 = React.createClass({
115 - render: function() {
116 - return (
117 - <html>
118 - <head>
119 - <title>Hello World</title>
120 - </head>
121 - <body>
122 - Goodbye world
123 - </body>
124 - </html>
125 - );
126 - }
127 - });
128 -
129 - var Root = React.createClass({
130 - getInitialState: function() {
131 - return {toggled: false};
132 - },
133 - toggle: function() {
134 - this.setState({toggled: !this.state.toggled});
135 - },
136 - render: function() {
137 - if (this.state.toggled) {
138 - return <Component2 />;
139 - }
140 - return <Component />;
141 - }
142 - });
143 -
144 - ReactMount.allowFullPageRender = true;
145 - var component = React.renderComponent(<Root />, testDocument);
90 + expect(function() {
91 + React.unmountComponentAtNode(testDocument);
92 + }).toThrow(
93 + 'Invariant Violation: ReactFullPageComponenthtml tried to unmount. ' +
94 + 'Because of cross-browser quirks it is impossible to unmount some ' +
95 + 'top-level components (eg <html>, <head>, and <body>) reliably and ' +
96 + 'efficiently. To fix this, have a single top-level component that ' +
97 + 'never unmounts render these elements.'
98 + );
99 expect(testDocument.body.innerHTML).toBe(' Hello world ');
147 -
148 - // Reactive update via state transition
149 - component.toggle();
150 -
151 - expect(testDocument.body.innerHTML).toBe(' Goodbye world ');
152 -
100 });
101
155 - it('should be able to switch root constructors', function() {
102 + it('should not be able to switch root constructors', function() {
103 expect(testDocument).not.toBeUndefined();
104
105 var Component = React.createClass({
@@ -191,9 +138,17 @@ describe('rendering React components at document', function() {
138 expect(testDocument.body.innerHTML).toBe(' Hello world ');
139
140 // Reactive update
194 - React.renderComponent(<Component2 />, testDocument);
141 + expect(function() {
142 + React.renderComponent(<Component2 />, testDocument);
143 + }).toThrow(
144 + 'Invariant Violation: ReactFullPageComponenthtml tried to unmount. ' +
145 + 'Because of cross-browser quirks it is impossible to unmount some ' +
146 + 'top-level components (eg <html>, <head>, and <body>) reliably and ' +
147 + 'efficiently. To fix this, have a single top-level component that ' +
148 + 'never unmounts render these elements.'
149 + );
150
196 - expect(testDocument.body.innerHTML).toBe(' Goodbye world ');
151 + expect(testDocument.body.innerHTML).toBe(' Hello world ');
152
153 });
154
src/dom/components/createFullPageComponent.js new
+63
@@ -0,0 +1,63 @@
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 createFullPageComponent
17 + * @typechecks
18 + */
19 +
20 +"use strict";
21 +
22 +// Defeat circular references by requiring this directly.
23 +var ReactCompositeComponent = require('ReactCompositeComponent');
24 +
25 +var invariant = require('invariant');
26 +
27 +/**
28 + * Create a component that will throw an exception when unmounted.
29 + *
30 + * Components like <html> <head> and <body> can't be removed or added
31 + * easily in a cross-browser way, however it's valuable to be able to
32 + * take advantage of React's reconciliation for styling and <title>
33 + * management. So we just document it and throw in dangerous cases.
34 + *
35 + * @param {function} componentClass convenience constructor to wrap
36 + * @return {function} convenience constructor of new component
37 + */
38 +function createFullPageComponent(componentClass) {
39 + var FullPageComponent = ReactCompositeComponent.createClass({
40 + displayName: 'ReactFullPageComponent' + (
41 + componentClass.componentConstructor.displayName || ''
42 + ),
43 +
44 + componentWillUnmount: function() {
45 + invariant(
46 + false,
47 + '%s tried to unmount. Because of cross-browser quirks it is ' +
48 + 'impossible to unmount some top-level components (eg <html>, <head>, ' +
49 + ' and <body>) reliably and efficiently. To fix this, have a single ' +
50 + ' top-level component that never unmounts render these elements.',
51 + this.constructor.displayName
52 + );
53 + },
54 +
55 + render: function() {
56 + return this.transferPropsTo(componentClass(null, this.props.children));
57 + }
58 + });
59 +
60 + return FullPageComponent;
61 +}
62 +
63 +module.exports = createFullPageComponent;