Skip to content

Separate React Composite and Class #2445

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 7, 2014
Merged

Separate React Composite and Class #2445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/browser/ui/ReactBrowserComponentMixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

"use strict";

var ReactEmptyComponent = require('ReactEmptyComponent');
var ReactMount = require('ReactMount');

var invariant = require('invariant');
Expand All @@ -29,10 +28,7 @@ var ReactBrowserComponentMixin = {
this.isMounted(),
'getDOMNode(): A component must be mounted to have a DOM node.'
);
if (ReactEmptyComponent.isNullComponentID(this._rootNodeID)) {
return null;
}
return ReactMount.getNode(this._rootNodeID);
return ReactMount.getNodeFromInstance(this);
}
};

Expand Down
32 changes: 30 additions & 2 deletions src/browser/ui/ReactMount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ var DOMProperty = require('DOMProperty');
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactElement = require('ReactElement');
var ReactEmptyComponent = require('ReactEmptyComponent');
var ReactInstanceHandles = require('ReactInstanceHandles');
var ReactInstanceMap = require('ReactInstanceMap');
var ReactPerf = require('ReactPerf');

var containsNode = require('containsNode');
Expand Down Expand Up @@ -125,6 +127,30 @@ function getNode(id) {
return nodeCache[id];
}

/**
* Finds the node with the supplied public React instance.
*
* @param {*} instance A public React instance.
* @return {?DOMElement} DOM node with the suppled `id`.
* @internal
*/
function getNodeFromInstance(instance) {
// This instance can currently be either a public or private instance since
// native nodes are still public.
var id = instance._rootNodeID;
// TODO: Once these are only public instances, remove this conditional.
if (id == null) {
id = ReactInstanceMap.get(instance)._rootNodeID;
}
if (ReactEmptyComponent.isNullComponentID(id)) {
return null;
}
if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) {
nodeCache[id] = ReactMount.findReactNodeByID(id);
}
return nodeCache[id];
}

/**
* A node is "valid" if it is contained by a currently mounted container.
*
Expand Down Expand Up @@ -358,7 +384,7 @@ var ReactMount = {
nextElement,
container,
callback
);
).getPublicInstance();
} else {
ReactMount.unmountComponentAtNode(container);
}
Expand All @@ -374,7 +400,7 @@ var ReactMount = {
nextElement,
container,
shouldReuseMarkup
);
).getPublicInstance();
callback && callback.call(component);
return component;
},
Expand Down Expand Up @@ -677,6 +703,8 @@ var ReactMount = {

getNode: getNode,

getNodeFromInstance: getNodeFromInstance,

purgeID: purgeID
};

Expand Down
129 changes: 66 additions & 63 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,41 +21,39 @@ describe('ReactDOMComponent', function() {
describe('updateDOM', function() {
var React;
var ReactTestUtils;
var transaction;

beforeEach(function() {
React = require('React');
ReactTestUtils = require('ReactTestUtils');

var ReactReconcileTransaction = require('ReactReconcileTransaction');
transaction = new ReactReconcileTransaction();
});

it("should handle className", function() {
var stub = ReactTestUtils.renderIntoDocument(<div style={{}} />);

stub.receiveComponent({props: { className: 'foo' }}, transaction);
expect(stub.getDOMNode().className).toEqual('foo');
stub.receiveComponent({props: { className: 'bar' }}, transaction);
expect(stub.getDOMNode().className).toEqual('bar');
stub.receiveComponent({props: { className: null }}, transaction);
expect(stub.getDOMNode().className).toEqual('');
var container = document.createElement('div');
React.render(<div style={{}} />, container);

React.render(<div className={'foo'} />, container);
expect(container.firstChild.className).toEqual('foo');
React.render(<div className={'bar'} />, container);
expect(container.firstChild.className).toEqual('bar');
React.render(<div className={null} />, container);
expect(container.firstChild.className).toEqual('');
});

it("should gracefully handle various style value types", function() {
var stub = ReactTestUtils.renderIntoDocument(<div style={{}} />);
var stubStyle = stub.getDOMNode().style;
var container = document.createElement('div');
React.render(<div style={{}} />, container);
var stubStyle = container.firstChild.style;

// set initial style
var setup = { display: 'block', left: '1', top: 2, fontFamily: 'Arial' };
stub.receiveComponent({props: { style: setup }}, transaction);
React.render(<div style={setup} />, container);
expect(stubStyle.display).toEqual('block');
expect(stubStyle.left).toEqual('1px');
expect(stubStyle.fontFamily).toEqual('Arial');

// reset the style to their default state
var reset = { display: '', left: null, top: false, fontFamily: true };
stub.receiveComponent({props: { style: reset }}, transaction);
React.render(<div style={reset} />, container);
expect(stubStyle.display).toEqual('');
expect(stubStyle.left).toEqual('');
expect(stubStyle.top).toEqual('');
Expand All @@ -64,127 +62,132 @@ describe('ReactDOMComponent', function() {

it("should update styles when mutating style object", function() {
var styles = { display: 'none', fontFamily: 'Arial', lineHeight: 1.2 };
var stub = ReactTestUtils.renderIntoDocument(<div style={styles} />);
var container = document.createElement('div');
React.render(<div style={styles} />, container);

var stubStyle = stub.getDOMNode().style;
var stubStyle = container.firstChild.style;
Copy link
Member

Choose a reason for hiding this comment

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

getDOMNode

stubStyle.display = styles.display;
stubStyle.fontFamily = styles.fontFamily;

styles.display = 'block';

stub.receiveComponent({props: { style: styles }}, transaction);
React.render(<div style={styles} />, container);
expect(stubStyle.display).toEqual('block');
expect(stubStyle.fontFamily).toEqual('Arial');
expect(stubStyle.lineHeight).toEqual('1.2');

styles.fontFamily = 'Helvetica';

stub.receiveComponent({props: { style: styles }}, transaction);
React.render(<div style={styles} />, container);
expect(stubStyle.display).toEqual('block');
expect(stubStyle.fontFamily).toEqual('Helvetica');
expect(stubStyle.lineHeight).toEqual('1.2');

styles.lineHeight = 0.5;

stub.receiveComponent({props: { style: styles }}, transaction);
React.render(<div style={styles} />, container);
expect(stubStyle.display).toEqual('block');
expect(stubStyle.fontFamily).toEqual('Helvetica');
expect(stubStyle.lineHeight).toEqual('0.5');

stub.receiveComponent({props: { style: undefined }}, transaction);
React.render(<div style={undefined} />, container);
expect(stubStyle.display).toBe('');
expect(stubStyle.fontFamily).toBe('');
expect(stubStyle.lineHeight).toBe('');
});

it("should update styles if initially null", function() {
var styles = null;
var stub = ReactTestUtils.renderIntoDocument(<div style={styles} />);
var container = document.createElement('div');
React.render(<div style={styles} />, container);

var stubStyle = stub.getDOMNode().style;
var stubStyle = container.firstChild.style;

styles = {display: 'block'};

stub.receiveComponent({props: { style: styles }}, transaction);
React.render(<div style={styles} />, container);
expect(stubStyle.display).toEqual('block');
});

it("should remove attributes", function() {
var stub = ReactTestUtils.renderIntoDocument(<img height='17' />);
var container = document.createElement('div');
React.render(<img height='17' />, container);

expect(stub.getDOMNode().hasAttribute('height')).toBe(true);
stub.receiveComponent({props: {}}, transaction);
expect(stub.getDOMNode().hasAttribute('height')).toBe(false);
expect(container.firstChild.hasAttribute('height')).toBe(true);
React.render(<img />, container);
expect(container.firstChild.hasAttribute('height')).toBe(false);
});

it("should remove properties", function() {
var stub = ReactTestUtils.renderIntoDocument(<div className='monkey' />);
var container = document.createElement('div');
React.render(<div className='monkey' />, container);

expect(stub.getDOMNode().className).toEqual('monkey');
stub.receiveComponent({props: {}}, transaction);
expect(stub.getDOMNode().className).toEqual('');
expect(container.firstChild.className).toEqual('monkey');
React.render(<div />, container);
expect(container.firstChild.className).toEqual('');
});

it("should clear a single style prop when changing 'style'", function() {
var styles = {display: 'none', color: 'red'};
var stub = ReactTestUtils.renderIntoDocument(<div style={styles} />);
var container = document.createElement('div');
React.render(<div style={styles} />, container);

var stubStyle = stub.getDOMNode().style;
var stubStyle = container.firstChild.style;

styles = {color: 'green'};
stub.receiveComponent({props: { style: styles }}, transaction);
React.render(<div style={styles} />, container);
expect(stubStyle.display).toEqual('');
expect(stubStyle.color).toEqual('green');
});

it("should clear all the styles when removing 'style'", function() {
var styles = {display: 'none', color: 'red'};
var stub = ReactTestUtils.renderIntoDocument(<div style={styles} />);
var container = document.createElement('div');
React.render(<div style={styles} />, container);

var stubStyle = stub.getDOMNode().style;
var stubStyle = container.firstChild.style;

stub.receiveComponent({props: {}}, transaction);
React.render(<div />, container);
expect(stubStyle.display).toEqual('');
expect(stubStyle.color).toEqual('');
});

it("should empty element when removing innerHTML", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div dangerouslySetInnerHTML={{__html: ':)'}} />
);
var container = document.createElement('div');
React.render(<div dangerouslySetInnerHTML={{__html: ':)'}} />, container);

expect(stub.getDOMNode().innerHTML).toEqual(':)');
stub.receiveComponent({props: {}}, transaction);
expect(stub.getDOMNode().innerHTML).toEqual('');
expect(container.firstChild.innerHTML).toEqual(':)');
React.render(<div />, container);
expect(container.firstChild.innerHTML).toEqual('');
});

it("should transition from string content to innerHTML", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div>hello</div>
);
var container = document.createElement('div');
React.render(<div>hello</div>, container);

expect(stub.getDOMNode().innerHTML).toEqual('hello');
stub.receiveComponent(
{props: {dangerouslySetInnerHTML: {__html: 'goodbye'}}},
transaction
expect(container.firstChild.innerHTML).toEqual('hello');
React.render(
<div dangerouslySetInnerHTML={{__html: 'goodbye'}} />,
container
);
expect(stub.getDOMNode().innerHTML).toEqual('goodbye');
expect(container.firstChild.innerHTML).toEqual('goodbye');
});

it("should transition from innerHTML to string content", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div dangerouslySetInnerHTML={{__html: 'bonjour'}} />
);
var container = document.createElement('div');
React.render(<div dangerouslySetInnerHTML={{__html: 'bonjour'}} />
, container);

expect(stub.getDOMNode().innerHTML).toEqual('bonjour');
stub.receiveComponent({props: {children: 'adieu'}}, transaction);
expect(stub.getDOMNode().innerHTML).toEqual('adieu');
expect(container.firstChild.innerHTML).toEqual('bonjour');
React.render(<div>adieu</div>, container);
expect(container.firstChild.innerHTML).toEqual('adieu');
});

it("should not incur unnecessary DOM mutations", function() {
var stub = ReactTestUtils.renderIntoDocument(<div value="" />);
var container = document.createElement('div');
React.render(<div value="" />, container);

var node = stub.getDOMNode();
var node = container.firstChild;
var nodeValue = ''; // node.value always returns undefined
var nodeValueSetter = mocks.getMockFunction();
Object.defineProperty(node, 'value', {
Expand All @@ -196,10 +199,10 @@ describe('ReactDOMComponent', function() {
})
});

stub.receiveComponent({props: {value: ''}}, transaction);
React.render(<div value="" />, container);
expect(nodeValueSetter.mock.calls.length).toBe(0);

stub.receiveComponent({props: {}}, transaction);
React.render(<div />, container);
expect(nodeValueSetter.mock.calls.length).toBe(1);
});
});
Expand Down
6 changes: 5 additions & 1 deletion src/browser/ui/__tests__/ReactRenderDocument-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"use strict";

var React;
var ReactInstanceMap;
var ReactMount;

var getTestDocument;
Expand All @@ -32,6 +33,7 @@ describe('rendering React components at document', function() {
require('mock-modules').dumpCache();

React = require('React');
ReactInstanceMap = require('ReactInstanceMap');
ReactMount = require('ReactMount');
getTestDocument = require('getTestDocument');

Expand Down Expand Up @@ -61,8 +63,10 @@ describe('rendering React components at document', function() {
var component = React.render(<Root />, testDocument);
expect(testDocument.body.innerHTML).toBe('Hello world');

// TODO: This is a bad test. I have no idea what this is testing.
// Node IDs is an implementation detail and not part of the public API.
var componentID = ReactMount.getReactRootID(testDocument);
expect(componentID).toBe(component._rootNodeID);
expect(componentID).toBe(ReactInstanceMap.get(component)._rootNodeID);
});

it('should not be able to unmount component from document node', function() {
Expand Down
Loading