From 40bb9c3a7b18342ab1cab087fd243436a56385a8 Mon Sep 17 00:00:00 2001 From: Andrey Popp <8mayday@gmail.com> Date: Thu, 21 Nov 2013 12:59:45 +0400 Subject: [PATCH 1/2] Remove support for full page rendering --- src/core/ReactComponentBrowserEnvironment.js | 15 +- src/core/ReactMount.js | 15 +- .../__tests__/ReactRenderDocument-test.js | 273 ------------------ .../ReactRenderInvalidContainer-test.js | 56 ++++ src/core/getReactRootElementInContainer.js | 8 +- src/dom/Danger.js | 8 +- .../mutateHTMLNodeWithMarkup-test.js | 52 ---- src/dom/mutateHTMLNodeWithMarkup.js | 100 ------- 8 files changed, 60 insertions(+), 467 deletions(-) delete mode 100644 src/core/__tests__/ReactRenderDocument-test.js create mode 100644 src/core/__tests__/ReactRenderInvalidContainer-test.js delete mode 100644 src/dom/__tests__/mutateHTMLNodeWithMarkup-test.js delete mode 100644 src/dom/mutateHTMLNodeWithMarkup.js diff --git a/src/core/ReactComponentBrowserEnvironment.js b/src/core/ReactComponentBrowserEnvironment.js index 719f91a62d4c3..de9f8974eb914 100644 --- a/src/core/ReactComponentBrowserEnvironment.js +++ b/src/core/ReactComponentBrowserEnvironment.js @@ -27,11 +27,9 @@ var ReactReconcileTransaction = require('ReactReconcileTransaction'); var getReactRootElementInContainer = require('getReactRootElementInContainer'); var invariant = require('invariant'); -var mutateHTMLNodeWithMarkup = require('mutateHTMLNodeWithMarkup'); var ELEMENT_NODE_TYPE = 1; -var DOC_NODE_TYPE = 9; /** @@ -82,10 +80,7 @@ var ReactComponentBrowserEnvironment = { */ mountImageIntoNode: function(markup, container, shouldReuseMarkup) { invariant( - container && ( - container.nodeType === ELEMENT_NODE_TYPE || - container.nodeType === DOC_NODE_TYPE && ReactMount.allowFullPageRender - ), + container && container.nodeType === ELEMENT_NODE_TYPE, 'mountComponentIntoNode(...): Target container is not valid.' ); if (shouldReuseMarkup) { @@ -108,14 +103,6 @@ var ReactComponentBrowserEnvironment = { } } - // You can't naively set the innerHTML of the entire document. You need - // to mutate documentElement which requires doing some crazy tricks. See - // mutateHTMLNodeWithMarkup() - if (container.nodeType === DOC_NODE_TYPE) { - mutateHTMLNodeWithMarkup(container.documentElement, markup); - return; - } - // Asynchronously inject markup by ensuring that the container is not in // the document when settings its `innerHTML`. var parent = container.parentNode; diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 5d636a0a95e1c..7f0dc9f79661c 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -33,7 +33,6 @@ var ATTR_NAME = 'data-reactid'; var nodeCache = {}; var ELEMENT_NODE_TYPE = 1; -var DOC_NODE_TYPE = 9; /** Mapping from reactRootID to React component instance. */ var instancesByReactRootID = {}; @@ -175,11 +174,6 @@ function purgeID(id) { * Inside of `container`, the first element rendered is the "reactRoot". */ var ReactMount = { - /** - * Safety guard to prevent accidentally rendering over the entire HTML tree. - */ - allowFullPageRender: false, - /** Time spent generating markup. */ totalInstantiationTime: 0, @@ -213,10 +207,7 @@ var ReactMount = { */ prepareEnvironmentForDOM: function(container) { invariant( - container && ( - container.nodeType === ELEMENT_NODE_TYPE || - container.nodeType === DOC_NODE_TYPE - ), + container && container.nodeType === ELEMENT_NODE_TYPE 'prepareEnvironmentForDOM(...): Target container is not a DOM element.' ); var doc = container.nodeType === ELEMENT_NODE_TYPE ? @@ -431,10 +422,6 @@ var ReactMount = { unmountComponentFromNode: function(instance, container) { instance.unmountComponent(); - if (container.nodeType === DOC_NODE_TYPE) { - container = container.documentElement; - } - // http://jsperf.com/emptying-a-node while (container.lastChild) { container.removeChild(container.lastChild); diff --git a/src/core/__tests__/ReactRenderDocument-test.js b/src/core/__tests__/ReactRenderDocument-test.js deleted file mode 100644 index dd5250811ca0c..0000000000000 --- a/src/core/__tests__/ReactRenderDocument-test.js +++ /dev/null @@ -1,273 +0,0 @@ -/** - * Copyright 2013 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * @jsx React.DOM - * @emails react-core - */ - -/*jslint evil: true */ - -"use strict"; - -var React; -var ReactMount; - -var getTestDocument; - -var testDocument; - -describe('rendering React components at document', function() { - beforeEach(function() { - require('mock-modules').dumpCache(); - - React = require('React'); - ReactMount = require('ReactMount'); - getTestDocument = require('getTestDocument'); - - testDocument = getTestDocument(); - }); - - it('should be able to get root component id for document node', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var Root = React.createClass({ - render: function() { - return ( - - - Hello World - - - Hello world - - - ); - } - }); - - ReactMount.allowFullPageRender = true; - var component = React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe(' Hello world '); - - var componentID = ReactMount.getReactRootID(testDocument); - expect(componentID).toBe(component._rootNodeID); - }); - - it('should be able to unmount component from document node', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var Root = React.createClass({ - render: function() { - return ( - - - Hello World - - - Hello world - - - ); - } - }); - - ReactMount.allowFullPageRender = true; - React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe(' Hello world '); - - var unmounted = React.unmountComponentAtNode(testDocument); - expect(unmounted).toBe(true); - expect(testDocument.documentElement).not.toBe(null); - expect(testDocument.documentElement.innerHTML).toBe(''); - }); - - it('should be able to switch root constructors via state', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var Component = React.createClass({ - render: function() { - return ( - - - Hello World - - - Hello world - - - ); - } - }); - - var Component2 = React.createClass({ - render: function() { - return ( - - - Hello World - - - Goodbye world - - - ); - } - }); - - var Root = React.createClass({ - getInitialState: function() { - return {toggled: false}; - }, - toggle: function() { - this.setState({toggled: !this.state.toggled}); - }, - render: function() { - if (this.state.toggled) { - return ; - } - return ; - } - }); - - ReactMount.allowFullPageRender = true; - var component = React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe(' Hello world '); - - // Reactive update via state transition - component.toggle(); - - expect(testDocument.body.innerHTML).toBe(' Goodbye world '); - - }); - - it('should be able to switch root constructors', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var Component = React.createClass({ - render: function() { - return ( - - - Hello World - - - Hello world - - - ); - } - }); - - var Component2 = React.createClass({ - render: function() { - return ( - - - Hello World - - - Goodbye world - - - ); - } - }); - - ReactMount.allowFullPageRender = true; - React.renderComponent(, testDocument); - - expect(testDocument.body.innerHTML).toBe(' Hello world '); - - // Reactive update - React.renderComponent(, testDocument); - - expect(testDocument.body.innerHTML).toBe(' Goodbye world '); - - }); - - it('should be able to mount into document', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var Component = React.createClass({ - render: function() { - return ( - - - Hello World - - - {this.props.text} - - - ); - } - }); - ReactMount.allowFullPageRender = true; - React.renderComponent(, testDocument); - - expect(testDocument.body.innerHTML).toBe('Hello world'); - }); - - it('should throw on full document render', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var container = testDocument; - expect(function() { - React.renderComponent(, container); - }).toThrow( - 'Invariant Violation: mountComponentIntoNode(...): Target container is ' + - 'not valid.' - ); - ReactMount.allowFullPageRender = true; - expect(function() { - React.renderComponent(, container); - }).not.toThrow(); - }); - - it('should throw on full document render of non-html', function() { - if (!testDocument) { - // These tests are not applicable in jst, since jsdom is buggy. - return; - } - - var container = testDocument; - ReactMount.allowFullPageRender = true; - expect(function() { - React.renderComponent(
, container); - }).toThrow( - 'Invariant Violation: mutateHTMLNodeWithMarkup(): ' + - 'markup must start with , container); + }).toThrow( + 'Invariant Violation: prepareEnvironmentForDOM(...): Target container is not a DOM element.' + ); + }); + +}); diff --git a/src/core/getReactRootElementInContainer.js b/src/core/getReactRootElementInContainer.js index f90f5178982f1..98afb12cd29d4 100644 --- a/src/core/getReactRootElementInContainer.js +++ b/src/core/getReactRootElementInContainer.js @@ -18,8 +18,6 @@ "use strict"; -var DOC_NODE_TYPE = 9; - /** * @param {DOMElement|DOMDocument} container DOM element that may contain * a React component @@ -30,11 +28,7 @@ function getReactRootElementInContainer(container) { return null; } - if (container.nodeType === DOC_NODE_TYPE) { - return container.documentElement; - } else { - return container.firstChild; - } + return container.firstChild; } module.exports = getReactRootElementInContainer; diff --git a/src/dom/Danger.js b/src/dom/Danger.js index 1310673ee366e..c36973a9808f7 100644 --- a/src/dom/Danger.js +++ b/src/dom/Danger.js @@ -27,7 +27,6 @@ var createNodesFromMarkup = require('createNodesFromMarkup'); var emptyFunction = require('emptyFunction'); var getMarkupWrap = require('getMarkupWrap'); var invariant = require('invariant'); -var mutateHTMLNodeWithMarkup = require('mutateHTMLNodeWithMarkup'); var OPEN_TAG_NAME_EXP = /^(<[^ \/>]+)/; var RESULT_INDEX_ATTR = 'data-danger-index'; @@ -171,12 +170,7 @@ var Danger = { 'immediately.' ); invariant(markup, 'dangerouslyReplaceNodeWithMarkup(...): Missing markup.'); - // createNodesFromMarkup() won't work if the markup is rooted by - // since it has special semantic meaning. So we use an alternatie strategy. - if (oldChild.tagName.toLowerCase() === 'html') { - mutateHTMLNodeWithMarkup(oldChild, markup); - return; - } + var newChild = createNodesFromMarkup(markup, emptyFunction)[0]; oldChild.parentNode.replaceChild(newChild, oldChild); } diff --git a/src/dom/__tests__/mutateHTMLNodeWithMarkup-test.js b/src/dom/__tests__/mutateHTMLNodeWithMarkup-test.js deleted file mode 100644 index 73c3ae120ae27..0000000000000 --- a/src/dom/__tests__/mutateHTMLNodeWithMarkup-test.js +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Copyright 2013 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * @jsx React.DOM - * @emails react-core - */ - -/*jslint evil: true */ - -"use strict"; - -var getTestDocument = require('getTestDocument'); - -var mutateHTMLNodeWithMarkup = require('mutateHTMLNodeWithMarkup'); - -describe('mutateHTMLNodeWithMarkup', function() { - it('should mutate the document html', function() { - var html = 'testtest'; - var testDocument = getTestDocument() || document; - - mutateHTMLNodeWithMarkup(testDocument.documentElement, html); - expect(testDocument.body.innerHTML).toBe('test'); - }); - - it('should change attributes', function() { - var html = 'testtest'; - var testDocument = getTestDocument() || document; - - mutateHTMLNodeWithMarkup(testDocument.documentElement, html); - expect(!!testDocument.documentElement.getAttribute('lang')).toBe(false); - - var html2 = 'test' + - 'test'; - mutateHTMLNodeWithMarkup(testDocument.documentElement, html2); - expect(testDocument.documentElement.getAttribute('lang')).toBe('en'); - - mutateHTMLNodeWithMarkup(testDocument.documentElement, html); - expect(!!testDocument.documentElement.getAttribute('lang')).toBe(false); - }); -}); diff --git a/src/dom/mutateHTMLNodeWithMarkup.js b/src/dom/mutateHTMLNodeWithMarkup.js deleted file mode 100644 index a7843928849f3..0000000000000 --- a/src/dom/mutateHTMLNodeWithMarkup.js +++ /dev/null @@ -1,100 +0,0 @@ -/** - * Copyright 2013 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * @providesModule mutateHTMLNodeWithMarkup - * @typechecks static-only - */ - -/*jslint evil: true */ - -'use strict'; - -var createNodesFromMarkup = require('createNodesFromMarkup'); -var filterAttributes = require('filterAttributes'); -var invariant = require('invariant'); - -/** - * You can't set the innerHTML of a document. Unless you have - * this function. - * - * @param {DOMElement} node with tagName == 'html' - * @param {string} markup markup string including . - */ -function mutateHTMLNodeWithMarkup(node, markup) { - invariant( - node.tagName.toLowerCase() === 'html', - 'mutateHTMLNodeWithMarkup(): node must have tagName of "html", got %s', - node.tagName - ); - - markup = markup.trim(); - invariant( - markup.toLowerCase().indexOf('') + 1; - var htmlCloseTagStart = markup.lastIndexOf('<'); - var htmlOpenTag = markup.substring(0, htmlOpenTagEnd); - var innerHTML = markup.substring(htmlOpenTagEnd, htmlCloseTagStart); - - // Now for the fun stuff. Pass through both sets of attributes and - // bring them up-to-date. We get the new set by creating a markup - // fragment. - var shouldExtractAttributes = htmlOpenTag.indexOf(' ') > -1; - var attributeHolder = null; - - if (shouldExtractAttributes) { - // We extract the attributes by creating a and evaluating - // the node. - attributeHolder = createNodesFromMarkup( - htmlOpenTag.replace('html ', 'span ') + '' - )[0]; - - // Add all attributes present in attributeHolder - var attributesToSet = filterAttributes( - attributeHolder, - function(attr) { - return node.getAttributeNS(attr.namespaceURI, attr.name) !== attr.value; - } - ); - attributesToSet.forEach(function(attr) { - node.setAttributeNS(attr.namespaceURI, attr.name, attr.value); - }); - } - - // Remove all attributes not present in attributeHolder - var attributesToRemove = filterAttributes( - node, - function(attr) { - // Remove all attributes if attributeHolder is null or if it does not have - // the desired attribute. - return !( - attributeHolder && - attributeHolder.hasAttributeNS(attr.namespaceURI, attr.name) - ); - } - ); - attributesToRemove.forEach(function(attr) { - node.removeAttributeNS(attr.namespaceURI, attr.name); - }); - - // Finally, set the inner HTML. No tricks needed. Do this last to - // minimize likelihood of triggering reflows. - node.innerHTML = innerHTML; -} - -module.exports = mutateHTMLNodeWithMarkup; From 4de37058505707b155ef15b310318f37834da7cc Mon Sep 17 00:00:00 2001 From: Andrey Popp <8mayday@gmail.com> Date: Thu, 21 Nov 2013 13:00:34 +0400 Subject: [PATCH 2/2] Restrict rendering into document.head --- src/core/ReactMount.js | 4 +++- .../__tests__/ReactRenderInvalidContainer-test.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 7f0dc9f79661c..f9a61191b0449 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -207,7 +207,9 @@ var ReactMount = { */ prepareEnvironmentForDOM: function(container) { invariant( - container && container.nodeType === ELEMENT_NODE_TYPE + container && + container.nodeType === ELEMENT_NODE_TYPE && + container.tagName !== 'HEAD', 'prepareEnvironmentForDOM(...): Target container is not a DOM element.' ); var doc = container.nodeType === ELEMENT_NODE_TYPE ? diff --git a/src/core/__tests__/ReactRenderInvalidContainer-test.js b/src/core/__tests__/ReactRenderInvalidContainer-test.js index c0b24c0ff2f5b..a7f052cd0dd9e 100644 --- a/src/core/__tests__/ReactRenderInvalidContainer-test.js +++ b/src/core/__tests__/ReactRenderInvalidContainer-test.js @@ -53,4 +53,18 @@ describe('rendering React components into invalid containers', function() { ); }); + it('should throw on render into head element', function() { + if (!testDocument) { + // These tests are not applicable in jst, since jsdom is buggy. + return; + } + + var container = testDocument.head; + expect(function() { + React.renderComponent(