Skip to content

Commit

Permalink
Add flag to portalContainer on mount instead of in createPortal
Browse files Browse the repository at this point in the history
**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:**
facebook#8854
  • Loading branch information
flarnie committed Jul 20, 2017
1 parent c927309 commit 7f9648b
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions src/renderers/shared/fiber/isomorphic/ReactPortal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 7f9648b

Please sign in to comment.