Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ReactTextComponent properly injectable #2382

Merged
merged 1 commit into from
Nov 15, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 0 additions & 8 deletions docs/docs/10.4-test-utils.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,6 @@ boolean isCompositeComponentWithType(ReactComponent instance, function component

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

### isTextComponent

```javascript
boolean isTextComponent(ReactComponent instance)
```

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

### findAllInRenderedTree

```javascript
Expand Down
4 changes: 2 additions & 2 deletions src/browser/ui/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var ReactElement = require('ReactElement');
var ReactElementValidator = require('ReactElementValidator');
var ReactDOM = require('ReactDOM');
var ReactDOMComponent = require('ReactDOMComponent');
var ReactDOMTextComponent = require('ReactDOMTextComponent');
var ReactDefaultInjection = require('ReactDefaultInjection');
var ReactInstanceHandles = require('ReactInstanceHandles');
var ReactMount = require('ReactMount');
Expand All @@ -30,7 +31,6 @@ var ReactPerf = require('ReactPerf');
var ReactPropTypes = require('ReactPropTypes');
var ReactRef = require('ReactRef');
var ReactServerRendering = require('ReactServerRendering');
var ReactTextComponent = require('ReactTextComponent');

var assign = require('Object.assign');
var deprecated = require('deprecated');
Expand Down Expand Up @@ -122,7 +122,7 @@ if (
InstanceHandles: ReactInstanceHandles,
Mount: ReactMount,
MultiChild: ReactMultiChild,
TextComponent: ReactTextComponent
TextComponent: ReactDOMTextComponent
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactTextComponent
* @providesModule ReactDOMTextComponent
* @typechecks static-only
*/

"use strict";

var DOMPropertyOperations = require('DOMPropertyOperations');
var ReactComponent = require('ReactComponent');
var ReactElement = require('ReactElement');
var ReactComponentBrowserEnvironment =
require('ReactComponentBrowserEnvironment');

var assign = require('Object.assign');
var escapeTextForBrowser = require('escapeTextForBrowser');
Expand All @@ -31,15 +32,25 @@ var invariant = require('invariant');
*
* TODO: Investigate representing React components in the DOM with text nodes.
*
* @class ReactTextComponent
* @class ReactDOMTextComponent
* @extends ReactComponent
* @internal
*/
var ReactTextComponent = function(props) {
// This constructor and it's argument is currently used by mocks.
var ReactDOMTextComponent = function(props) {
// This constructor and its argument is currently used by mocks.
};

assign(ReactTextComponent.prototype, ReactComponent.Mixin, {
assign(ReactDOMTextComponent.prototype, {

/**
* @param {ReactText} node
* @internal
*/
construct: function(text) {
// TODO: This is really a ReactText (ReactNode), not a ReactElement
this._currentElement = text;
this._stringText = '' + text;
},

/**
* Creates the markup for this text node. This node is not intended to have
Expand All @@ -52,14 +63,8 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, {
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth) {
ReactComponent.Mixin.mountComponent.call(
this,
rootID,
transaction,
mountDepth
);

var escapedText = escapeTextForBrowser(this.props);
this._rootNodeID = rootID;
var escapedText = escapeTextForBrowser(this._stringText);

if (transaction.renderToStaticMarkup) {
// Normally we'd wrap this in a `span` for the reasons stated above, but
Expand All @@ -78,38 +83,39 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, {
/**
* Updates this component by updating the text content.
*
* @param {object} nextComponent Contains the next text content.
* @param {ReactText} nextText The next text content
* @param {ReactReconcileTransaction} transaction
* @internal
*/
receiveComponent: function(nextComponent, transaction) {
var nextProps = nextComponent.props;
if (nextProps !== this.props) {
// TODO: Save this as pending props and use performUpdateIfNecessary
// and/or updateComponent to do the actual update for consistency with
// other component types?
this.props = nextProps;
ReactComponent.BackendIDOperations.updateTextContentByID(
this._rootNodeID,
nextProps
);
receiveComponent: function(nextText, transaction) {
if (nextText !== this._currentElement) {
this._currentElement = nextText;
var nextStringText = '' + nextText;
if (nextStringText !== this._stringText) {
// TODO: Save this as pending props and use performUpdateIfNecessary
// and/or updateComponent to do the actual update for consistency with
// other component types?
this._stringText = nextStringText;
ReactComponent.BackendIDOperations.updateTextContentByID(
this._rootNodeID,
nextStringText
);
}
}
},

updateComponent: function() {
invariant(
false,
'ReactTextComponent: updateComponent() should never be called'
'ReactDOMTextComponent: updateComponent() should never be called'
);
},

unmountComponent: function() {
// TODO: Is this necessary?
ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);
}

});

var ReactTextComponentFactory = function(text) {
// Bypass validation and configuration
return new ReactElement(ReactTextComponent, null, null, null, null, text);
};

ReactTextComponentFactory.type = ReactTextComponent;

module.exports = ReactTextComponentFactory;
module.exports = ReactDOMTextComponent;
5 changes: 5 additions & 0 deletions src/browser/ui/ReactDefaultInjection.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var ReactDOMInput = require('ReactDOMInput');
var ReactDOMOption = require('ReactDOMOption');
var ReactDOMSelect = require('ReactDOMSelect');
var ReactDOMTextarea = require('ReactDOMTextarea');
var ReactDOMTextComponent = require('ReactDOMTextComponent');
var ReactEventListener = require('ReactEventListener');
var ReactInjection = require('ReactInjection');
var ReactInstanceHandles = require('ReactInstanceHandles');
Expand Down Expand Up @@ -73,6 +74,10 @@ function inject() {
ReactDOMComponent
);

ReactInjection.NativeComponent.injectTextComponentClass(
ReactDOMTextComponent
);

ReactInjection.NativeComponent.injectComponentClasses({
'button': ReactDOMButton,
'form': ReactDOMForm,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

var React;

describe('ReactTextComponent', function() {
describe('ReactDOMTextComponent', function() {
beforeEach(function() {
React = require('React');
});
Expand Down
2 changes: 1 addition & 1 deletion src/core/ReactMultiChild.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ var ReactMultiChild = {
// Remove children that are no longer present.
for (name in prevChildren) {
if (prevChildren.hasOwnProperty(name) &&
!(nextChildren && nextChildren[name])) {
!(nextChildren && nextChildren.hasOwnProperty(name))) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated. Separate PR maybe? Helps us track potentially breaking changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No – previously, nextChildren would have ReactTextComponent('') and now it has just ''. Null children are already stripped out from flattenChildren.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

this._unmountChildByName(prevChildren[name], name);
}
}
Expand Down
24 changes: 24 additions & 0 deletions src/core/ReactNativeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ var invariant = require('invariant');
var genericComponentClass = null;
// This registry keeps track of wrapper classes around native tags
var tagToComponentClass = {};
var textComponentClass = null;

var ReactNativeComponentInjection = {
// This accepts a class that receives the tag string. This is a catch all
// that can render any kind of tag.
injectGenericComponentClass: function(componentClass) {
genericComponentClass = componentClass;
},
// This accepts a text component class that takes the text string to be
// rendered as props.
injectTextComponentClass: function(componentClass) {
textComponentClass = componentClass;
},
// This accepts a keyed object with classes as values. Each key represents a
// tag. That particular tag will use this class instead of the generic one.
injectComponentClasses: function(componentClasses) {
Expand Down Expand Up @@ -60,8 +66,26 @@ function createInstanceForTag(tag, props, parentType) {
return new componentClass(props);
}

/**
* @param {ReactText} text
* @return {ReactComponent}
*/
function createInstanceForText(text) {
return new textComponentClass(text);
}

/**
* @param {ReactComponent} component
* @return {boolean}
*/
function isTextComponent(component) {
return component instanceof textComponentClass;
}

var ReactNativeComponent = {
createInstanceForTag: createInstanceForTag,
createInstanceForText: createInstanceForText,
isTextComponent: isTextComponent,
injection: ReactNativeComponentInjection
};

Expand Down
3 changes: 1 addition & 2 deletions src/core/__tests__/ReactMultiChildText-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ var expectChildren = function(d, children) {
if (typeof child === 'string') {
reactComponentExpect(d)
.expectRenderedChildAt(i)
.toBeTextComponent()
.instance();
.toBeTextComponent();

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

Expand Down
78 changes: 45 additions & 33 deletions src/core/instantiateReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ReactCompositeComponent = require('ReactCompositeComponent');
var ReactNativeComponent = require('ReactNativeComponent');

var assign = require('Object.assign');
var invariant = require('invariant');
var warning = require('warning');

// To avoid a cyclic dependency, we create the final class in this module
Expand Down Expand Up @@ -46,60 +47,71 @@ function isInternalComponentType(type) {
}

/**
* Given an `element` create an instance that will actually be mounted.
* Given a ReactNode, create an instance that will actually be mounted.
*
* @param {object} element
* @param {ReactNode} node
* @param {*} parentCompositeType The composite type that resolved this.
* @return {object} A new instance of the element's constructor.
* @protected
*/
function instantiateReactComponent(element, parentCompositeType) {
function instantiateReactComponent(node, parentCompositeType) {
var instance;

if (__DEV__) {
warning(
element && (typeof element.type === 'function' ||
typeof element.type === 'string'),
'Only functions or strings can be mounted as React components.'
);
}
if (typeof node === 'object') {
var element = node;
if (__DEV__) {
warning(
element && (typeof element.type === 'function' ||
typeof element.type === 'string'),
'Only functions or strings can be mounted as React components.'
);
}

// Special case string values
if (typeof element.type === 'string') {
instance = ReactNativeComponent.createInstanceForTag(
element.type,
element.props,
parentCompositeType
);
// If the injected special class is not an internal class, but another
// composite, then we must wrap it.
// TODO: Move this resolution around to something cleaner.
if (typeof instance.mountComponent !== 'function') {
instance = new ReactCompositeComponentWrapper(instance);
// Special case string values
if (typeof element.type === 'string') {
instance = ReactNativeComponent.createInstanceForTag(
element.type,
element.props,
parentCompositeType
);
// If the injected special class is not an internal class, but another
// composite, then we must wrap it.
// TODO: Move this resolution around to something cleaner.
if (typeof instance.mountComponent !== 'function') {
instance = new ReactCompositeComponentWrapper(instance);
}
} else if (isInternalComponentType(element.type)) {
// This is temporarily available for custom components that are not string
// represenations. I.e. ART. Once those are updated to use the string
// representation, we can drop this code path.
instance = new element.type(element);
} else {
// TODO: Update to follow new ES6 initialization. Ideally, we can use
// props in property initializers.
var inst = new element.type(element.props);
instance = new ReactCompositeComponentWrapper(inst);
}
} else if (isInternalComponentType(element.type)) {
// This is temporarily available for custom components that are not string
// represenations. I.e. ART. Once those are updated to use the string
// representation, we can drop this code path.
instance = new element.type(element);
} else if (typeof node === 'string' || typeof node === 'number') {
instance = ReactNativeComponent.createInstanceForText(node);
} else {
// TODO: Update to follow new ES6 initialization. Ideally, we can use props
// in property initializers.
var inst = new element.type(element.props);
instance = new ReactCompositeComponentWrapper(inst);
invariant(
false,
'Encountered invalid React node of type ' + typeof node
);
}

if (__DEV__) {
warning(
typeof instance.construct === 'function' &&
typeof instance.mountComponent === 'function' &&
typeof instance.receiveComponent === 'function',
typeof instance.receiveComponent === 'function' &&
typeof instance.unmountComponent === 'function',
'Only React Components can be mounted.'
);
}

// Sets up the instance. This can probably just move into the constructor now.
instance.construct(element);
instance.construct(node);

return instance;
}
Expand Down