Skip to content

Commit

Permalink
Fix portal unmounting
Browse files Browse the repository at this point in the history
When unmount a portal, we need to unmount *its* children from itself.
This is similar to what we would do for a root if we allowed deleting roots.
  • Loading branch information
gaearon committed Dec 9, 2016
1 parent 8eb192b commit 3ba8069
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
1 change: 0 additions & 1 deletion scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should throw with an error code in production

src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should keep track of namespace across portals (medium)
* should unwind namespaces on caught errors in a portal

src/renderers/dom/shared/__tests__/ReactDOM-test.js
Expand Down
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render many portals
* should render nested portals
* should keep track of namespace across portals (simple)
* should keep track of namespace across portals (medium)
* should keep track of namespace across portals (complex)
* should unwind namespaces on uncaught errors
* should unwind namespaces on caught errors
Expand Down
23 changes: 19 additions & 4 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,16 +265,19 @@ module.exports = function<T, P, I, TI, C, CX>(
// After all the children have unmounted, it is now safe to remove the
// node from the tree.
removeChild(parent, node.stateNode);
// Don't visit children because we already visited them.
} else if (node.tag === HostPortal) {
// When we go into a portal, it becomes the parent to remove from.
// We will reassign it back when we pop the portal on the way up.
parent = node.stateNode.containerInfo;
// Visit children because portals might contain host components.
if (node.child) {
node = node.child;
continue;
}
} else {
commitUnmount(node);
// Visit children because we may find more host components below.
if (node.child) {
// TODO: Coroutines need to visit the stateNode.
node.child.return = node;
Expand Down Expand Up @@ -302,10 +305,22 @@ module.exports = function<T, P, I, TI, C, CX>(
}

function commitDeletion(current : Fiber) : void {
// Recursively delete all host nodes from the parent.
const parent = getHostParent(current);
// Detach refs and call componentWillUnmount() on the whole subtree.
unmountHostComponents(parent, current);
// Recursively delete the closest child host nodes from the closest host parent.
// Then detach refs and call componentWillUnmount() on the whole subtree.
if (current.tag === HostPortal) {
// When deleting a portal, there are no host nodes above it.
// It is as a host parent itself.
const parent = current.stateNode.containerInfo;
let child = current.child;
while (child) {
unmountHostComponents(parent, child);
child = child.sibling;
}
} else {
// When deleting anything other than a portal, search for the host parent.
const parent = getHostParent(current);
unmountHostComponents(parent, current);
}

// Cut off the return pointers to disconnect it from the tree. Ideally, we
// should clear the child pointer of the parent alternate to let this
Expand Down

0 comments on commit 3ba8069

Please sign in to comment.