Skip to content

Commit

Permalink
Ignore portals in DOMRenderer.findHostInstance
Browse files Browse the repository at this point in the history
**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:**
facebook#8854
  • Loading branch information
flarnie committed Jul 20, 2017
1 parent 2225b98 commit f7efe64
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 12 deletions.
11 changes: 3 additions & 8 deletions src/renderers/dom/fiber/ReactDOMFiberEntry.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ' +
Expand Down
@@ -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`] = `""`;
8 changes: 4 additions & 4 deletions src/renderers/shared/fiber/ReactFiberTreeReflection.js
Expand Up @@ -25,10 +25,11 @@ if (__DEV__) {
}

var {
HostRoot,
ClassComponent,
HostComponent,
HostRoot,
HostPortal,
HostText,
ClassComponent,
} = require('ReactTypeOfWork');

var {NoEffect, Placement} = require('ReactTypeOfSideEffect');
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit f7efe64

Please sign in to comment.