Skip to content

Commit dc2570e

Browse files
sophiebitssophiebits
committed
Merge pull request #4825 from spicyj/gh-2770
Preserve DOM node when updating empty component
2 parents 14ede77 + db589a7 commit dc2570e

11 files changed

+152
-104
lines changed

‎src/renderers/dom/client/ReactMount.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
1616
var ReactCurrentOwner = require('ReactCurrentOwner');
1717
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
1818
var ReactElement = require('ReactElement');
19-
var ReactEmptyComponent = require('ReactEmptyComponent');
19+
var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry');
2020
var ReactInstanceHandles = require('ReactInstanceHandles');
2121
var ReactInstanceMap = require('ReactInstanceMap');
2222
var ReactMarkupChecksum = require('ReactMarkupChecksum');
@@ -181,7 +181,7 @@ function getNode(id) {
181181
*/
182182
function getNodeFromInstance(instance) {
183183
var id = ReactInstanceMap.get(instance)._rootNodeID;
184-
if (ReactEmptyComponent.isNullComponentID(id)) {
184+
if (ReactEmptyComponentRegistry.isNullComponentID(id)) {
185185
return null;
186186
}
187187
if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) {

‎src/renderers/shared/reconciler/ReactChildReconciler.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ var ReactChildReconciler = {
9090
var prevChild = prevChildren && prevChildren[name];
9191
var prevElement = prevChild && prevChild._currentElement;
9292
var nextElement = nextChildren[name];
93-
if (shouldUpdateReactComponent(prevElement, nextElement)) {
93+
if (prevChild != null &&
94+
shouldUpdateReactComponent(prevElement, nextElement)) {
9495
ReactReconciler.receiveComponent(
9596
prevChild, nextElement, transaction, context
9697
);

‎src/renderers/shared/reconciler/ReactEmptyComponent.js‎

Lines changed: 33 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -12,81 +12,47 @@
1212
'use strict';
1313

1414
var ReactElement = require('ReactElement');
15-
var ReactInstanceMap = require('ReactInstanceMap');
15+
var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry');
16+
var ReactReconciler = require('ReactReconciler');
1617

17-
var invariant = require('invariant');
18+
var assign = require('Object.assign');
1819

19-
var component;
20-
// This registry keeps track of the React IDs of the components that rendered to
21-
// `null` (in reality a placeholder such as `noscript`)
22-
var nullComponentIDsRegistry = {};
20+
var placeholderElement;
2321

2422
var ReactEmptyComponentInjection = {
25-
injectEmptyComponent: function(emptyComponent) {
26-
component = ReactElement.createFactory(emptyComponent);
23+
injectEmptyComponent: function(component) {
24+
placeholderElement = ReactElement.createElement(component);
2725
},
2826
};
2927

30-
var ReactEmptyComponentType = function() {};
31-
ReactEmptyComponentType.isReactClass = {};
32-
ReactEmptyComponentType.prototype.componentDidMount = function() {
33-
var internalInstance = ReactInstanceMap.get(this);
34-
// TODO: Make sure we run these methods in the correct order, we shouldn't
35-
// need this check. We're going to assume if we're here it means we ran
36-
// componentWillUnmount already so there is no internal instance (it gets
37-
// removed as part of the unmounting process).
38-
if (!internalInstance) {
39-
return;
40-
}
41-
registerNullComponentID(internalInstance._rootNodeID);
28+
var ReactEmptyComponent = function(instantiate) {
29+
this._currentElement = null;
30+
this._rootNodeID = null;
31+
this._renderedComponent = instantiate(placeholderElement);
4232
};
43-
ReactEmptyComponentType.prototype.componentWillUnmount = function() {
44-
var internalInstance = ReactInstanceMap.get(this);
45-
// TODO: Get rid of this check. See TODO in componentDidMount.
46-
if (!internalInstance) {
47-
return;
48-
}
49-
deregisterNullComponentID(internalInstance._rootNodeID);
50-
};
51-
ReactEmptyComponentType.prototype.render = function() {
52-
invariant(
53-
component,
54-
'Trying to return null from a render, but no null placeholder component ' +
55-
'was injected.'
56-
);
57-
return component();
58-
};
59-
60-
var emptyElement = ReactElement.createElement(ReactEmptyComponentType);
61-
62-
/**
63-
* Mark the component as having rendered to null.
64-
* @param {string} id Component's `_rootNodeID`.
65-
*/
66-
function registerNullComponentID(id) {
67-
nullComponentIDsRegistry[id] = true;
68-
}
69-
70-
/**
71-
* Unmark the component as having rendered to null: it renders to something now.
72-
* @param {string} id Component's `_rootNodeID`.
73-
*/
74-
function deregisterNullComponentID(id) {
75-
delete nullComponentIDsRegistry[id];
76-
}
77-
78-
/**
79-
* @param {string} id Component's `_rootNodeID`.
80-
* @return {boolean} True if the component is rendered to null.
81-
*/
82-
function isNullComponentID(id) {
83-
return !!nullComponentIDsRegistry[id];
84-
}
33+
assign(ReactEmptyComponent.prototype, {
34+
construct: function(element) {
35+
},
36+
mountComponent: function(rootID, transaction, context) {
37+
ReactEmptyComponentRegistry.registerNullComponentID(rootID);
38+
this._rootNodeID = rootID;
39+
return ReactReconciler.mountComponent(
40+
this._renderedComponent,
41+
rootID,
42+
transaction,
43+
context
44+
);
45+
},
46+
receiveComponent: function() {
47+
},
48+
unmountComponent: function(rootID, transaction, context) {
49+
ReactReconciler.unmountComponent(this._renderedComponent);
50+
ReactEmptyComponentRegistry.deregisterNullComponentID(this._rootNodeID);
51+
this._rootNodeID = null;
52+
this._renderedComponent = null;
53+
},
54+
});
8555

86-
var ReactEmptyComponent = {
87-
emptyElement: emptyElement,
88-
injection: ReactEmptyComponentInjection,
89-
isNullComponentID: isNullComponentID,
90-
};
56+
ReactEmptyComponent.injection = ReactEmptyComponentInjection;
9157

9258
module.exports = ReactEmptyComponent;

‎src/renderers/shared/reconciler/ReactEmptyComponentRegistry.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Copyright 2014-2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule ReactEmptyComponentRegistry
10+
*/
11+
12+
'use strict';
13+
14+
// This registry keeps track of the React IDs of the components that rendered to
15+
// `null` (in reality a placeholder such as `noscript`)
16+
var nullComponentIDsRegistry = {};
17+
18+
/**
19+
* @param {string} id Component's `_rootNodeID`.
20+
* @return {boolean} True if the component is rendered to null.
21+
*/
22+
function isNullComponentID(id) {
23+
return !!nullComponentIDsRegistry[id];
24+
}
25+
26+
/**
27+
* Mark the component as having rendered to null.
28+
* @param {string} id Component's `_rootNodeID`.
29+
*/
30+
function registerNullComponentID(id) {
31+
nullComponentIDsRegistry[id] = true;
32+
}
33+
34+
/**
35+
* Unmark the component as having rendered to null: it renders to something now.
36+
* @param {string} id Component's `_rootNodeID`.
37+
*/
38+
function deregisterNullComponentID(id) {
39+
delete nullComponentIDsRegistry[id];
40+
}
41+
42+
var ReactEmptyComponentRegistry = {
43+
isNullComponentID: isNullComponentID,
44+
registerNullComponentID: registerNullComponentID,
45+
deregisterNullComponentID: deregisterNullComponentID,
46+
};
47+
48+
module.exports = ReactEmptyComponentRegistry;

‎src/renderers/shared/reconciler/ReactReconciler.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ var ReactReconciler = {
3535
*/
3636
mountComponent: function(internalInstance, rootID, transaction, context) {
3737
var markup = internalInstance.mountComponent(rootID, transaction, context);
38-
if (internalInstance._currentElement.ref != null) {
38+
if (internalInstance._currentElement &&
39+
internalInstance._currentElement.ref != null) {
3940
transaction.getReactMountReady().enqueue(attachRefs, internalInstance);
4041
}
4142
return markup;
@@ -93,7 +94,9 @@ var ReactReconciler = {
9394

9495
internalInstance.receiveComponent(nextElement, transaction, context);
9596

96-
if (refsChanged) {
97+
if (refsChanged &&
98+
internalInstance._currentElement &&
99+
internalInstance._currentElement.ref != null) {
97100
transaction.getReactMountReady().enqueue(attachRefs, internalInstance);
98101
}
99102
},

‎src/renderers/shared/reconciler/ReactRef.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ function detachRef(ref, component, owner) {
3434
}
3535

3636
ReactRef.attachRefs = function(instance, element) {
37+
if (element === null || element === false) {
38+
return;
39+
}
3740
var ref = element.ref;
3841
if (ref != null) {
3942
attachRef(ref, instance, element._owner);
@@ -53,13 +56,21 @@ ReactRef.shouldUpdateRefs = function(prevElement, nextElement) {
5356
// is made. It probably belongs where the key checking and
5457
// instantiateReactComponent is done.
5558

59+
var prevEmpty = prevElement === null || prevElement === false;
60+
var nextEmpty = nextElement === null || nextElement === false;
61+
5662
return (
63+
// This has a few false positives w/r/t empty components.
64+
prevEmpty || nextEmpty ||
5765
nextElement._owner !== prevElement._owner ||
5866
nextElement.ref !== prevElement.ref
5967
);
6068
};
6169

6270
ReactRef.detachRefs = function(instance, element) {
71+
if (element === null || element === false) {
72+
return;
73+
}
6374
var ref = element.ref;
6475
if (ref != null) {
6576
detachRef(ref, instance, element._owner);

‎src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
var React;
1515
var ReactDOM;
16-
var ReactEmptyComponent;
1716
var ReactTestUtils;
1817
var TogglingComponent;
1918

@@ -25,7 +24,6 @@ describe('ReactEmptyComponent', function() {
2524

2625
React = require('React');
2726
ReactDOM = require('ReactDOM');
28-
ReactEmptyComponent = require('ReactEmptyComponent');
2927
ReactTestUtils = require('ReactTestUtils');
3028

3129
reactComponentExpect = require('reactComponentExpect');
@@ -64,10 +62,10 @@ describe('ReactEmptyComponent', function() {
6462
var instance2 = ReactTestUtils.renderIntoDocument(<Component2 />);
6563
reactComponentExpect(instance1)
6664
.expectRenderedChild()
67-
.toBeComponentOfType(ReactEmptyComponent.emptyElement.type);
65+
.toBeEmptyComponent();
6866
reactComponentExpect(instance2)
6967
.expectRenderedChild()
70-
.toBeComponentOfType(ReactEmptyComponent.emptyElement.type);
68+
.toBeEmptyComponent();
7169
});
7270

7371
it('should still throw when rendering to undefined', () => {
@@ -97,10 +95,8 @@ describe('ReactEmptyComponent', function() {
9795
secondComponent={null}
9896
/>;
9997

100-
expect(function() {
101-
ReactTestUtils.renderIntoDocument(instance1);
102-
ReactTestUtils.renderIntoDocument(instance2);
103-
}).not.toThrow();
98+
ReactTestUtils.renderIntoDocument(instance1);
99+
ReactTestUtils.renderIntoDocument(instance2);
104100

105101
expect(console.log.argsForCall.length).toBe(4);
106102
expect(console.log.argsForCall[0][0]).toBe(null);
@@ -269,4 +265,25 @@ describe('ReactEmptyComponent', function() {
269265
ReactTestUtils.renderIntoDocument(<Parent />);
270266
}).not.toThrow();
271267
});
268+
269+
it('preserves the dom node during updates', function() {
270+
var Empty = React.createClass({
271+
render: function() {
272+
return null;
273+
},
274+
});
275+
276+
var container = document.createElement('div');
277+
278+
ReactDOM.render(<Empty />, container);
279+
var noscript1 = container.firstChild;
280+
expect(noscript1.tagName).toBe('NOSCRIPT');
281+
282+
// This update shouldn't create a DOM node
283+
ReactDOM.render(<Empty />, container);
284+
var noscript2 = container.firstChild;
285+
expect(noscript2.tagName).toBe('NOSCRIPT');
286+
287+
expect(noscript1).toBe(noscript2);
288+
});
272289
});

‎src/renderers/shared/reconciler/instantiateReactComponent.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,8 @@ function instantiateReactComponent(node) {
6767
var instance;
6868

6969
if (node === null || node === false) {
70-
node = ReactEmptyComponent.emptyElement;
71-
}
72-
73-
if (typeof node === 'object') {
70+
instance = new ReactEmptyComponent(instantiateReactComponent);
71+
} else if (typeof node === 'object') {
7472
var element = node;
7573
invariant(
7674
element && (typeof element.type === 'function' ||
@@ -86,7 +84,7 @@ function instantiateReactComponent(node) {
8684
instance = ReactNativeComponent.createInternalComponent(element);
8785
} else if (isInternalComponentType(element.type)) {
8886
// This is temporarily available for custom components that are not string
89-
// represenations. I.e. ART. Once those are updated to use the string
87+
// representations. I.e. ART. Once those are updated to use the string
9088
// representation, we can drop this code path.
9189
instance = new element.type(element);
9290
} else {

‎src/renderers/shared/reconciler/shouldUpdateReactComponent.js‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,22 @@
2424
* @protected
2525
*/
2626
function shouldUpdateReactComponent(prevElement, nextElement) {
27-
if (prevElement != null && nextElement != null) {
28-
var prevType = typeof prevElement;
29-
var nextType = typeof nextElement;
30-
if (prevType === 'string' || prevType === 'number') {
31-
return (nextType === 'string' || nextType === 'number');
32-
} else {
33-
return (
34-
nextType === 'object' &&
35-
prevElement.type === nextElement.type &&
36-
prevElement.key === nextElement.key
37-
);
38-
}
27+
var prevEmpty = prevElement === null || prevElement === false;
28+
var nextEmpty = nextElement === null || nextElement === false;
29+
if (prevEmpty || nextEmpty) {
30+
return prevEmpty === nextEmpty;
31+
}
32+
33+
var prevType = typeof prevElement;
34+
var nextType = typeof nextElement;
35+
if (prevType === 'string' || prevType === 'number') {
36+
return (nextType === 'string' || nextType === 'number');
37+
} else {
38+
return (
39+
nextType === 'object' &&
40+
prevElement.type === nextElement.type &&
41+
prevElement.key === nextElement.key
42+
);
3943
}
4044
return false;
4145
}

‎src/test/ReactTestUtils.js‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ var EventPropagators = require('EventPropagators');
1717
var React = require('React');
1818
var ReactDOM = require('ReactDOM');
1919
var ReactElement = require('ReactElement');
20-
var ReactEmptyComponent = require('ReactEmptyComponent');
2120
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
2221
var ReactCompositeComponent = require('ReactCompositeComponent');
2322
var ReactInstanceHandles = require('ReactInstanceHandles');
@@ -359,9 +358,7 @@ ReactShallowRenderer.prototype.getRenderOutput = function() {
359358

360359
var NoopInternalComponent = function(element) {
361360
this._renderedOutput = element;
362-
this._currentElement = element === null || element === false ?
363-
ReactEmptyComponent.emptyElement :
364-
element;
361+
this._currentElement = element;
365362
};
366363

367364
NoopInternalComponent.prototype = {
@@ -371,9 +368,7 @@ NoopInternalComponent.prototype = {
371368

372369
receiveComponent: function(element) {
373370
this._renderedOutput = element;
374-
this._currentElement = element === null || element === false ?
375-
ReactEmptyComponent.emptyElement :
376-
element;
371+
this._currentElement = element;
377372
},
378373

379374
unmountComponent: function() {

0 commit comments

Comments
 (0)