@samitouri / QOS-React-2 / commits / 9c3e2d833d

Wrap every DOM node in a Composite Component

...unless they already have a wrapper. Also, add tagName to every wrapper. This ensures that refs are consistent. They always look like composite components. This effectively hides the internal implementation details of real DOM components since you can no longer get a ref to one. In the future we might want to drop this wrapper and have refs refer directly to the DOM node. I currently use a hacky way of auto-wrapping inside of ReactNativeComponent so that any given string can be wrapped. Better suggestions are welcome.

Sebastian Markbage committed Nov 16, 2014 at 10:32 UTC 9c3e2d833d339eb885f1a1549e0043e768bdb209
18 files changed +80 -61
src/browser/__tests__/ReactDOM-test.js
+3 -4
@@ -15,7 +15,6 @@
15
16 var React = require('React');
17 var ReactDOM = require('ReactDOM');
18 -var ReactMount = require('ReactMount');
18 var ReactTestUtils = require('ReactTestUtils');
19 var div = React.createFactory('div');
20
@@ -53,7 +52,7 @@ describe('ReactDOM', function() {
52 var argDiv = ReactTestUtils.renderIntoDocument(
53 div(null, 'child')
54 );
56 - var argNode = ReactMount.getNode(argDiv._rootNodeID);
55 + var argNode = argDiv.getDOMNode();
56 expect(argNode.innerHTML).toBe('child');
57 });
58
@@ -61,7 +60,7 @@ describe('ReactDOM', function() {
60 var conflictDiv = ReactTestUtils.renderIntoDocument(
61 div({children: 'fakechild'}, 'child')
62 );
64 - var conflictNode = ReactMount.getNode(conflictDiv._rootNodeID);
63 + var conflictNode = conflictDiv.getDOMNode();
64 expect(conflictNode.innerHTML).toBe('child');
65 });
66
@@ -103,7 +102,7 @@ describe('ReactDOM', function() {
102 theBird: <div className="bird" />
103 }
104 });
106 - var root = ReactMount.getNode(myDiv._rootNodeID);
105 + var root = myDiv.getDOMNode();
106 var dog = root.childNodes[0];
107 expect(dog.className).toBe('bigdog');
108 });
src/browser/ui/ReactDOMComponent.js
+1 -4
@@ -15,7 +15,6 @@
15 var CSSPropertyOperations = require('CSSPropertyOperations');
16 var DOMProperty = require('DOMProperty');
17 var DOMPropertyOperations = require('DOMPropertyOperations');
18 -var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
18 var ReactComponent = require('ReactComponent');
19 var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
20 var ReactMount = require('ReactMount');
@@ -148,7 +147,6 @@ function validateDangerousTag(tag) {
147 function ReactDOMComponent(tag) {
148 validateDangerousTag(tag);
149 this._tag = tag;
151 - this.tagName = tag.toUpperCase();
150 }
151
152 ReactDOMComponent.displayName = 'ReactDOMComponent';
@@ -488,8 +486,7 @@ assign(
486 ReactDOMComponent.prototype,
487 ReactComponent.Mixin,
488 ReactDOMComponent.Mixin,
491 - ReactMultiChild.Mixin,
492 - ReactBrowserComponentMixin
489 + ReactMultiChild.Mixin
490 );
491
492 module.exports = ReactDOMComponent;
src/browser/ui/ReactMount.js
+1 -7
@@ -135,13 +135,7 @@ function getNode(id) {
135 * @internal
136 */
137 function getNodeFromInstance(instance) {
138 - // This instance can currently be either a public or private instance since
139 - // native nodes are still public.
140 - var id = instance._rootNodeID;
141 - // TODO: Once these are only public instances, remove this conditional.
142 - if (id == null) {
143 - id = ReactInstanceMap.get(instance)._rootNodeID;
144 - }
138 + var id = ReactInstanceMap.get(instance)._rootNodeID;
139 if (ReactEmptyComponent.isNullComponentID(id)) {
140 return null;
141 }
src/browser/ui/dom/components/ReactDOMButton.js
+1
@@ -39,6 +39,7 @@ var mouseListenerNames = keyMirror({
39 */
40 var ReactDOMButton = ReactClass.createClass({
41 displayName: 'ReactDOMButton',
42 + tagName: 'BUTTON',
43
44 mixins: [AutoFocusMixin, ReactBrowserComponentMixin],
45
src/browser/ui/dom/components/ReactDOMForm.js
+1
@@ -27,6 +27,7 @@ var form = ReactElement.createFactory('form');
27 */
28 var ReactDOMForm = ReactClass.createClass({
29 displayName: 'ReactDOMForm',
30 + tagName: 'FORM',
31
32 mixins: [ReactBrowserComponentMixin, LocalEventTrapMixin],
33
src/browser/ui/dom/components/ReactDOMInput.js
+1
@@ -52,6 +52,7 @@ function forceUpdateIfMounted() {
52 */
53 var ReactDOMInput = ReactClass.createClass({
54 displayName: 'ReactDOMInput',
55 + tagName: 'INPUT',
56
57 mixins: [AutoFocusMixin, LinkedValueUtils.Mixin, ReactBrowserComponentMixin],
58
src/browser/ui/dom/components/ReactDOMOption.js
+1
@@ -24,6 +24,7 @@ var option = ReactElement.createFactory('option');
24 */
25 var ReactDOMOption = ReactClass.createClass({
26 displayName: 'ReactDOMOption',
27 + tagName: 'OPTION',
28
29 mixins: [ReactBrowserComponentMixin],
30
src/browser/ui/dom/components/ReactDOMSelect.js
+1
@@ -109,6 +109,7 @@ function updateOptions(component, propValue) {
109 */
110 var ReactDOMSelect = ReactClass.createClass({
111 displayName: 'ReactDOMSelect',
112 + tagName: 'SELECT',
113
114 mixins: [AutoFocusMixin, LinkedValueUtils.Mixin, ReactBrowserComponentMixin],
115
src/browser/ui/dom/components/ReactDOMTextarea.js
+1
@@ -50,6 +50,7 @@ function forceUpdateIfMounted() {
50 */
51 var ReactDOMTextarea = ReactClass.createClass({
52 displayName: 'ReactDOMTextarea',
53 + tagName: 'TEXTAREA',
54
55 mixins: [AutoFocusMixin, LinkedValueUtils.Mixin, ReactBrowserComponentMixin],
56
src/core/ReactComponent.js
+5 -1
@@ -453,7 +453,11 @@ var ReactComponent = {
453 * @internal
454 */
455 getPublicInstance: function() {
456 - return this;
456 + invariant(
457 + false,
458 + 'getPublicInstance should never be called on the base class. It must ' +
459 + 'be overriden.'
460 + );
461 }
462 }
463 };
src/core/ReactNativeComponent.js
+20 -6
@@ -11,6 +11,9 @@
11
12 "use strict";
13
14 +var ReactClass = require('ReactClass');
15 +var ReactElement = require('ReactElement');
16 +
17 var assign = require('Object.assign');
18 var invariant = require('invariant');
19
@@ -37,6 +40,22 @@ var ReactNativeComponentInjection = {
40 }
41 };
42
43 +function autoGenerateWrapperClass(type) {
44 + return ReactClass.createClass({
45 + tagName: type.toUpperCase(),
46 + render: function() {
47 + return new ReactElement(
48 + type,
49 + null,
50 + null,
51 + null,
52 + null,
53 + this.props
54 + );
55 + }
56 + });
57 +}
58 +
59 /**
60 * Create an internal class for a specific tag.
61 *
@@ -47,12 +66,7 @@ var ReactNativeComponentInjection = {
66 function createInstanceForTag(tag, props, parentType) {
67 var componentClass = tagToComponentClass[tag];
68 if (componentClass == null) {
50 - invariant(
51 - genericComponentClass,
52 - 'There is no registered component for the tag %s',
53 - tag
54 - );
55 - return new genericComponentClass(tag, props);
69 + tagToComponentClass[tag] = componentClass = autoGenerateWrapperClass(tag);
70 }
71 if (parentType === tag) {
72 // Avoid recursion
src/core/__tests__/ReactComponent-test.js
+3 -7
@@ -26,10 +26,6 @@ describe('ReactComponent', function() {
26 reactComponentExpect = require('reactComponentExpect');
27
28 getMountDepth = function(instance) {
29 - if (instance.mountComponent) {
30 - // Native instance
31 - return instance._mountDepth;
32 - }
29 return ReactInstanceMap.get(instance)._mountDepth;
30 };
31 });
@@ -306,9 +302,9 @@ describe('ReactComponent', function() {
302 expect(getMountDepth(root)).toBe(0);
303 expect(getMountDepth(root.refs.switcher)).toBe(1);
304 expect(getMountDepth(root.refs.switcher.refs.box)).toBe(2);
309 - expect(getMountDepth(root.refs.switcher.refs.switcherDiv)).toBe(4);
310 - expect(getMountDepth(root.refs.child)).toBe(5);
305 + expect(getMountDepth(root.refs.switcher.refs.switcherDiv)).toBe(5);
306 + expect(getMountDepth(root.refs.child)).toBe(7);
307 expect(getMountDepth(root.refs.switcher.refs.box.refs.boxDiv)).toBe(3);
312 - expect(getMountDepth(root.refs.child.refs.span)).toBe(6);
308 + expect(getMountDepth(root.refs.child.refs.span)).toBe(8);
309 });
310 });
src/core/__tests__/ReactCompositeComponent-test.js
+9 -7
@@ -66,8 +66,8 @@ describe('ReactCompositeComponent', function() {
66 * reallocated again.
67 */
68 ChildUpdates = React.createClass({
69 - getAnchorID: function() {
70 - return this.refs.anch._rootNodeID;
69 + getAnchor: function() {
70 + return this.refs.anch;
71 },
72 render: function() {
73 var className = cx({'anchorClass': this.props.anchorClassOn});
@@ -182,8 +182,8 @@ describe('ReactCompositeComponent', function() {
182 instance.setProps({renderAnchor: false}); // Clear out the anchor
183 // rerender
184 instance.setProps({renderAnchor: true, anchorClassOn: false});
185 - var anchorID = instance.getAnchorID();
186 - var actualDOMAnchorNode = ReactMount.getNode(anchorID);
185 + var anchor = instance.getAnchor();
186 + var actualDOMAnchorNode = anchor.getDOMNode();
187 expect(actualDOMAnchorNode.className).toBe('');
188 });
189
@@ -779,9 +779,11 @@ describe('ReactCompositeComponent', function() {
779 React.unmountComponentAtNode(container);
780 expect(innerUnmounted).toBe(true);
781
782 - // <Component />, <Inner />, and both <div /> elements each call
783 - // unmountIDFromEnvironment which calls purgeID, for a total of 4.
784 - expect(ReactMount.purgeID.callCount).toBe(4);
782 + // <Component />, <Inner />, and both <div /> elements and their wrappers
783 + // each call unmountIDFromEnvironment which calls purgeID, for a total of 6.
784 + // TODO: Test the effect of this. E.g. does the node cache get repopulated
785 + // after a getDOMNode call?
786 + expect(ReactMount.purgeID.callCount).toBe(6);
787 });
788
789 it('should warn when shouldComponentUpdate() returns undefined', function() {
src/core/__tests__/ReactCompositeComponentError-test.js
+2 -2
@@ -26,8 +26,8 @@ describe('ReactCompositeComponent-error', function() {
26 }
27 });
28
29 - var instance = <Component />;
30 - instance = ReactTestUtils.renderIntoDocument(instance);
29 + new Component();
30 +
31 expect(ReactErrorUtils.guard.mostRecentCall.args[1])
32 .toEqual('Component.someHandler');
33 });
src/core/__tests__/ReactInstanceHandles-test.js
+2 -2
@@ -12,6 +12,7 @@
12 "use strict";
13
14 var React = require('React');
15 +var ReactInstanceMap = require('ReactInstanceMap');
16 var ReactTestUtils = require('ReactTestUtils');
17 var ReactMount = require('ReactMount');
18
@@ -66,8 +67,7 @@ describe('ReactInstanceHandles', function() {
67 if (instance === null) {
68 return '';
69 }
69 - var internal = ReactTestUtils.getInternalRepresentation(instance);
70 - return internal._rootNodeID;
70 + return ReactInstanceMap.get(instance)._rootNodeID;
71 }
72
73 beforeEach(function() {
src/core/__tests__/ReactMultiChildReconcile-test.js
+4 -1
@@ -87,7 +87,10 @@ var FriendsStatusDisplay = React.createClass({
87 // TODO: Update this to a better test that doesn't rely so much on internal
88 // implementation details.
89 var statusDisplays =
90 - ReactInstanceMap.get(this)._renderedComponent._renderedChildren;
90 + ReactInstanceMap.get(this)
91 + ._renderedComponent
92 + ._renderedComponent
93 + ._renderedChildren;
94 for (name in statusDisplays) {
95 var child = statusDisplays[name];
96 var isPresent = !!child;
src/test/ReactTestUtils.js
+4 -9
@@ -99,14 +99,6 @@ var ReactTestUtils = {
99 (inst.constructor === type));
100 },
101
102 - getInternalRepresentation: function(inst) {
103 - // TODO: Remove this duck check once we have a separate DOM/Native instance
104 - if (typeof inst.mountComponent === 'function') {
105 - return inst;
106 - }
107 - return ReactInstanceMap.get(inst);
108 - },
109 -
102 getRenderedChildOfCompositeComponent: function(inst) {
103 if (!ReactTestUtils.isCompositeComponent(inst)) {
104 return null;
@@ -121,7 +113,10 @@ var ReactTestUtils = {
113 }
114 var ret = test(inst) ? [inst] : [];
115 if (ReactTestUtils.isDOMComponent(inst)) {
124 - var renderedChildren = inst._renderedChildren;
116 + var internalInstance = ReactInstanceMap.get(inst);
117 + var renderedChildren = internalInstance
118 + ._renderedComponent
119 + ._renderedChildren;
120 var key;
121 for (key in renderedChildren) {
122 if (!renderedChildren.hasOwnProperty(key)) {
src/test/reactComponentExpect.js
+20 -11
@@ -12,6 +12,7 @@
12
13 "use strict";
14
15 +var ReactInstanceMap = require('ReactInstanceMap');
16 var ReactTestUtils = require('ReactTestUtils');
17
18 var assign = require('Object.assign');
@@ -27,14 +28,20 @@ function reactComponentExpect(instance) {
28
29 expect(instance).not.toBeNull();
30
30 - this._instance = ReactTestUtils.getInternalRepresentation(instance);
31 + var internalInstance = ReactInstanceMap.get(instance);
32
32 - expect(typeof instance).toBe('object');
33 - expect(typeof instance.constructor).toBe('function');
34 - expect(ReactTestUtils.isElement(instance)).toBe(false);
33 + expect(typeof internalInstance).toBe('object');
34 + expect(typeof internalInstance.constructor).toBe('function');
35 + expect(ReactTestUtils.isElement(internalInstance)).toBe(false);
36 +
37 + return new reactComponentExpectInternal(internalInstance);
38 +}
39 +
40 +function reactComponentExpectInternal(internalInstance) {
41 + this._instance = internalInstance;
42 }
43
37 -assign(reactComponentExpect.prototype, {
44 +assign(reactComponentExpectInternal.prototype, {
45 // Getters -------------------------------------------------------------------
46
47 /**
@@ -62,7 +69,7 @@ assign(reactComponentExpect.prototype, {
69 this.toBeCompositeComponent();
70 var child = this._instance._renderedComponent;
71 // TODO: Hide ReactEmptyComponent instances here?
65 - return new reactComponentExpect(child);
72 + return new reactComponentExpectInternal(child);
73 },
74
75 /**
@@ -72,14 +79,15 @@ assign(reactComponentExpect.prototype, {
79 // Currently only dom components have arrays of children, but that will
80 // change soon.
81 this.toBeDOMComponent();
75 - var renderedChildren = this._instance._renderedChildren || {};
82 + var renderedChildren =
83 + this._instance._renderedComponent._renderedChildren || {};
84 for (var name in renderedChildren) {
85 if (!renderedChildren.hasOwnProperty(name)) {
86 continue;
87 }
88 if (renderedChildren[name]) {
89 if (renderedChildren[name]._mountIndex === childIndex) {
82 - return new reactComponentExpect(renderedChildren[name]);
90 + return new reactComponentExpectInternal(renderedChildren[name]);
91 }
92 }
93 }
@@ -88,15 +96,16 @@ assign(reactComponentExpect.prototype, {
96
97 toBeDOMComponentWithChildCount: function(n) {
98 this.toBeDOMComponent();
91 - expect(this._instance._renderedChildren).toBeTruthy();
92 - var len = Object.keys(this._instance._renderedChildren).length;
99 + expect(this._instance._renderedComponent._renderedChildren).toBeTruthy();
100 + var len = Object.keys(this._instance._renderedComponent._renderedChildren)
101 + .length;
102 expect(len).toBe(n);
103 return this;
104 },
105
106 toBeDOMComponentWithNoChildren: function() {
107 this.toBeDOMComponent();
99 - expect(this._instance._renderedChildren).toBeFalsy();
108 + expect(this._instance._renderedComponent._renderedChildren).toBeFalsy();
109 return this;
110 },
111