From 845d3496f8c8d4c478d362fe9cc6064602466b23 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 13:48:04 -0700
Subject: [PATCH 01/20] RFC Add warning for rendering into container that was
 updated manually

RFC because we still need to tidy this up and verify that all tests
pass.

**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.

Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.

**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854

last item under the '16 beta' checklist.
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 15 ++++++
 .../dom/fiber/__tests__/ReactDOMFiber-test.js | 50 +++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 571a3bfe0c1b0..8e09175ccb225 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -531,6 +531,21 @@ function renderSubtreeIntoContainer(
   );
 
   if (__DEV__) {
+    if (container._reactRootContainer) {
+      const firstChild = container.firstChild;
+      const hostInstance =
+        DOMRenderer.findHostInstance(container._reactRootContainer.current);
+      if (hostInstance) {
+        warning(
+          hostInstance === firstChild,
+          'render(...): It looks like the content of this container may have ' +
+          'been updated or cleared outside of React. ' +
+          'This can cause errors or failed updates to the container. ' +
+          'Please call `ReactDOM.render` with your new content.',
+        );
+      }
+    }
+
     const isRootRenderedBySomeReact = !!container._reactRootContainer;
     const rootEl = getReactRootElementInContainer(container);
     const hasNonRootReactChild = !!(rootEl &&
diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index 219f2e93f9f2d..8f423267aeed2 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -1004,6 +1004,56 @@ describe('ReactDOMFiber', () => {
         container,
       );
     });
+
+    it('should not warn when rendering into an empty container', () => {
+      spyOn(console, 'error');
+      ReactDOM.render(<div>'foo'</div>, container);
+      expect(container.innerHTML).toBe("<div>'foo'</div>");
+      ReactDOM.render(null, container);
+      expect(container.innerHTML).toBe('');
+      expectDev(console.error.calls.count()).toBe(0);
+      ReactDOM.render(<div>'bar'</div>, container);
+      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      expectDev(console.error.calls.count()).toBe(0);
+    });
+
+    it('should warn when doing an update to a container manually updated outside of React', () => {
+       spyOn(console, 'error');
+      // when not messing with the DOM outside of React
+      ReactDOM.render(<div>'foo'</div>, container);
+      ReactDOM.render(<div>'bar'</div>, container);
+      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      // then we mess with the DOM before an update
+      container.innerHTML = '<div>MEOW.</div>';
+      ReactDOM.render(<div>'baz'</div>, container);
+      // fails
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
+        'It looks like the content of this container may have been ' +
+        'updated or cleared outside of React. This can cause errors or ' +
+        'failed updates to the container. Please call `ReactDOM.render` ' +
+        'with your new content.',
+      );
+    });
+
+    it('should warn when doing an update to a container manually cleared outside of React', () => {
+      spyOn(console, 'error');
+      // when not messing with the DOM outside of React
+      ReactDOM.render(<div>'foo'</div>, container);
+      ReactDOM.render(<div>'bar'</div>, container);
+      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      // then we mess with the DOM before an update
+      container.innerHTML = '';
+      ReactDOM.render(<div>'baz'</div>, container);
+      // fails to update
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
+        'It looks like the content of this container may have been ' +
+        'updated or cleared outside of React. This can cause errors or ' +
+        'failed updates to the container. Please call `ReactDOM.render` ' +
+        'with your new content.',
+      );
+    });
   }
 });
 

From 72dfbafe10b9f38369e093afac4f7f6fccbb8c04 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 16:16:56 -0700
Subject: [PATCH 02/20] Add test and tweak check for rendering into manually
 updated container

STILL TODO: figure out how to skip this warning when the component
renders to a portal.

Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.

**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
  container, using a key or a different type, after the contents of the
  first container were messed with outside of React. This case throws,
  and now at least there will be an informative warning along with the
  error.
- We updated the check to compare the parent of the 'hostInstance' to
  the container; this seems less fragile
- tweaked some comments

**why make this change?:**
Continue improving this to make it more final.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js |  6 +++--
 .../dom/fiber/__tests__/ReactDOMFiber-test.js | 25 +++++++++++++++++--
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 8e09175ccb225..1e08302143f5e 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -531,13 +531,15 @@ function renderSubtreeIntoContainer(
   );
 
   if (__DEV__) {
-    if (container._reactRootContainer) {
+    let warned;
+    if (!warned && container._reactRootContainer) {
       const firstChild = container.firstChild;
       const hostInstance =
         DOMRenderer.findHostInstance(container._reactRootContainer.current);
       if (hostInstance) {
+        warned = true;
         warning(
-          hostInstance === firstChild,
+          hostInstance.parentNode === container,
           'render(...): It looks like the content of this container may have ' +
           'been updated or cleared outside of React. ' +
           'This can cause errors or failed updates to the container. ' +
diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index 8f423267aeed2..2d37a2e114969 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -1017,6 +1017,27 @@ describe('ReactDOMFiber', () => {
       expectDev(console.error.calls.count()).toBe(0);
     });
 
+    it('should warn when replacing a container which was manually updated outside of React', () => {
+       spyOn(console, 'error');
+      // when not messing with the DOM outside of React
+      ReactDOM.render(<div key='1'>'foo'</div>, container);
+      ReactDOM.render(<div key='1'>'bar'</div>, container);
+      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      // then we mess with the DOM before an update
+      // we know this will error - that is expected right now
+      expect(() => {
+        container.innerHTML = '<div>MEOW.</div>';
+        ReactDOM.render(<div key='2'>'baz'</div>, container);
+      }).toThrow();
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
+        'It looks like the content of this container may have been ' +
+        'updated or cleared outside of React. This can cause errors or ' +
+        'failed updates to the container. Please call `ReactDOM.render` ' +
+        'with your new content.',
+      );
+    });
+
     it('should warn when doing an update to a container manually updated outside of React', () => {
        spyOn(console, 'error');
       // when not messing with the DOM outside of React
@@ -1026,7 +1047,7 @@ describe('ReactDOMFiber', () => {
       // then we mess with the DOM before an update
       container.innerHTML = '<div>MEOW.</div>';
       ReactDOM.render(<div>'baz'</div>, container);
-      // fails
+      // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
         'It looks like the content of this container may have been ' +
@@ -1045,7 +1066,7 @@ describe('ReactDOMFiber', () => {
       // then we mess with the DOM before an update
       container.innerHTML = '';
       ReactDOM.render(<div>'baz'</div>, container);
-      // fails to update
+      // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
         'It looks like the content of this container may have been ' +

From a439ba677be1a780aa76655aba07d28866232812 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 16:41:58 -0700
Subject: [PATCH 03/20] Stub our `console.error` in one of the portal tests

**what is the change?:**
See title

**why make this change?:**
See comment in the code

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index 2d37a2e114969..efd6ee988438a 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -758,6 +758,13 @@ describe('ReactDOMFiber', () => {
     });
 
     it('should update portal context if it changes due to re-render', () => {
+      // We mock out console error because this causes a warning to fire;
+      // it looks as if the content should be in the 'container' div,
+      // and when it's not there after the first render React assumes the
+      // container was cleared outside of React.
+      // for now we are not concerned about this warning firing.
+      spyOn(console, 'error');
+
       var portalContainer = document.createElement('div');
 
       class Component extends React.Component {

From 5a7da89439d5e31819ddee1d18d0b242f53ad257 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 17:08:02 -0700
Subject: [PATCH 04/20] Skip warning in 'ReactDOMFiberEntry' when mounting to
 Comment node

**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.

We are skipping that warning when you render into a 'comment' node.

**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js         | 4 +++-
 src/renderers/dom/shared/__tests__/ReactMount-test.js | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 1e08302143f5e..b96443bfc8acb 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -532,7 +532,9 @@ function renderSubtreeIntoContainer(
 
   if (__DEV__) {
     let warned;
-    if (!warned && container._reactRootContainer) {
+    if (!warned
+        && container._reactRootContainer
+      && container.nodeType !== COMMENT_NODE) {
       const firstChild = container.firstChild;
       const hostInstance =
         DOMRenderer.findHostInstance(container._reactRootContainer.current);
diff --git a/src/renderers/dom/shared/__tests__/ReactMount-test.js b/src/renderers/dom/shared/__tests__/ReactMount-test.js
index 05c798d6bdf38..5b4f2695563f4 100644
--- a/src/renderers/dom/shared/__tests__/ReactMount-test.js
+++ b/src/renderers/dom/shared/__tests__/ReactMount-test.js
@@ -12,6 +12,7 @@
 'use strict';
 
 const ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
+const {COMMENT_NODE} = require('HTMLNodeType');
 
 const invariant = require('invariant');
 
@@ -371,7 +372,7 @@ describe('ReactMount', () => {
         containerDiv = document.createElement('div');
         containerDiv.innerHTML = 'A<!-- react-mount-point-unstable -->B';
         mountPoint = containerDiv.childNodes[1];
-        invariant(mountPoint.nodeType === 8, 'Expected comment');
+        invariant(mountPoint.nodeType === COMMENT_NODE, 'Expected comment');
       });
 
       it('renders at a comment node', () => {

From cfd3c35675c3c4e33fa9c60821ed9939150580ad Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 17:38:35 -0700
Subject: [PATCH 05/20] Improve warning message, remove dedup check, and unmock
 console.error

**what is the change?:**
Various changes to get this closer to being finished;
- Improved warning message (thanks @spicyj!!!)
- Removed dedup check on warning
- Remove mocking of 'console.error' in portals test

**why make this change?:**
- warning message improvement: communicates better with users
- Remove dedup check: it wasn't important in this case
- Remove mocking of 'console.error'; we don't want to ignore an
  inaccurate warning, even for an "unstable" feature.

**test plan:**
`yarn test` -> follow-up commits will fix the remaining tests

**issue:**
issue #8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js       | 13 +++++--------
 .../dom/fiber/__tests__/ReactDOMFiber-test.js       |  7 -------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index b96443bfc8acb..ff1f00cf1c763 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -531,21 +531,18 @@ function renderSubtreeIntoContainer(
   );
 
   if (__DEV__) {
-    let warned;
-    if (!warned
-        && container._reactRootContainer
+    if (container._reactRootContainer
       && container.nodeType !== COMMENT_NODE) {
       const firstChild = container.firstChild;
       const hostInstance =
         DOMRenderer.findHostInstance(container._reactRootContainer.current);
       if (hostInstance) {
-        warned = true;
         warning(
           hostInstance.parentNode === container,
-          'render(...): It looks like the content of this container may have ' +
-          'been updated or cleared outside of React. ' +
-          'This can cause errors or failed updates to the container. ' +
-          'Please call `ReactDOM.render` with your new content.',
+          'render(...): It looks like the React-rendered content of this ' +
+          'container was removed without using React. This is not ' +
+          'supported and will cause errors. Instead, call ' +
+          'ReactDOM.unmountComponentAtNode to empty a container.',
         );
       }
     }
diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index efd6ee988438a..2d37a2e114969 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -758,13 +758,6 @@ describe('ReactDOMFiber', () => {
     });
 
     it('should update portal context if it changes due to re-render', () => {
-      // We mock out console error because this causes a warning to fire;
-      // it looks as if the content should be in the 'container' div,
-      // and when it's not there after the first render React assumes the
-      // container was cleared outside of React.
-      // for now we are not concerned about this warning firing.
-      spyOn(console, 'error');
-
       var portalContainer = document.createElement('div');
 
       class Component extends React.Component {

From fff93e3e01e681abe5fed99d45e71f41645ee709 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Tue, 18 Jul 2017 18:27:05 -0700
Subject: [PATCH 06/20] Possible fix for issue of incorrect warning for portal
 re-render

**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.

**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.

concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
  sure.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js        | 2 +-
 src/renderers/shared/fiber/isomorphic/ReactPortal.js | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index ff1f00cf1c763..4e8f4b3d5b831 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -536,7 +536,7 @@ function renderSubtreeIntoContainer(
       const firstChild = container.firstChild;
       const hostInstance =
         DOMRenderer.findHostInstance(container._reactRootContainer.current);
-      if (hostInstance) {
+      if (hostInstance && !hostInstance.parentNode._reactPortalContainer) {
         warning(
           hostInstance.parentNode === container,
           'render(...): It looks like the React-rendered content of this ' +
diff --git a/src/renderers/shared/fiber/isomorphic/ReactPortal.js b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
index 78198e099d697..b6de86221f00b 100644
--- a/src/renderers/shared/fiber/isomorphic/ReactPortal.js
+++ b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
@@ -27,6 +27,7 @@ exports.createPortal = function(
   implementation: any,
   key: ?string = null,
 ): ReactPortal {
+  containerInfo._reactPortalContainer = {};
   return {
     // This tag allow us to uniquely identify this as a React Portal
     $$typeof: REACT_PORTAL_TYPE,

From 178ac2bfff9cc90c090825d2ed5fc0726c38d4f2 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 08:09:30 -0700
Subject: [PATCH 07/20] Fix logic for checking if the host instance container
 is a portal

**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.

**why make this change?:**
To fix a bug that the previous commit introduced.

**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 4e8f4b3d5b831..8dd36195ea5b2 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -536,7 +536,11 @@ function renderSubtreeIntoContainer(
       const firstChild = container.firstChild;
       const hostInstance =
         DOMRenderer.findHostInstance(container._reactRootContainer.current);
-      if (hostInstance && !hostInstance.parentNode._reactPortalContainer) {
+      const hostInstanceParentIsPortal =
+        hostInstance &&
+        hostInstance.parentNode &&
+        hostInstance.parentNode._reactPortalContainer;
+      if (hostInstance && !hostInstanceParentIsPortal) {
         warning(
           hostInstance.parentNode === container,
           'render(...): It looks like the React-rendered content of this ' +

From c9a6e6c7ac68e12e3920eae8673370c2c8f0c566 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 08:44:04 -0700
Subject: [PATCH 08/20] Clean up new tests in ReactDOMFiber-test

**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation

**why make this change?:**
- get tests passing
- code maintainability/readability

**test plan:**
`yarn test`
follow up commits will fix the remaining tests

**issue:**
https://github.com/facebook/react/issues/8854
---
 .../dom/fiber/__tests__/ReactDOMFiber-test.js | 60 +++++++++----------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index 2d37a2e114969..d2f82ef847a22 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -1007,72 +1007,72 @@ describe('ReactDOMFiber', () => {
 
     it('should not warn when rendering into an empty container', () => {
       spyOn(console, 'error');
-      ReactDOM.render(<div>'foo'</div>, container);
-      expect(container.innerHTML).toBe("<div>'foo'</div>");
+      ReactDOM.render(<div>foo</div>, container);
+      expect(container.innerHTML).toBe('<div>foo</div>');
       ReactDOM.render(null, container);
       expect(container.innerHTML).toBe('');
       expectDev(console.error.calls.count()).toBe(0);
-      ReactDOM.render(<div>'bar'</div>, container);
-      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      ReactDOM.render(<div>bar</div>, container);
+      expect(container.innerHTML).toBe('<div>bar</div>');
       expectDev(console.error.calls.count()).toBe(0);
     });
 
     it('should warn when replacing a container which was manually updated outside of React', () => {
        spyOn(console, 'error');
       // when not messing with the DOM outside of React
-      ReactDOM.render(<div key='1'>'foo'</div>, container);
-      ReactDOM.render(<div key='1'>'bar'</div>, container);
-      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      ReactDOM.render(<div key='1'>foo</div>, container);
+      ReactDOM.render(<div key='1'>bar</div>, container);
+      expect(container.innerHTML).toBe('<div>bar</div>');
       // then we mess with the DOM before an update
       // we know this will error - that is expected right now
       expect(() => {
         container.innerHTML = '<div>MEOW.</div>';
-        ReactDOM.render(<div key='2'>'baz'</div>, container);
-      }).toThrow();
+        ReactDOM.render(<div key='2'>baz</div>, container);
+      }).toThrowErrorMatchingSnapshot();
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the content of this container may have been ' +
-        'updated or cleared outside of React. This can cause errors or ' +
-        'failed updates to the container. Please call `ReactDOM.render` ' +
-        'with your new content.',
+        'It looks like the React-rendered content of this container was ' +
+        'removed without using React. This is not supported and will ' +
+        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+        'to empty a container.',
       );
     });
 
     it('should warn when doing an update to a container manually updated outside of React', () => {
-       spyOn(console, 'error');
+      spyOn(console, 'error');
       // when not messing with the DOM outside of React
-      ReactDOM.render(<div>'foo'</div>, container);
-      ReactDOM.render(<div>'bar'</div>, container);
-      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      ReactDOM.render(<div>foo</div>, container);
+      ReactDOM.render(<div>bar</div>, container);
+      expect(container.innerHTML).toBe('<div>bar</div>');
       // then we mess with the DOM before an update
       container.innerHTML = '<div>MEOW.</div>';
-      ReactDOM.render(<div>'baz'</div>, container);
+      ReactDOM.render(<div>baz</div>, container);
       // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the content of this container may have been ' +
-        'updated or cleared outside of React. This can cause errors or ' +
-        'failed updates to the container. Please call `ReactDOM.render` ' +
-        'with your new content.',
+        'It looks like the React-rendered content of this container was ' +
+        'removed without using React. This is not supported and will ' +
+        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+        'to empty a container.',
       );
     });
 
     it('should warn when doing an update to a container manually cleared outside of React', () => {
       spyOn(console, 'error');
       // when not messing with the DOM outside of React
-      ReactDOM.render(<div>'foo'</div>, container);
-      ReactDOM.render(<div>'bar'</div>, container);
-      expect(container.innerHTML).toBe("<div>'bar'</div>");
+      ReactDOM.render(<div>foo</div>, container);
+      ReactDOM.render(<div>bar</div>, container);
+      expect(container.innerHTML).toBe('<div>bar</div>');
       // then we mess with the DOM before an update
       container.innerHTML = '';
-      ReactDOM.render(<div>'baz'</div>, container);
+      ReactDOM.render(<div>baz</div>, container);
       // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the content of this container may have been ' +
-        'updated or cleared outside of React. This can cause errors or ' +
-        'failed updates to the container. Please call `ReactDOM.render` ' +
-        'with your new content.',
+        'It looks like the React-rendered content of this container was ' +
+        'removed without using React. This is not supported and will ' +
+        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+        'to empty a container.',
       );
     });
   }

From 194adf1bc6167da1ffa1c7de50c044e20d58b334 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 09:06:29 -0700
Subject: [PATCH 09/20] Add 'unmountComponentAtNode' call in test for
 reconciling pre-rendered markup

**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.

This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.

**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.

**test plan:**
`yarn test`

Only one more failing test, will fix that in the next commit.

**issue:**
https://github.com/facebook/react/issues/8854
---
 .../dom/shared/__tests__/ReactDOMTextComponent-test.js          | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
index 167bca42b6f41..013eabaf04a1b 100644
--- a/src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
+++ b/src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
@@ -120,6 +120,8 @@ describe('ReactDOMTextComponent', () => {
     ReactDOM.render(reactEl, el);
     expect(el.textContent).toBe('foobarbaz');
 
+    ReactDOM.unmountComponentAtNode(el);
+
     reactEl = <div>{''}{''}{''}</div>;
     el.innerHTML = ReactDOMServer.renderToString(reactEl);
 

From a2fdfd2d00a1485b727f686204b08067c6a7d617 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 09:34:01 -0700
Subject: [PATCH 10/20] ran prettier

---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 14 +++----
 .../dom/fiber/__tests__/ReactDOMFiber-test.js | 41 ++++++++++---------
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 8dd36195ea5b2..693f12d7c59be 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -531,11 +531,11 @@ function renderSubtreeIntoContainer(
   );
 
   if (__DEV__) {
-    if (container._reactRootContainer
-      && container.nodeType !== COMMENT_NODE) {
+    if (container._reactRootContainer && container.nodeType !== COMMENT_NODE) {
       const firstChild = container.firstChild;
-      const hostInstance =
-        DOMRenderer.findHostInstance(container._reactRootContainer.current);
+      const hostInstance = DOMRenderer.findHostInstance(
+        container._reactRootContainer.current,
+      );
       const hostInstanceParentIsPortal =
         hostInstance &&
         hostInstance.parentNode &&
@@ -544,9 +544,9 @@ function renderSubtreeIntoContainer(
         warning(
           hostInstance.parentNode === container,
           'render(...): It looks like the React-rendered content of this ' +
-          'container was removed without using React. This is not ' +
-          'supported and will cause errors. Instead, call ' +
-          'ReactDOM.unmountComponentAtNode to empty a container.',
+            'container was removed without using React. This is not ' +
+            'supported and will cause errors. Instead, call ' +
+            'ReactDOM.unmountComponentAtNode to empty a container.',
         );
       }
     }
diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index d2f82ef847a22..19c45a907c7f8 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -1018,23 +1018,24 @@ describe('ReactDOMFiber', () => {
     });
 
     it('should warn when replacing a container which was manually updated outside of React', () => {
-       spyOn(console, 'error');
+      spyOn(console, 'error');
       // when not messing with the DOM outside of React
-      ReactDOM.render(<div key='1'>foo</div>, container);
-      ReactDOM.render(<div key='1'>bar</div>, container);
+      ReactDOM.render(<div key="1">foo</div>, container);
+      ReactDOM.render(<div key="1">bar</div>, container);
       expect(container.innerHTML).toBe('<div>bar</div>');
       // then we mess with the DOM before an update
       // we know this will error - that is expected right now
       expect(() => {
         container.innerHTML = '<div>MEOW.</div>';
-        ReactDOM.render(<div key='2'>baz</div>, container);
+        ReactDOM.render(<div key="2">baz</div>, container);
       }).toThrowErrorMatchingSnapshot();
       expectDev(console.error.calls.count()).toBe(1);
-      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the React-rendered content of this container was ' +
-        'removed without using React. This is not supported and will ' +
-        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
-        'to empty a container.',
+      expectDev(console.error.calls.argsFor(0)[0]).toContain(
+        'render(...): ' +
+          'It looks like the React-rendered content of this container was ' +
+          'removed without using React. This is not supported and will ' +
+          'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+          'to empty a container.',
       );
     });
 
@@ -1049,11 +1050,12 @@ describe('ReactDOMFiber', () => {
       ReactDOM.render(<div>baz</div>, container);
       // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
-      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the React-rendered content of this container was ' +
-        'removed without using React. This is not supported and will ' +
-        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
-        'to empty a container.',
+      expectDev(console.error.calls.argsFor(0)[0]).toContain(
+        'render(...): ' +
+          'It looks like the React-rendered content of this container was ' +
+          'removed without using React. This is not supported and will ' +
+          'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+          'to empty a container.',
       );
     });
 
@@ -1068,11 +1070,12 @@ describe('ReactDOMFiber', () => {
       ReactDOM.render(<div>baz</div>, container);
       // silently fails to update
       expectDev(console.error.calls.count()).toBe(1);
-      expectDev(console.error.calls.argsFor(0)[0]).toContain('render(...): ' +
-        'It looks like the React-rendered content of this container was ' +
-        'removed without using React. This is not supported and will ' +
-        'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
-        'to empty a container.',
+      expectDev(console.error.calls.argsFor(0)[0]).toContain(
+        'render(...): ' +
+          'It looks like the React-rendered content of this container was ' +
+          'removed without using React. This is not supported and will ' +
+          'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
+          'to empty a container.',
       );
     });
   }

From 48f8f8351a4d781e070c0ea174b398fbf57aee8a Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 09:35:40 -0700
Subject: [PATCH 11/20] remove unused variable

---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 693f12d7c59be..97e98ae14ae2c 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -532,7 +532,6 @@ function renderSubtreeIntoContainer(
 
   if (__DEV__) {
     if (container._reactRootContainer && container.nodeType !== COMMENT_NODE) {
-      const firstChild = container.firstChild;
       const hostInstance = DOMRenderer.findHostInstance(
         container._reactRootContainer.current,
       );

From 64ce33441377a9f51be20bbcb0dbd095d81a8b66 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 09:39:38 -0700
Subject: [PATCH 12/20] run scripts/fiber/record-tests

---
 scripts/fiber/tests-passing.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
index f599832df086c..21ef07af4d525 100644
--- a/scripts/fiber/tests-passing.txt
+++ b/scripts/fiber/tests-passing.txt
@@ -667,6 +667,10 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
 * should not onMouseLeave when staying in the portal
 * should not update event handlers until commit
 * should not crash encountering low-priority tree
+* should not warn when rendering into an empty container
+* should warn when replacing a container which was manually updated outside of React
+* should warn when doing an update to a container manually updated outside of React
+* should warn when doing an update to a container manually cleared outside of React
 * throws if non-element passed to top-level render
 * throws if something other than false, null, or an element is returned from render
 * treats mocked render functions as if they return null

From c92730947efef564afdfd0fc1bd393d2daf62461 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 13:15:40 -0700
Subject: [PATCH 13/20] Fix type error and improve name of portal container
 flag

**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.

**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
  indicates it was used as a portal container.
- renamed the flag to something which makes more sense

**why make this change?:**
- get Flow passing
- make this change make more sense

We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js        | 12 +++++++++---
 src/renderers/shared/fiber/isomorphic/ReactPortal.js |  3 ++-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 97e98ae14ae2c..ac95d15a3c33d 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -73,8 +73,14 @@ findDOMNode._injectFiber(function(fiber: Fiber) {
 });
 
 type DOMContainer =
-  | (Element & {_reactRootContainer: ?Object})
-  | (Document & {_reactRootContainer: ?Object});
+  | (Element & {
+    _reactRootContainer: ?Object,
+    __reactInternalIsPortalContainer: ?boolean,
+  })
+  | (Document & {
+    _reactRootContainer: ?Object,
+    __reactInternalIsPortalContainer: ?boolean,
+  });
 
 type Container = Element | Document;
 type Props = {
@@ -538,7 +544,7 @@ function renderSubtreeIntoContainer(
       const hostInstanceParentIsPortal =
         hostInstance &&
         hostInstance.parentNode &&
-        hostInstance.parentNode._reactPortalContainer;
+        hostInstance.parentNode.__reactInternalIsPortalContainer;
       if (hostInstance && !hostInstanceParentIsPortal) {
         warning(
           hostInstance.parentNode === container,
diff --git a/src/renderers/shared/fiber/isomorphic/ReactPortal.js b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
index b6de86221f00b..3908b64b7c9e8 100644
--- a/src/renderers/shared/fiber/isomorphic/ReactPortal.js
+++ b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
@@ -27,7 +27,8 @@ exports.createPortal = function(
   implementation: any,
   key: ?string = null,
 ): ReactPortal {
-  containerInfo._reactPortalContainer = {};
+  // This flag allows us to check if a node was used with a portal
+  containerInfo.__reactInternalIsPortalContainer = true;
   return {
     // This tag allow us to uniquely identify this as a React Portal
     $$typeof: REACT_PORTAL_TYPE,

From 7f9648b3510997dfc2e425815546a83a66a95b96 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 15:10:21 -0700
Subject: [PATCH 14/20] Add flag to portalContainer on mount instead of in
 `createPortal`

**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.

**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
  reference to the node that was rendered the first time, and expect it
  to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
  when a component renders something returned by
  `ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
  then the child is inside the `portalContainer` and not the `container,
  but that is valid and we want to skip warning in that case.

Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.

We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/shared/fiber/ReactFiberCommitWork.js   | 4 ++++
 src/renderers/shared/fiber/isomorphic/ReactPortal.js | 2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js
index 23a0230e06541..02a8f59e6de98 100644
--- a/src/renderers/shared/fiber/ReactFiberCommitWork.js
+++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js
@@ -218,6 +218,10 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
           }
         } else {
           if (isContainer) {
+            if (parentFiber.tag === HostPortal) {
+              // allows us to identify the portal container in other places
+              parent.__reactInternalIsPortalContainer = true;
+            }
             appendChildToContainer(parent, node.stateNode);
           } else {
             appendChild(parent, node.stateNode);
diff --git a/src/renderers/shared/fiber/isomorphic/ReactPortal.js b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
index 3908b64b7c9e8..78198e099d697 100644
--- a/src/renderers/shared/fiber/isomorphic/ReactPortal.js
+++ b/src/renderers/shared/fiber/isomorphic/ReactPortal.js
@@ -27,8 +27,6 @@ exports.createPortal = function(
   implementation: any,
   key: ?string = null,
 ): ReactPortal {
-  // This flag allows us to check if a node was used with a portal
-  containerInfo.__reactInternalIsPortalContainer = true;
   return {
     // This tag allow us to uniquely identify this as a React Portal
     $$typeof: REACT_PORTAL_TYPE,

From 2225b980aa14248dd647a55505e27093e4e48623 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Wed, 19 Jul 2017 16:02:02 -0700
Subject: [PATCH 15/20] Force an 'any' type for the `hostInstance.parentNode`
 in warning check

**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.

To at least make the type information clear we added a comment.

**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(

**test plan:**
`yarn flow`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index ac95d15a3c33d..e01fff18f0f8a 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -75,11 +75,9 @@ findDOMNode._injectFiber(function(fiber: Fiber) {
 type DOMContainer =
   | (Element & {
     _reactRootContainer: ?Object,
-    __reactInternalIsPortalContainer: ?boolean,
   })
   | (Document & {
     _reactRootContainer: ?Object,
-    __reactInternalIsPortalContainer: ?boolean,
   });
 
 type Container = Element | Document;
@@ -541,10 +539,11 @@ function renderSubtreeIntoContainer(
       const hostInstance = DOMRenderer.findHostInstance(
         container._reactRootContainer.current,
       );
+      const hostInstanceParentNode: any =
+        hostInstance && hostInstance.parentNode;
       const hostInstanceParentIsPortal =
-        hostInstance &&
-        hostInstance.parentNode &&
-        hostInstance.parentNode.__reactInternalIsPortalContainer;
+        hostInstanceParentNode &&
+        hostInstanceParentNode.__reactInternalIsPortalContainer;
       if (hostInstance && !hostInstanceParentIsPortal) {
         warning(
           hostInstance.parentNode === container,

From f7efe64df3b5533c38bbf76fd1c92e9e32717737 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Thu, 20 Jul 2017 09:57:52 -0700
Subject: [PATCH 16/20] Ignore portals in `DOMRenderer.findHostInstance`

**what is the change?:**
We want to ignore portals when firing a certain warning.

This allows us to get the host instance and ignore portals.

Also added a new snapshot recording while fixing things.

**why make this change?:**
Originally we had added a flag to the DOM node which was used for

rendering the portal, and then could notice and ignore children rendered
into those nodes.

However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
 - we will not ignore a non-portal second child with this approach
 - we meant to ignore portals in this method anyway (according to a
   'TODO' comment)
 - this change only affects the DOM renderer, instead of changing code
   which is shared with RN and other renderers
 - we avoid adding unneeded expandos

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js         | 11 +++--------
 .../__snapshots__/ReactDOMFiber-test.js.snap          |  3 +++
 .../shared/fiber/ReactFiberTreeReflection.js          |  8 ++++----
 3 files changed, 10 insertions(+), 12 deletions(-)
 create mode 100644 src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index e01fff18f0f8a..96c4a92cc4f90 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -74,10 +74,10 @@ findDOMNode._injectFiber(function(fiber: Fiber) {
 
 type DOMContainer =
   | (Element & {
-    _reactRootContainer: ?Object,
+    _reactRootContainer: ?Object
   })
   | (Document & {
-    _reactRootContainer: ?Object,
+    _reactRootContainer: ?Object
   });
 
 type Container = Element | Document;
@@ -539,12 +539,7 @@ function renderSubtreeIntoContainer(
       const hostInstance = DOMRenderer.findHostInstance(
         container._reactRootContainer.current,
       );
-      const hostInstanceParentNode: any =
-        hostInstance && hostInstance.parentNode;
-      const hostInstanceParentIsPortal =
-        hostInstanceParentNode &&
-        hostInstanceParentNode.__reactInternalIsPortalContainer;
-      if (hostInstance && !hostInstanceParentIsPortal) {
+      if (hostInstance) {
         warning(
           hostInstance.parentNode === container,
           'render(...): It looks like the React-rendered content of this ' +
diff --git a/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap b/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap
new file mode 100644
index 0000000000000..55f93688b35ca
--- /dev/null
+++ b/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap
@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`ReactDOMFiber should warn when replacing a container which was manually updated outside of React 1`] = `""`;
diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js
index 3e557375bb210..ce6e0a354bac0 100644
--- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js
+++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js
@@ -25,10 +25,11 @@ if (__DEV__) {
 }
 
 var {
-  HostRoot,
+  ClassComponent,
   HostComponent,
+  HostRoot,
+  HostPortal,
   HostText,
-  ClassComponent,
 } = require('ReactTypeOfWork');
 
 var {NoEffect, Placement} = require('ReactTypeOfSideEffect');
@@ -241,8 +242,7 @@ exports.findCurrentHostFiber = function(parent: Fiber): Fiber | null {
   while (true) {
     if (node.tag === HostComponent || node.tag === HostText) {
       return node;
-    } else if (node.child) {
-      // TODO: If we hit a Portal, we're supposed to skip it.
+    } else if (node.child && node.tag !== HostPortal) {
       node.child.return = node;
       node = node.child;
       continue;

From e64c2c18de11f246218de0337111eedad1f263ec Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Thu, 20 Jul 2017 10:50:27 -0700
Subject: [PATCH 17/20] Ran prettier

---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 96c4a92cc4f90..13c59b88a4d3f 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -74,10 +74,10 @@ findDOMNode._injectFiber(function(fiber: Fiber) {
 
 type DOMContainer =
   | (Element & {
-    _reactRootContainer: ?Object
+    _reactRootContainer: ?Object,
   })
   | (Document & {
-    _reactRootContainer: ?Object
+    _reactRootContainer: ?Object,
   });
 
 type Container = Element | Document;

From 0b0035c9f41e6292799511f5f36fbc7c93ed37c0 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Thu, 20 Jul 2017 11:04:05 -0700
Subject: [PATCH 18/20] Remove error snapshot test

I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.

If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
---
 src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js        | 3 ++-
 .../fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap   | 3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)
 delete mode 100644 src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap

diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
index 19c45a907c7f8..4a4fdbe808ffb 100644
--- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
+++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
@@ -1025,10 +1025,11 @@ describe('ReactDOMFiber', () => {
       expect(container.innerHTML).toBe('<div>bar</div>');
       // then we mess with the DOM before an update
       // we know this will error - that is expected right now
+      // It's an error of type 'NotFoundError' with no message
       expect(() => {
         container.innerHTML = '<div>MEOW.</div>';
         ReactDOM.render(<div key="2">baz</div>, container);
-      }).toThrowErrorMatchingSnapshot();
+      }).toThrowError();
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
         'render(...): ' +
diff --git a/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap b/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap
deleted file mode 100644
index 55f93688b35ca..0000000000000
--- a/src/renderers/dom/fiber/__tests__/__snapshots__/ReactDOMFiber-test.js.snap
+++ /dev/null
@@ -1,3 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`ReactDOMFiber should warn when replacing a container which was manually updated outside of React 1`] = `""`;

From 178e1a415270932ae0c96dfd2bae13da5c9b81d0 Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Thu, 20 Jul 2017 13:02:26 -0700
Subject: [PATCH 19/20] Remove expando that we were adding to portal container

**what is the change?:**
see title

**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/shared/fiber/ReactFiberCommitWork.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js
index 02a8f59e6de98..23a0230e06541 100644
--- a/src/renderers/shared/fiber/ReactFiberCommitWork.js
+++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js
@@ -218,10 +218,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
           }
         } else {
           if (isContainer) {
-            if (parentFiber.tag === HostPortal) {
-              // allows us to identify the portal container in other places
-              parent.__reactInternalIsPortalContainer = true;
-            }
             appendChildToContainer(parent, node.stateNode);
           } else {
             appendChild(parent, node.stateNode);

From f9edf89922db00fb2ed33e01d9cf10faa1d270fc Mon Sep 17 00:00:00 2001
From: Flarnie Marchan <flarnie@fb.com>
Date: Thu, 20 Jul 2017 14:35:03 -0700
Subject: [PATCH 20/20] Fork `findHostInstance` to make
 `findHostInstanceWithNoPortals`

**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.

**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.

**test plan:**
`yarn test`

**issue:**
https://github.com/facebook/react/issues/8854
---
 src/renderers/dom/fiber/ReactDOMFiberEntry.js |  2 +-
 .../shared/fiber/ReactFiberReconciler.js      | 16 ++++++++-
 .../shared/fiber/ReactFiberTreeReflection.js  | 35 +++++++++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
index 13c59b88a4d3f..8ce0ca92ff12e 100644
--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -536,7 +536,7 @@ function renderSubtreeIntoContainer(
 
   if (__DEV__) {
     if (container._reactRootContainer && container.nodeType !== COMMENT_NODE) {
-      const hostInstance = DOMRenderer.findHostInstance(
+      const hostInstance = DOMRenderer.findHostInstanceWithNoPortals(
         container._reactRootContainer.current,
       );
       if (hostInstance) {
diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js
index d2f38655226dd..07c642cb2b7fd 100644
--- a/src/renderers/shared/fiber/ReactFiberReconciler.js
+++ b/src/renderers/shared/fiber/ReactFiberReconciler.js
@@ -37,7 +37,10 @@ if (__DEV__) {
   var getComponentName = require('getComponentName');
 }
 
-var {findCurrentHostFiber} = require('ReactFiberTreeReflection');
+var {
+  findCurrentHostFiber,
+  findCurrentHostFiberWithNoPortals,
+} = require('ReactFiberTreeReflection');
 
 var getContextForSubtree = require('getContextForSubtree');
 
@@ -173,6 +176,9 @@ export type Reconciler<C, I, TI> = {
 
   // Use for findDOMNode/findHostNode. Legacy API.
   findHostInstance(component: Fiber): I | TI | null,
+
+  // Used internally for filtering out portals. Legacy API.
+  findHostInstanceWithNoPortals(component: Fiber): I | TI | null,
 };
 
 getContextForSubtree._injectFiber(function(fiber: Fiber) {
@@ -309,5 +315,13 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
       }
       return hostFiber.stateNode;
     },
+
+    findHostInstanceWithNoPortals(fiber: Fiber): PI | null {
+      const hostFiber = findCurrentHostFiberWithNoPortals(fiber);
+      if (hostFiber === null) {
+        return null;
+      }
+      return hostFiber.stateNode;
+    },
   };
 };
diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js
index ce6e0a354bac0..3ea6368a11101 100644
--- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js
+++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js
@@ -237,6 +237,41 @@ exports.findCurrentHostFiber = function(parent: Fiber): Fiber | null {
     return null;
   }
 
+  // Next we'll drill down this component to find the first HostComponent/Text.
+  let node: Fiber = currentParent;
+  while (true) {
+    if (node.tag === HostComponent || node.tag === HostText) {
+      return node;
+    } else if (node.child) {
+      node.child.return = node;
+      node = node.child;
+      continue;
+    }
+    if (node === currentParent) {
+      return null;
+    }
+    while (!node.sibling) {
+      if (!node.return || node.return === currentParent) {
+        return null;
+      }
+      node = node.return;
+    }
+    node.sibling.return = node.return;
+    node = node.sibling;
+  }
+  // Flow needs the return null here, but ESLint complains about it.
+  // eslint-disable-next-line no-unreachable
+  return null;
+};
+
+exports.findCurrentHostFiberWithNoPortals = function(
+  parent: Fiber,
+): Fiber | null {
+  const currentParent = findCurrentFiberUsingSlowPath(parent);
+  if (!currentParent) {
+    return null;
+  }
+
   // Next we'll drill down this component to find the first HostComponent/Text.
   let node: Fiber = currentParent;
   while (true) {