Skip to content

Commit

Permalink
ReactMount now never expects invalid nodes in its cache
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sophiebits committed Oct 7, 2015
1 parent fe9a76e commit 8ce7b71
Showing 1 changed file with 34 additions and 24 deletions.
58 changes: 34 additions & 24 deletions src/renderers/dom/client/ReactMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function getReactRootElementInContainer(container) {
*/
function getReactRootID(container) {
var rootElement = getReactRootElementInContainer(container);
return rootElement && ReactMount.getID(rootElement);
return rootElement && internalGetID(rootElement);
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -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.
*
Expand All @@ -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];
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 8ce7b71

Please sign in to comment.