From 8ce7b7120cff9f46237fef73120c9ddca25e30fc Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sat, 26 Sep 2015 15:11:44 -0700 Subject: [PATCH] ReactMount now never expects invalid nodes in its cache It never really made sense for us to have "invalid" nodes in the cache -- when we unmount things, we should always remove them from the cache properly. Now that swapping composite types doesn't repopulate the cache, we should be okay to now assume that everything in the cache is good. --- src/renderers/dom/client/ReactMount.js | 58 +++++++++++++++----------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 41a370f783..8ab20475fe 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -100,7 +100,7 @@ function getReactRootElementInContainer(container) { */ function getReactRootID(container) { var rootElement = getReactRootElementInContainer(container); - return rootElement && ReactMount.getID(rootElement); + return rootElement && internalGetID(rootElement); } /** @@ -116,20 +116,12 @@ function getReactRootID(container) { function getID(node) { var id = internalGetID(node); if (id) { - if (nodeCache.hasOwnProperty(id)) { - var cached = nodeCache[id]; - if (cached !== node) { - invariant( - !isValid(cached, id), - 'ReactMount: Two valid but unequal nodes with the same `%s`: %s', - ATTR_NAME, id - ); - - nodeCache[id] = node; - } - } else { - nodeCache[id] = node; - } + invariant( + !nodeCache.hasOwnProperty(id) || nodeCache[id] === node, + 'ReactMount: Two unequal nodes with the same `%s`: %s', + ATTR_NAME, id + ); + nodeCache[id] = node; } return id; @@ -157,6 +149,25 @@ function setID(node, id) { nodeCache[id] = node; } +/** + * Finds the node with the supplied ID if present in the cache. + */ +function getNodeIfCached(id) { + var node = nodeCache[id]; + // TODO: Since this "isValid" business is now just a sanity check, we can + // probably drop it with no consequences. + invariant( + !node || isValid(node, id), + 'ReactMount: Cached node with `%s`: %s is missing from the document. ' + + 'This probably means the DOM was unexpectedly mutated -- when removing ' + + 'React-rendered children from the DOM, rerender without those children ' + + 'or call ReactDOM.unmountComponentAtNode on the container to unmount an ' + + 'entire subtree.', + ATTR_NAME, id + ); + return node; +} + /** * Finds the node with the supplied React-generated DOM ID. * @@ -165,10 +176,12 @@ function setID(node, id) { * @internal */ function getNode(id) { - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); + var node = getNodeIfCached(id); + if (node) { + return node; + } else { + return nodeCache[id] = ReactMount.findReactNodeByID(id); } - return nodeCache[id]; } /** @@ -183,10 +196,7 @@ function getNodeFromInstance(instance) { if (ReactEmptyComponentRegistry.isNullComponentID(id)) { return null; } - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); - } - return nodeCache[id]; + return getNode(id); } /** @@ -227,8 +237,8 @@ function purgeID(id) { var deepestNodeSoFar = null; function findDeepestCachedAncestorImpl(ancestorID) { - var ancestor = nodeCache[ancestorID]; - if (ancestor && isValid(ancestor, ancestorID)) { + var ancestor = getNodeIfCached(ancestorID); + if (ancestor) { deepestNodeSoFar = ancestor; } else { // This node isn't populated in the cache, so presumably none of its