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.

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

Jump to bottom

Remove legacy dom node/ref stuff. #5495

Merged
merged 1 commit into from
Nov 19, 2015
Merged

Remove legacy dom node/ref stuff. #5495

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
130 changes: 1 addition & 129 deletions src/renderers/dom/shared/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ var ReactDOMSelect = require('ReactDOMSelect');
var ReactDOMTextarea = require('ReactDOMTextarea');
var ReactMultiChild = require('ReactMultiChild');
var ReactPerf = require('ReactPerf');
var ReactUpdateQueue = require('ReactUpdateQueue');

var assign = require('Object.assign');
var canDefineProperty = require('canDefineProperty');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var invariant = require('invariant');
var isEventSupported = require('isEventSupported');
Expand Down Expand Up @@ -73,102 +71,6 @@ function getDeclarationErrorAddendum(internalInstance) {
return '';
}

var legacyPropsDescriptor;
if (__DEV__) {
legacyPropsDescriptor = {
props: {
enumerable: false,
get: function() {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
warning(
false,
'ReactDOMComponent: Do not access .props of a DOM node; instead, ' +
'recreate the props as `render` did originally or read the DOM ' +
'properties/attributes directly from this node (e.g., ' +
'this.refs.box.className).%s',
getDeclarationErrorAddendum(component)
);
return component._currentElement.props;
},
},
};
}

function legacyGetDOMNode() {
if (__DEV__) {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
warning(
false,
'ReactDOMComponent: Do not access .getDOMNode() of a DOM node; ' +
'instead, use the node directly.%s',
getDeclarationErrorAddendum(component)
);
}
return this;
}

function legacyIsMounted() {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
if (__DEV__) {
warning(
false,
'ReactDOMComponent: Do not access .isMounted() of a DOM node.%s',
getDeclarationErrorAddendum(component)
);
}
return !!component;
}

function legacySetStateEtc() {
if (__DEV__) {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
warning(
false,
'ReactDOMComponent: Do not access .setState(), .replaceState(), or ' +
'.forceUpdate() of a DOM node. This is a no-op.%s',
getDeclarationErrorAddendum(component)
);
}
}

function legacySetProps(partialProps, callback) {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
if (__DEV__) {
warning(
false,
'ReactDOMComponent: Do not access .setProps() of a DOM node. ' +
'Instead, call ReactDOM.render again at the top level.%s',
getDeclarationErrorAddendum(component)
);
}
if (!component) {
return;
}
ReactUpdateQueue.enqueueSetPropsInternal(component, partialProps);
if (callback) {
ReactUpdateQueue.enqueueCallbackInternal(component, callback);
}
}

function legacyReplaceProps(partialProps, callback) {
var component = ReactDOMComponentTree.getInstanceFromNode(this);
if (__DEV__) {
warning(
false,
'ReactDOMComponent: Do not access .replaceProps() of a DOM node. ' +
'Instead, call ReactDOM.render again at the top level.%s',
getDeclarationErrorAddendum(component)
);
}
if (!component) {
return;
}
ReactUpdateQueue.enqueueReplacePropsInternal(component, partialProps);
if (callback) {
ReactUpdateQueue.enqueueCallbackInternal(component, callback);
}
}

function friendlyStringify(obj) {
if (typeof obj === 'object') {
if (Array.isArray(obj)) {
Expand Down Expand Up @@ -908,10 +810,6 @@ ReactDOMComponent.Mixin = {
context
);

if (!canDefineProperty && this._flags & Flags.nodeHasLegacyProperties) {
this._nativeNode.props = nextProps;
}

if (this._tag === 'select') {
// <select> value update needs to occur after <option> children
// reconciliation
Expand Down Expand Up @@ -1157,33 +1055,7 @@ ReactDOMComponent.Mixin = {
},

getPublicInstance: function() {
var node = getNode(this);
if (this._flags & Flags.nodeHasLegacyProperties) {
return node;
} else {
node.getDOMNode = legacyGetDOMNode;
node.isMounted = legacyIsMounted;
node.setState = legacySetStateEtc;
node.replaceState = legacySetStateEtc;
node.forceUpdate = legacySetStateEtc;
node.setProps = legacySetProps;
node.replaceProps = legacyReplaceProps;

if (__DEV__) {
if (canDefineProperty) {
Object.defineProperties(node, legacyPropsDescriptor);
} else {
// updateComponent will update this property on subsequent renders
node.props = this._currentElement.props;
}
} else {
// updateComponent will update this property on subsequent renders
node.props = this._currentElement.props;
}

this._flags |= Flags.nodeHasLegacyProperties;
return node;
}
return getNode(this);
},

};
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/dom/shared/ReactDOMComponentFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
'use strict';

var ReactDOMComponentFlags = {
nodeHasLegacyProperties: 1 << 0,
hasCachedChildNodes: 1 << 1,
hasCachedChildNodes: 1 << 0,
};

module.exports = ReactDOMComponentFlags;
82 changes: 0 additions & 82 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,86 +1088,4 @@ describe('ReactDOMComponent', function() {
);
});
});

describe('DOM nodes as refs', function() {
var ReactTestUtils;

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

it('warns when accessing properties on DOM components', function() {
spyOn(console, 'error');
var innerDiv;
var Animal = React.createClass({
render: function() {
return <div ref="div">iguana</div>;
},
componentDidMount: function() {
innerDiv = this.refs.div;

void this.refs.div.props;
this.refs.div.setState();
expect(this.refs.div.getDOMNode()).toBe(this.refs.div);
expect(this.refs.div.isMounted()).toBe(true);
},
});
var container = document.createElement('div');
ReactDOM.render(<Animal />, container);
ReactDOM.unmountComponentAtNode(container);
expect(innerDiv.isMounted()).toBe(false);

expect(console.error.calls.length).toBe(5);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: ReactDOMComponent: Do not access .props of a DOM ' +
'node; instead, recreate the props as `render` did originally or ' +
'read the DOM properties/attributes directly from this node (e.g., ' +
'this.refs.box.className). This DOM node was rendered by `Animal`.'
);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: ReactDOMComponent: Do not access .setState(), ' +
'.replaceState(), or .forceUpdate() of a DOM node. This is a no-op. ' +
'This DOM node was rendered by `Animal`.'
);
expect(console.error.argsForCall[2][0]).toBe(
'Warning: ReactDOMComponent: Do not access .getDOMNode() of a DOM ' +
'node; instead, use the node directly. This DOM node was ' +
'rendered by `Animal`.'
);
expect(console.error.argsForCall[3][0]).toBe(
'Warning: ReactDOMComponent: Do not access .isMounted() of a DOM ' +
'node. This DOM node was rendered by `Animal`.'
);
expect(console.error.argsForCall[4][0]).toContain('isMounted');
});

it('handles legacy setProps and replaceProps', function() {
spyOn(console, 'error');
var node = ReactTestUtils.renderIntoDocument(<div>rhinoceros</div>);

node.setProps({className: 'herbiverous'});
expect(node.className).toBe('herbiverous');
expect(node.textContent).toBe('rhinoceros');

node.replaceProps({className: 'invisible rhino'});
expect(node.className).toBe('invisible rhino');
expect(node.textContent).toBe('');

expect(console.error.calls.length).toBe(2);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: ReactDOMComponent: Do not access .setProps() of a DOM node. ' +
'Instead, call ReactDOM.render again at the top level.'
);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: ReactDOMComponent: Do not access .replaceProps() of a DOM ' +
'node. Instead, call ReactDOM.render again at the top level.'
);
});

it('does not touch ref-less nodes', function() {
var node = ReactTestUtils.renderIntoDocument(<div><span /></div>);
expect(typeof node.getDOMNode).toBe('function');
expect(typeof node.firstChild.getDOMNode).toBe('undefined');
});
});
});
2 changes: 1 addition & 1 deletion src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe('ReactCompositeComponent', function() {
var anchor = instance.getAnchor();
var actualDOMAnchorNode = ReactDOM.findDOMNode(anchor);
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't need ReactDOM.findDOMNode here, if I'm reading this right? IIRC since the ref is a non-composite, it should directly return the node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think I agree, we don't need any of the actualDOMAnchorNode stuff. But I didn't want to confuse the diff by modifying test logic unnecessarily.

expect(actualDOMAnchorNode.className).toBe('');
expect(actualDOMAnchorNode).toBe(anchor.getDOMNode());
expect(actualDOMAnchorNode).toBe(ReactDOM.findDOMNode(anchor));
});

it('should auto bind methods and values correctly', function() {
Expand Down