From 3ba8069fefed06dae0e3be74b9c42734752e1580 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 9 Dec 2016 15:17:17 +0000 Subject: [PATCH] Fix portal unmounting 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. --- scripts/fiber/tests-failing.txt | 1 - scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberCommitWork.js | 23 +++++++++++++++---- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 04626aca6a312..e0ccac276681e 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -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 diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 119983286a6ef..da2bae16f6a60 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -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 diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 5e31b11fcc12b..5432935624e03 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -265,16 +265,19 @@ module.exports = function( // 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; @@ -302,10 +305,22 @@ module.exports = function( } 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