Add check for string and null 'rootElement' in ReactDOMFiber (#9869)

* Add check for string and null 'rootElement' in ReactDOMFiber

**what is the change?:**
Before we call 'rootElement.getAttribute' we check that the method is
defined.

**why make this change?:**
There is an internal use case I found where 'rootElement' is a string
and null at different points as the page is rendered.

It looks like this method was added as part of support for re-hydration
of server-side rendered content. I can't imagine we would want to reuse
content if the rootnode is a string or null. Not sure if we want an
earlier check that it's an element before this point.

**test plan:**
`yarn test`
and I manually tested this fix in the internal case where it was
breaking

* Add test and improve check for non-element rootElement

**what is the change?:**
We use the nodeType to check that we have the correct type of
rootElement, and we added a unit test.

**why make this change?:**
Improve this solution to the problem.

**test plan:**
`yarn test`

* run ./scripts/fiber/record-tests
This commit is contained in:
Flarnie Marchan
2017-06-07 07:51:05 -07:00
committed by GitHub
parent e9a53a7284
commit d27cbaebbb
3 changed files with 11 additions and 1 deletions
+1
View File
@@ -1314,6 +1314,7 @@ src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
* should not be able to unmount component from document node
* should not be able to switch root constructors
* should be able to mount into document
* renders over an existing text child without throwing
* should give helpful errors on state desync
* should throw on full document render w/ no markup
* supports findDOMNode on full-page components
+3 -1
View File
@@ -116,7 +116,9 @@ function getReactRootElementInContainer(container: any) {
function shouldReuseContent(container) {
const rootElement = getReactRootElementInContainer(container);
return !!(rootElement && rootElement.getAttribute(ID_ATTRIBUTE_NAME));
return !!(rootElement &&
rootElement.nodeType === ELEMENT_NODE &&
rootElement.getAttribute(ID_ATTRIBUTE_NAME));
}
function shouldAutoFocusHostComponent(type: string, props: Props): boolean {
@@ -178,6 +178,13 @@ describe('rendering React components at document', () => {
expect(testDocument.body.innerHTML).toBe('Hello world');
});
it('renders over an existing text child without throwing', () => {
const container = document.createElement('div');
container.textContent = 'potato';
ReactDOM.render(<div>parsnip</div>, container);
expect(container.textContent).toBe('parsnip');
});
it('should give helpful errors on state desync', () => {
class Component extends React.Component {
render() {