Skip to content

Commit

Permalink
Removed getDOMNode from react classes.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimfb committed Jan 12, 2016
1 parent b60e8c2 commit e8af100
Show file tree
Hide file tree
Showing 13 changed files with 11 additions and 148 deletions.
19 changes: 0 additions & 19 deletions src/isomorphic/classic/class/__tests__/ReactClass-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,23 +362,4 @@ describe('ReactClass-spec', function() {
);
});

it('warns when calling getDOMNode', function() {
var MyComponent = React.createClass({
render: function() {
return <div />;
},
});

var container = document.createElement('div');
var instance = ReactDOM.render(<MyComponent />, container);

instance.getDOMNode();

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'MyComponent.getDOMNode(...) is deprecated. Please use ' +
'ReactDOM.findDOMNode(instance) instead.'
);
});

});
4 changes: 0 additions & 4 deletions src/isomorphic/modern/class/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ ReactComponent.prototype.forceUpdate = function(callback) {
*/
if (__DEV__) {
var deprecatedAPIs = {
getDOMNode: [
'getDOMNode',
'Use ReactDOM.findDOMNode(component) instead.',
],
isMounted: [
'isMounted',
'Instead, make sure to clean up subscriptions and pending requests in ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,19 +345,15 @@ describe 'ReactCoffeeScriptClass', ->
spyOn console, 'error'
instance =
test Inner(name: 'foo'), 'DIV', 'foo'
expect(-> instance.getDOMNode()).toThrow()
expect(-> instance.replaceState {}).toThrow()
expect(-> instance.isMounted()).toThrow()
expect(-> instance.setProps name: 'bar').toThrow()
expect(-> instance.replaceProps name: 'bar').toThrow()
expect(console.error.calls.length).toBe 3
expect(console.error.calls.length).toBe 2
expect(console.error.argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes'
)
expect(console.error.argsForCall[1][0]).toContain(
'replaceState(...) is deprecated in plain JavaScript React classes'
)
expect(console.error.argsForCall[2][0]).toContain(
expect(console.error.argsForCall[1][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
)

Expand Down
9 changes: 2 additions & 7 deletions src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,20 +402,15 @@ describe('ReactES6Class', function() {
it('should throw AND warn when trying to access classic APIs', function() {
spyOn(console, 'error');
var instance = test(<Inner name="foo" />, 'DIV', 'foo');
expect(() => instance.getDOMNode()).toThrow();
expect(() => instance.replaceState({})).toThrow();
expect(() => instance.isMounted()).toThrow();
expect(() => instance.setProps({name: 'bar'})).toThrow();
expect(() => instance.replaceProps({name: 'bar'})).toThrow();
expect(console.error.calls.length).toBe(3);
expect(console.error.calls.length).toBe(2);
expect(console.error.argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes. ' +
'Use ReactDOM.findDOMNode(component) instead.'
);
expect(console.error.argsForCall[1][0]).toContain(
'replaceState(...) is deprecated in plain JavaScript React classes'
);
expect(console.error.argsForCall[2][0]).toContain(
expect(console.error.argsForCall[1][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,19 +486,15 @@ describe('ReactTypeScriptClass', function() {
React.createElement(Inner, {name: 'foo'}),
'DIV','foo'
);
expect(() => instance.getDOMNode()).toThrow();
expect(() => instance.replaceState({})).toThrow();
expect(() => instance.isMounted()).toThrow();
expect(() => instance.setProps({ name: 'bar' })).toThrow();
expect(() => instance.replaceProps({ name: 'bar' })).toThrow();
expect((<any>console.error).argsForCall.length).toBe(3);
expect((<any>console.error).argsForCall.length).toBe(2);
expect((<any>console.error).argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes'
);
expect((<any>console.error).argsForCall[1][0]).toContain(
'replaceState(...) is deprecated in plain JavaScript React classes'
);
expect((<any>console.error).argsForCall[2][0]).toContain(
expect((<any>console.error).argsForCall[1][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
);
});
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/client/findDOMNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function findDOMNode(componentOrElement) {
if (owner !== null) {
warning(
owner._warnedAboutRefsInRender,
'%s is accessing getDOMNode or findDOMNode inside its render(). ' +
'%s is accessing findDOMNode inside its render(). ' +
'render() should be a pure function of props and state. It should ' +
'never access something that requires stale data from the previous ' +
'render, such as refs. Move this logic to componentDidMount and ' +
Expand Down
41 changes: 0 additions & 41 deletions src/renderers/dom/shared/ReactBrowserComponentMixin.js

This file was deleted.

3 changes: 0 additions & 3 deletions src/renderers/dom/shared/ReactDefaultInjection.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var DefaultEventPluginOrder = require('DefaultEventPluginOrder');
var EnterLeaveEventPlugin = require('EnterLeaveEventPlugin');
var ExecutionEnvironment = require('ExecutionEnvironment');
var HTMLDOMPropertyConfig = require('HTMLDOMPropertyConfig');
var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
var ReactComponentBrowserEnvironment =
require('ReactComponentBrowserEnvironment');
var ReactDOMComponent = require('ReactDOMComponent');
Expand Down Expand Up @@ -75,8 +74,6 @@ function inject() {
ReactDOMTextComponent
);

ReactInjection.Class.injectMixin(ReactBrowserComponentMixin);

This comment has been minimized.

Copy link
@vjeux

vjeux May 22, 2016

Contributor

See #6831


ReactInjection.DOMProperty.injectDOMPropertyConfig(HTMLDOMPropertyConfig);
ReactInjection.DOMProperty.injectDOMPropertyConfig(SVGDOMPropertyConfig);

Expand Down
20 changes: 0 additions & 20 deletions src/renderers/shared/reconciler/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,26 +265,6 @@ describe('ReactComponent', function() {
expect(callback.mock.calls.length).toBe(3);
});

it('warns when calling getDOMNode', function() {
spyOn(console, 'error');

var Potato = React.createClass({
render: function() {
return <div />;
},
});
var container = document.createElement('div');
var instance = ReactDOM.render(<Potato />, container);

instance.getDOMNode();

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'Potato.getDOMNode(...) is deprecated. Please use ' +
'ReactDOM.findDOMNode(instance) instead.'
);
});

it('throws usefully when rendering badly-typed elements', function() {
spyOn(console, 'error');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ describe('ReactComponentLifeCycle', function() {
ReactTestUtils.renderIntoDocument(<Component />);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'Component is accessing getDOMNode or findDOMNode inside its render()'
'Component is accessing findDOMNode inside its render()'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1073,21 +1073,11 @@ describe('ReactCompositeComponent', function() {
expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('A');
expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('B');

expect(ReactDOM.findDOMNode(comp.refs.static0))
.toBe(comp.refs.static0.getDOMNode());
expect(ReactDOM.findDOMNode(comp.refs.static1))
.toBe(comp.refs.static1.getDOMNode());

// When flipping the order, the refs should update even though the actual
// contents do not
ReactDOM.render(<Component flipped={true} />, container);
expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('B');
expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('A');

expect(ReactDOM.findDOMNode(comp.refs.static0))
.toBe(comp.refs.static0.getDOMNode());
expect(ReactDOM.findDOMNode(comp.refs.static1))
.toBe(comp.refs.static1.getDOMNode());
});

it('should allow access to findDOMNode in componentWillUnmount', function() {
Expand All @@ -1096,11 +1086,11 @@ describe('ReactCompositeComponent', function() {
var Component = React.createClass({
componentDidMount: function() {
a = ReactDOM.findDOMNode(this);
expect(a).toBe(this.getDOMNode());
expect(a).not.toBe(null);
},
componentWillUnmount: function() {
b = ReactDOM.findDOMNode(this);
expect(b).toBe(this.getDOMNode());
expect(b).not.toBe(null);
},
render: function() {
return <div />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('ReactEmptyComponent', function() {
}
);

it('should have getDOMNode return null when multiple layers of composite ' +
it('should have findDOMNode return null when multiple layers of composite ' +
'components render to the same null placeholder',
() => {
var GrandChild = React.createClass({
Expand Down
27 changes: 0 additions & 27 deletions src/test/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,33 +352,6 @@ describe('ReactTestUtils', function() {
expect(log).toEqual(['orangepurple', 'orange', 'purple']);
});

it('does not warn for getDOMNode on ES6 classes', function() {
var Foo = React.createClass({
render: function() {
return <div />;
},
});

class Bar extends React.Component {
render() {
return <div />;
}
}

spyOn(console, 'error');

var foo = ReactTestUtils.renderIntoDocument(<Foo />);
expect(ReactTestUtils.isDOMComponent(foo)).toBe(false);

var bar = ReactTestUtils.renderIntoDocument(<Bar />);
expect(ReactTestUtils.isDOMComponent(bar)).toBe(false);

var div = ReactTestUtils.renderIntoDocument(<div />);
expect(ReactTestUtils.isDOMComponent(div)).toBe(true);

expect(console.error.calls.length).toBe(0);
});

it('should support injected wrapper components as DOM components', function() {
var getTestDocument = require('getTestDocument');

Expand Down

0 comments on commit e8af100

Please sign in to comment.