@samitouri / QOS-React-2 / commits / 230115da92

Make ReactTextComponent properly injectable

ReactTextComponent's implementation is DOM-specific; instead of flattenChildren creating the ReactTextComponent instances, ReactNativeComponent now takes care of having ReactTextComponent injected and creating the component instance. I also renamed ReactTextComponent to ReactDOMTextComponent and moved it to browser/ui/ where it belongs. ReactDOMTextComponent no longer inherits directly from ReactComponent and instead implements construct and {mount,receive,unmount}Component directly. This diff removes `ReactTestUtils.isTextComponent` which should have previously never returned true when using public APIs. Test Plan: jest, use ballmer-peak example.

Ben Alpert committed Oct 19, 2014 at 15:51 UTC 230115da9210f389e60f7eb0e402de1a06b5f269

14 files changed +140 -113

docs/docs/10.4-test-utils.md

-8

@@ -75,14 +75,6 @@ boolean isCompositeComponentWithType(ReactComponent instance, function component

76 Returns true if `instance` is a composite component (created with `React.createClass()`) whose type is of a React `componentClass`.

78 -### isTextComponent

79 -

80 -```javascript

81 -boolean isTextComponent(ReactComponent instance)

82 -```

83 -

84 -Returns true if `instance` is a plain text component.

85 -

78 ### findAllInRenderedTree

80 ```javascript

src/browser/ui/React.js

+2 -2

@@ -22,6 +22,7 @@ var ReactElement = require('ReactElement');

22 var ReactElementValidator = require('ReactElementValidator');

23 var ReactDOM = require('ReactDOM');

24 var ReactDOMComponent = require('ReactDOMComponent');

25 +var ReactDOMTextComponent = require('ReactDOMTextComponent');

26 var ReactDefaultInjection = require('ReactDefaultInjection');

27 var ReactInstanceHandles = require('ReactInstanceHandles');

28 var ReactMount = require('ReactMount');

@@ -30,7 +31,6 @@ var ReactPerf = require('ReactPerf');

31 var ReactPropTypes = require('ReactPropTypes');

32 var ReactRef = require('ReactRef');

33 var ReactServerRendering = require('ReactServerRendering');

33 -var ReactTextComponent = require('ReactTextComponent');

35 var assign = require('Object.assign');

36 var deprecated = require('deprecated');

@@ -122,7 +122,7 @@ if (

122 InstanceHandles: ReactInstanceHandles,

123 Mount: ReactMount,

124 MultiChild: ReactMultiChild,

125 - TextComponent: ReactTextComponent

125 + TextComponent: ReactDOMTextComponent

126 });

127 }

128

src/browser/ui/ReactDOMTextComponent.js renamed

+41 -35

@@ -6,7 +6,7 @@

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 ReactTextComponent

9 + * @providesModule ReactDOMTextComponent

10 * @typechecks static-only

11 */

@@ -14,7 +14,8 @@

15 var DOMPropertyOperations = require('DOMPropertyOperations');

16 var ReactComponent = require('ReactComponent');

17 -var ReactElement = require('ReactElement');

17 +var ReactComponentBrowserEnvironment =

18 + require('ReactComponentBrowserEnvironment');

20 var assign = require('Object.assign');

21 var escapeTextForBrowser = require('escapeTextForBrowser');

@@ -31,15 +32,25 @@ var invariant = require('invariant');

32 *

33 * TODO: Investigate representing React components in the DOM with text nodes.

34 *

34 - * @class ReactTextComponent

35 + * @class ReactDOMTextComponent

36 * @extends ReactComponent

37 * @internal

38 */

38 -var ReactTextComponent = function(props) {

39 - // This constructor and it's argument is currently used by mocks.

39 +var ReactDOMTextComponent = function(props) {

40 + // This constructor and its argument is currently used by mocks.

41 };

42 -assign(ReactTextComponent.prototype, ReactComponent.Mixin, {

43 +assign(ReactDOMTextComponent.prototype, {

44 +

45 + /**

46 + * @param {ReactText} node

47 + * @internal

48 + */

49 + construct: function(text) {

50 + // TODO: This is really a ReactText (ReactNode), not a ReactElement

51 + this._currentElement = text;

52 + this._stringText = '' + text;

53 + },

55 /**

56 * Creates the markup for this text node. This node is not intended to have

@@ -52,14 +63,8 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, {

63 * @internal

64 */

65 mountComponent: function(rootID, transaction, mountDepth) {

55 - ReactComponent.Mixin.mountComponent.call(

56 - this,

57 - rootID,

58 - transaction,

59 - mountDepth

60 - );

61 -

62 - var escapedText = escapeTextForBrowser(this.props);

66 + this._rootNodeID = rootID;

67 + var escapedText = escapeTextForBrowser(this._stringText);

69 if (transaction.renderToStaticMarkup) {

70 // Normally we'd wrap this in a `span` for the reasons stated above, but

@@ -78,38 +83,39 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, {

83 /**

84 * Updates this component by updating the text content.

85 *

81 - * @param {object} nextComponent Contains the next text content.

86 + * @param {ReactText} nextText The next text content

87 * @param {ReactReconcileTransaction} transaction

88 * @internal

89 */

85 - receiveComponent: function(nextComponent, transaction) {

86 - var nextProps = nextComponent.props;

87 - if (nextProps !== this.props) {

88 - // TODO: Save this as pending props and use performUpdateIfNecessary

89 - // and/or updateComponent to do the actual update for consistency with

90 - // other component types?

91 - this.props = nextProps;

92 - ReactComponent.BackendIDOperations.updateTextContentByID(

93 - this._rootNodeID,

94 - nextProps

95 - );

90 + receiveComponent: function(nextText, transaction) {

91 + if (nextText !== this._currentElement) {

92 + this._currentElement = nextText;

93 + var nextStringText = '' + nextText;

94 + if (nextStringText !== this._stringText) {

95 + // TODO: Save this as pending props and use performUpdateIfNecessary

96 + // and/or updateComponent to do the actual update for consistency with

97 + // other component types?

98 + this._stringText = nextStringText;

99 + ReactComponent.BackendIDOperations.updateTextContentByID(

100 + this._rootNodeID,

101 + nextStringText

102 + );

103 + }

104 }

105 },

106

107 updateComponent: function() {

108 invariant(

109 false,

102 - 'ReactTextComponent: updateComponent() should never be called'

110 + 'ReactDOMTextComponent: updateComponent() should never be called'

111 );

112 + },

113 +

114 + unmountComponent: function() {

115 + // TODO: Is this necessary?

116 + ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);

117 }

118

119 });

120

108 -var ReactTextComponentFactory = function(text) {

109 - // Bypass validation and configuration

110 - return new ReactElement(ReactTextComponent, null, null, null, null, text);

111 -};

112 -

113 -ReactTextComponentFactory.type = ReactTextComponent;

114 -

115 -module.exports = ReactTextComponentFactory;

121 +module.exports = ReactDOMTextComponent;

src/browser/ui/ReactDefaultInjection.js

@@ -32,6 +32,7 @@ var ReactDOMInput = require('ReactDOMInput');

32 var ReactDOMOption = require('ReactDOMOption');

33 var ReactDOMSelect = require('ReactDOMSelect');

34 var ReactDOMTextarea = require('ReactDOMTextarea');

35 +var ReactDOMTextComponent = require('ReactDOMTextComponent');

36 var ReactEventListener = require('ReactEventListener');

37 var ReactInjection = require('ReactInjection');

38 var ReactInstanceHandles = require('ReactInstanceHandles');

@@ -73,6 +74,10 @@ function inject() {

74 ReactDOMComponent

75 );

77 + ReactInjection.NativeComponent.injectTextComponentClass(

78 + ReactDOMTextComponent

79 + );

80 +

81 ReactInjection.NativeComponent.injectComponentClasses({

82 'button': ReactDOMButton,

83 'form': ReactDOMForm,

src/browser/ui/__tests__/ReactDOMTextComponent-test.js renamed

+1 -1

@@ -13,7 +13,7 @@

14 var React;

16 -describe('ReactTextComponent', function() {

16 +describe('ReactDOMTextComponent', function() {

17 beforeEach(function() {

18 React = require('React');

19 });

src/core/ReactMultiChild.js

+1 -1

@@ -306,7 +306,7 @@ var ReactMultiChild = {

306 // Remove children that are no longer present.

307 for (name in prevChildren) {

308 if (prevChildren.hasOwnProperty(name) &&

309 - !(nextChildren && nextChildren[name])) {

309 + !(nextChildren && nextChildren.hasOwnProperty(name))) {

310 this._unmountChildByName(prevChildren[name], name);

311 }

312 }

src/core/ReactNativeComponent.js

+24

@@ -17,6 +17,7 @@ var invariant = require('invariant');

17 var genericComponentClass = null;

18 // This registry keeps track of wrapper classes around native tags

19 var tagToComponentClass = {};

20 +var textComponentClass = null;

22 var ReactNativeComponentInjection = {

23 // This accepts a class that receives the tag string. This is a catch all

@@ -24,6 +25,11 @@ var ReactNativeComponentInjection = {

25 injectGenericComponentClass: function(componentClass) {

26 genericComponentClass = componentClass;

27 },

28 + // This accepts a text component class that takes the text string to be

29 + // rendered as props.

30 + injectTextComponentClass: function(componentClass) {

31 + textComponentClass = componentClass;

32 + },

33 // This accepts a keyed object with classes as values. Each key represents a

34 // tag. That particular tag will use this class instead of the generic one.

35 injectComponentClasses: function(componentClasses) {

@@ -60,8 +66,26 @@ function createInstanceForTag(tag, props, parentType) {

66 return new componentClass(props);

67 }

69 +/**

70 + * @param {ReactText} text

71 + * @return {ReactComponent}

72 + */

73 +function createInstanceForText(text) {

74 + return new textComponentClass(text);

75 +}

76 +

77 +/**

78 + * @param {ReactComponent} component

79 + * @return {boolean}

80 + */

81 +function isTextComponent(component) {

82 + return component instanceof textComponentClass;

83 +}

84 +

85 var ReactNativeComponent = {

86 createInstanceForTag: createInstanceForTag,

87 + createInstanceForText: createInstanceForText,

88 + isTextComponent: isTextComponent,

89 injection: ReactNativeComponentInjection

90 };

src/core/__tests__/ReactMultiChildText-test.js

+1 -2

@@ -70,8 +70,7 @@ var expectChildren = function(d, children) {

70 if (typeof child === 'string') {

71 reactComponentExpect(d)

72 .expectRenderedChildAt(i)

73 - .toBeTextComponent()

74 - .instance();

73 + .toBeTextComponent();

75 textNode = d.getDOMNode().childNodes[i].firstChild;

src/core/instantiateReactComponent.js

+45 -33

@@ -16,6 +16,7 @@ var ReactCompositeComponent = require('ReactCompositeComponent');

16 var ReactNativeComponent = require('ReactNativeComponent');

18 var assign = require('Object.assign');

19 +var invariant = require('invariant');

20 var warning = require('warning');

22 // To avoid a cyclic dependency, we create the final class in this module

@@ -46,60 +47,71 @@ function isInternalComponentType(type) {

47 }

49 /**

49 - * Given an `element` create an instance that will actually be mounted.

50 + * Given a ReactNode, create an instance that will actually be mounted.

51 *

51 - * @param {object} element

52 + * @param {ReactNode} node

53 * @param {*} parentCompositeType The composite type that resolved this.

54 * @return {object} A new instance of the element's constructor.

55 * @protected

56 */

56 -function instantiateReactComponent(element, parentCompositeType) {

57 +function instantiateReactComponent(node, parentCompositeType) {

58 var instance;

59 - if (__DEV__) {

60 - warning(

61 - element && (typeof element.type === 'function' ||

62 - typeof element.type === 'string'),

63 - 'Only functions or strings can be mounted as React components.'

64 - );

65 - }

60 + if (typeof node === 'object') {

61 + var element = node;

62 + if (__DEV__) {

63 + warning(

64 + element && (typeof element.type === 'function' ||

65 + typeof element.type === 'string'),

66 + 'Only functions or strings can be mounted as React components.'

67 + );

68 + }

67 - // Special case string values

68 - if (typeof element.type === 'string') {

69 - instance = ReactNativeComponent.createInstanceForTag(

70 - element.type,

71 - element.props,

72 - parentCompositeType

73 - );

74 - // If the injected special class is not an internal class, but another

75 - // composite, then we must wrap it.

76 - // TODO: Move this resolution around to something cleaner.

77 - if (typeof instance.mountComponent !== 'function') {

78 - instance = new ReactCompositeComponentWrapper(instance);

70 + // Special case string values

71 + if (typeof element.type === 'string') {

72 + instance = ReactNativeComponent.createInstanceForTag(

73 + element.type,

74 + element.props,

75 + parentCompositeType

76 + );

77 + // If the injected special class is not an internal class, but another

78 + // composite, then we must wrap it.

79 + // TODO: Move this resolution around to something cleaner.

80 + if (typeof instance.mountComponent !== 'function') {

81 + instance = new ReactCompositeComponentWrapper(instance);

82 + }

83 + } else if (isInternalComponentType(element.type)) {

84 + // This is temporarily available for custom components that are not string

85 + // represenations. I.e. ART. Once those are updated to use the string

86 + // representation, we can drop this code path.

87 + instance = new element.type(element);

88 + } else {

89 + // TODO: Update to follow new ES6 initialization. Ideally, we can use

90 + // props in property initializers.

91 + var inst = new element.type(element.props);

92 + instance = new ReactCompositeComponentWrapper(inst);

93 }

80 - } else if (isInternalComponentType(element.type)) {

81 - // This is temporarily available for custom components that are not string

82 - // represenations. I.e. ART. Once those are updated to use the string

83 - // representation, we can drop this code path.

84 - instance = new element.type(element);

94 + } else if (typeof node === 'string' || typeof node === 'number') {

95 + instance = ReactNativeComponent.createInstanceForText(node);

96 } else {

86 - // TODO: Update to follow new ES6 initialization. Ideally, we can use props

87 - // in property initializers.

88 - var inst = new element.type(element.props);

89 - instance = new ReactCompositeComponentWrapper(inst);

97 + invariant(

98 + false,

99 + 'Encountered invalid React node of type ' + typeof node

100 + );

101 }

102

103 if (__DEV__) {

104 warning(

105 typeof instance.construct === 'function' &&

106 typeof instance.mountComponent === 'function' &&

96 - typeof instance.receiveComponent === 'function',

107 + typeof instance.receiveComponent === 'function' &&

108 + typeof instance.unmountComponent === 'function',

109 'Only React Components can be mounted.'

110 );

111 }

112

113 // Sets up the instance. This can probably just move into the constructor now.

102 - instance.construct(element);

114 + instance.construct(node);

115

116 return instance;

117 }

src/core/shouldUpdateReactComponent.js

+15 -5

@@ -12,6 +12,8 @@

13 "use strict";

15 +var ReactNativeComponent = require('ReactNativeComponent');

16 +

17 /**

18 * Given a `prevElement` and `nextElement`, determines if the existing

19 * instance should be updated as opposed to being destroyed or replaced by a new

@@ -24,11 +26,19 @@

26 * @protected

27 */

28 function shouldUpdateReactComponent(prevElement, nextElement) {

27 - if (prevElement && nextElement &&

28 - prevElement.type === nextElement.type &&

29 - prevElement.key === nextElement.key &&

30 - prevElement._owner === nextElement._owner) {

31 - return true;

29 + if (prevElement != null && nextElement != null) {

30 + var prevType = typeof prevElement;

31 + var nextType = typeof nextElement;

32 + if (prevType === 'string' || prevType === 'number') {

33 + return (nextType === 'string' || nextType === 'number');

34 + } else {

35 + return (

36 + nextType === 'object' &&

37 + prevElement.type === nextElement.type &&

38 + prevElement.key === nextElement.key &&

39 + prevElement._owner === nextElement._owner

40 + );

41 + }

42 }

43 return false;

44 }

src/test/ReactTestUtils.js

-5

@@ -19,7 +19,6 @@ var ReactElement = require('ReactElement');

19 var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');

20 var ReactInstanceMap = require('ReactInstanceMap');

21 var ReactMount = require('ReactMount');

22 -var ReactTextComponent = require('ReactTextComponent');

22 var ReactUpdates = require('ReactUpdates');

23 var SyntheticEvent = require('SyntheticEvent');

@@ -100,10 +99,6 @@ var ReactTestUtils = {

99 (inst.constructor === type));

100 },

101

103 - isTextComponent: function(inst) {

104 - return inst instanceof ReactTextComponent.type;

105 - },

106 -

102 getInternalRepresentation: function(inst) {

103 // TODO: Remove this duck check once we have a separate DOM/Native instance

104 if (typeof inst.mountComponent === 'function') {

src/test/reactComponentExpect.js

+2 -2

@@ -12,7 +12,6 @@

13 "use strict";

15 -var ReactInstanceMap = require('ReactInstanceMap');

15 var ReactTestUtils = require('ReactTestUtils');

17 var assign = require('Object.assign');

@@ -131,7 +130,8 @@ assign(reactComponentExpect.prototype, {

130 },

131

132 toBeTextComponent: function() {

134 - expect(ReactTestUtils.isTextComponent(this.instance())).toBe(true);

133 + var elementType = typeof this._instance._currentElement;

134 + expect(elementType === 'string' || elementType === 'number').toBe(true);

135 return this;

136 },

137

src/utils/flattenChildren.js

+1 -14

@@ -11,8 +11,6 @@

12 "use strict";

14 -var ReactTextComponent = require('ReactTextComponent');

15 -

14 var traverseAllChildren = require('traverseAllChildren');

15 var warning = require('warning');

@@ -33,18 +31,7 @@ function flattenSingleChildIntoContext(traverseContext, child, name) {

31 name

32 );

33 if (keyUnique && child != null) {

36 - var type = typeof child;

37 - var normalizedValue;

38 -

39 - if (type === 'string') {

40 - normalizedValue = ReactTextComponent(child);

41 - } else if (type === 'number') {

42 - normalizedValue = ReactTextComponent('' + child);

43 - } else {

44 - normalizedValue = child;

45 - }

46 -

47 - result[name] = normalizedValue;

34 + result[name] = child;

35 }

36 }

src/utils/traverseAllChildren.js

+2 -5

@@ -21,11 +21,8 @@ var SEPARATOR = ReactInstanceHandles.SEPARATOR;

21 var SUBSEPARATOR = ':';

23 /**

24 - * TODO: Test that:

25 - * 1. `mapChildren` transforms strings and numbers into `ReactTextComponent`.

26 - * 2. it('should fail when supplied duplicate key', function() {

27 - * 3. That a single child and an array with one item have the same key pattern.

28 - * });

24 + * TODO: Test that a single child and an array with one item have the same key

25 + * pattern.

26 */

28 var userProvidedKeyEscaperLookup = {