Refactor empty component handling
Now doesn't use ReactCompositeComponent and `._currentElement` is actually null/false.
Ben Alpert committed
Sep 9, 2015 at 13:52 UTC
999b0f9b3e2d0b1e83c0b6339ed0af876702b2f3
9 files changed
+113
-91
src/renderers/dom/client/ReactMount.js
+2
-2
@@ -16,7 +16,7 @@ var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
16
var ReactCurrentOwner = require('ReactCurrentOwner');
17
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
18
var ReactElement = require('ReactElement');
19
-var ReactEmptyComponent = require('ReactEmptyComponent');
19
+var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry');
20
var ReactInstanceHandles = require('ReactInstanceHandles');
21
var ReactInstanceMap = require('ReactInstanceMap');
22
var ReactMarkupChecksum = require('ReactMarkupChecksum');
@@ -181,7 +181,7 @@ function getNode(id) {
181
*/
182
function getNodeFromInstance(instance) {
183
var id = ReactInstanceMap.get(instance)._rootNodeID;
184
- if (ReactEmptyComponent.isNullComponentID(id)) {
184
+ if (ReactEmptyComponentRegistry.isNullComponentID(id)) {
185
return null;
186
}
187
if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) {
src/renderers/shared/reconciler/ReactEmptyComponent.js
+33
-67
@@ -12,81 +12,47 @@
12
'use strict';
13
14
var ReactElement = require('ReactElement');
15
-var ReactInstanceMap = require('ReactInstanceMap');
15
+var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry');
16
+var ReactReconciler = require('ReactReconciler');
17
17
-var invariant = require('invariant');
18
+var assign = require('Object.assign');
19
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;
21
22
var ReactEmptyComponentInjection = {
25
- injectEmptyComponent: function(emptyComponent) {
26
- component = ReactElement.createFactory(emptyComponent);
23
+ injectEmptyComponent: function(component) {
24
+ placeholderElement = ReactElement.createElement(component);
25
},
26
};
27
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);
32
};
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
+});
55
86
-var ReactEmptyComponent = {
87
- emptyElement: emptyElement,
88
- injection: ReactEmptyComponentInjection,
89
- isNullComponentID: isNullComponentID,
90
-};
56
+ReactEmptyComponent.injection = ReactEmptyComponentInjection;
57
58
module.exports = ReactEmptyComponent;
src/renderers/shared/reconciler/ReactEmptyComponentRegistry.js
new
+48
@@ -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
+5
-2
@@ -35,7 +35,8 @@ var ReactReconciler = {
35
*/
36
mountComponent: function(internalInstance, rootID, transaction, context) {
37
var markup = internalInstance.mountComponent(rootID, transaction, context);
38
- if (internalInstance._currentElement.ref != null) {
38
+ if (internalInstance._currentElement &&
39
+ internalInstance._currentElement.ref != null) {
40
transaction.getReactMountReady().enqueue(attachRefs, internalInstance);
41
}
42
return markup;
@@ -93,7 +94,9 @@ var ReactReconciler = {
94
95
internalInstance.receiveComponent(nextElement, transaction, context);
96
96
- if (refsChanged) {
97
+ if (refsChanged &&
98
+ internalInstance._currentElement &&
99
+ internalInstance._currentElement.ref != null) {
100
transaction.getReactMountReady().enqueue(attachRefs, internalInstance);
101
}
102
},
src/renderers/shared/reconciler/ReactRef.js
+11
@@ -34,6 +34,9 @@ function detachRef(ref, component, owner) {
34
}
35
36
ReactRef.attachRefs = function(instance, element) {
37
+ if (element === null || element === false) {
38
+ return;
39
+ }
40
var ref = element.ref;
41
if (ref != null) {
42
attachRef(ref, instance, element._owner);
@@ -53,13 +56,21 @@ ReactRef.shouldUpdateRefs = function(prevElement, nextElement) {
56
// is made. It probably belongs where the key checking and
57
// instantiateReactComponent is done.
58
59
+ var prevEmpty = prevElement === null || prevElement === false;
60
+ var nextEmpty = nextElement === null || nextElement === false;
61
+
62
return (
63
+ // This has a few false positives w/r/t empty components.
64
+ prevEmpty || nextEmpty ||
65
nextElement._owner !== prevElement._owner ||
66
nextElement.ref !== prevElement.ref
67
);
68
};
69
70
ReactRef.detachRefs = function(instance, element) {
71
+ if (element === null || element === false) {
72
+ return;
73
+ }
74
var ref = element.ref;
75
if (ref != null) {
76
detachRef(ref, instance, element._owner);
src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js
+4
-8
@@ -13,7 +13,6 @@
13
14
var React;
15
var ReactDOM;
16
-var ReactEmptyComponent;
16
var ReactTestUtils;
17
var TogglingComponent;
18
@@ -25,7 +24,6 @@ describe('ReactEmptyComponent', function() {
24
25
React = require('React');
26
ReactDOM = require('ReactDOM');
28
- ReactEmptyComponent = require('ReactEmptyComponent');
27
ReactTestUtils = require('ReactTestUtils');
28
29
reactComponentExpect = require('reactComponentExpect');
@@ -64,10 +62,10 @@ describe('ReactEmptyComponent', function() {
62
var instance2 = ReactTestUtils.renderIntoDocument(<Component2 />);
63
reactComponentExpect(instance1)
64
.expectRenderedChild()
67
- .toBeComponentOfType(ReactEmptyComponent.emptyElement.type);
65
+ .toBeEmptyComponent();
66
reactComponentExpect(instance2)
67
.expectRenderedChild()
70
- .toBeComponentOfType(ReactEmptyComponent.emptyElement.type);
68
+ .toBeEmptyComponent();
69
});
70
71
it('should still throw when rendering to undefined', () => {
@@ -97,10 +95,8 @@ describe('ReactEmptyComponent', function() {
95
secondComponent={null}
96
/>;
97
100
- expect(function() {
101
- ReactTestUtils.renderIntoDocument(instance1);
102
- ReactTestUtils.renderIntoDocument(instance2);
103
- }).not.toThrow();
98
+ ReactTestUtils.renderIntoDocument(instance1);
99
+ ReactTestUtils.renderIntoDocument(instance2);
100
101
expect(console.log.argsForCall.length).toBe(4);
102
expect(console.log.argsForCall[0][0]).toBe(null);
src/renderers/shared/reconciler/instantiateReactComponent.js
+3
-5
@@ -67,10 +67,8 @@ function instantiateReactComponent(node) {
67
var instance;
68
69
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') {
72
var element = node;
73
invariant(
74
element && (typeof element.type === 'function' ||
@@ -86,7 +84,7 @@ function instantiateReactComponent(node) {
84
instance = ReactNativeComponent.createInternalComponent(element);
85
} else if (isInternalComponentType(element.type)) {
86
// 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
88
// representation, we can drop this code path.
89
instance = new element.type(element);
90
} else {
src/test/ReactTestUtils.js
+2
-7
@@ -17,7 +17,6 @@ var EventPropagators = require('EventPropagators');
17
var React = require('React');
18
var ReactDOM = require('ReactDOM');
19
var ReactElement = require('ReactElement');
20
-var ReactEmptyComponent = require('ReactEmptyComponent');
20
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
21
var ReactCompositeComponent = require('ReactCompositeComponent');
22
var ReactInstanceHandles = require('ReactInstanceHandles');
@@ -359,9 +358,7 @@ ReactShallowRenderer.prototype.getRenderOutput = function() {
358
359
var NoopInternalComponent = function(element) {
360
this._renderedOutput = element;
362
- this._currentElement = element === null || element === false ?
363
- ReactEmptyComponent.emptyElement :
364
- element;
361
+ this._currentElement = element;
362
};
363
364
NoopInternalComponent.prototype = {
@@ -371,9 +368,7 @@ NoopInternalComponent.prototype = {
368
369
receiveComponent: function(element) {
370
this._renderedOutput = element;
374
- this._currentElement = element === null || element === false ?
375
- ReactEmptyComponent.emptyElement :
376
- element;
371
+ this._currentElement = element;
372
},
373
374
unmountComponent: function() {
src/test/reactComponentExpect.js
+5
@@ -150,6 +150,11 @@ assign(reactComponentExpectInternal.prototype, {
150
return this;
151
},
152
153
+ toBeEmptyComponent: function() {
154
+ var element = this._instance._currentElement;
155
+ return element === null || element === false;
156
+ },
157
+
158
toBePresent: function() {
159
expect(this.instance()).toBeTruthy();
160
return this;