Be more lenient with invalid nodes in the cache
This matches our old behavior. I was a little too aggressive in turning on this error and it's still possible to trigger it (using the test added here).
Ben Alpert committed
Oct 13, 2015 at 15:04 UTC
e89e6755082f6b690ea4aaf280716a9fc2e9e9b0
2 files changed
+35
-14
src/renderers/dom/client/ReactMount.js
+10
-14
@@ -111,9 +111,12 @@ function getReactRootID(container) {
111
function getID(node) {
112
var id = internalGetID(node);
113
if (id) {
114
+ var cached = nodeCache[id];
115
+ // TODO: Fix this whole concept of "validity" -- the cache just shouldn't
116
+ // have nodes that have been unmounted.
117
invariant(
115
- !nodeCache.hasOwnProperty(id) || nodeCache[id] === node,
116
- 'ReactMount: Two unequal nodes with the same `%s`: %s',
118
+ !cached || cached === node || !isValid(cached, id),
119
+ 'ReactMount: Two valid but unequal nodes with the same `%s`: %s',
120
ATTR_NAME, id
121
);
122
nodeCache[id] = node;
@@ -149,18 +152,11 @@ function setID(node, id) {
152
*/
153
function getNodeIfCached(id) {
154
var node = nodeCache[id];
152
- // TODO: Since this "isValid" business is now just a sanity check, we can
153
- // probably drop it with no consequences.
154
- invariant(
155
- !node || isValid(node, id),
156
- 'ReactMount: Cached node with `%s`: %s is missing from the document. ' +
157
- 'This probably means the DOM was unexpectedly mutated -- when removing ' +
158
- 'React-rendered children from the DOM, rerender without those children ' +
159
- 'or call ReactDOM.unmountComponentAtNode on the container to unmount an ' +
160
- 'entire subtree.',
161
- ATTR_NAME, id
162
- );
163
- return node;
155
+ // TODO: Fix this whole concept of "validity" -- the cache just shouldn't have
156
+ // nodes that have been unmounted.
157
+ if (node && isValid(node, id)) {
158
+ return node;
159
+ }
160
}
161
162
/**
src/renderers/dom/client/__tests__/ReactMount-test.js
+25
@@ -275,4 +275,29 @@ describe('ReactMount', function() {
275
276
ReactDOM.unmountComponentAtNode(container);
277
});
278
+
279
+ it('should not crash in node cache when unmounting, case 2', function() {
280
+ var A = React.createClass({
281
+ render: function() {
282
+ return <a key={this.props.innerKey}>{this.props.innerKey}</a>;
283
+ },
284
+ });
285
+ var Component = React.createClass({
286
+ render: function() {
287
+ return (
288
+ <b>
289
+ <i>{this.props.step === 1 && <q />}</i>
290
+ {this.props.step === 1 && <A innerKey={this.props.step} />}
291
+ </b>
292
+ );
293
+ },
294
+ });
295
+
296
+ var container = document.createElement('container');
297
+
298
+ ReactDOM.render(<Component step={1} />, container);
299
+ ReactDOM.render(<Component step={2} />, container);
300
+ ReactDOM.render(<Component step={1} />, container);
301
+ ReactMount.getID(container.querySelector('a'));
302
+ });
303
});