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

[react-dom] Remove findDOMNode from OSS builds #28267

Merged
merged 1 commit into from
Mar 27, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Expose findDOMNode on internals
Exposes `findDOMNode` on internals and updates tests to read from internals
  • Loading branch information
gnoff committed Mar 27, 2024
commit 9928ca53e4f173d571e34e4eca050df088919511
1 change: 0 additions & 1 deletion packages/react-dom/index.experimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export {
createPortal,
createRoot,
hydrateRoot,
findDOMNode,
flushSync,
unstable_batchedUpdates,
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export {
createPortal,
createRoot,
hydrateRoot,
findDOMNode,
Copy link
Member

Choose a reason for hiding this comment

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

Should we provide an error like "findDOMNode has been removed, see react.dev/finddomnode for more info"?

flushSync,
render,
unmountComponentAtNode,
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom/index.stable.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export {
createPortal,
createRoot,
hydrateRoot,
findDOMNode,
flushSync,
render,
unmountComponentAtNode,
Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/ReactDOMSharedInternals.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* @flow
*/

import type {FindDOMNodeType} from './client/ReactDOMLegacy.js';
import type {HostDispatcher} from './shared/ReactDOMTypes';

type InternalsType = {
Expand All @@ -15,6 +16,7 @@ type InternalsType = {
ReactDOMCurrentDispatcher: {
current: HostDispatcher,
},
findDOMNode: null | FindDOMNodeType,
};

function noop() {}
Expand All @@ -35,6 +37,7 @@ const Internals: InternalsType = ({
ReactDOMCurrentDispatcher: {
current: DefaultDispatcher,
},
findDOMNode: null,
}: any);

export default Internals;
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ let React;
let ReactDOM;
let ReactDOMClient;
let PropTypes;
let findDOMNode;

const clone = function (o) {
return JSON.parse(JSON.stringify(o));
Expand Down Expand Up @@ -95,6 +96,8 @@ describe('ReactComponentLifeCycle', () => {

React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
PropTypes = require('prop-types');
});
Expand Down Expand Up @@ -383,7 +386,7 @@ describe('ReactComponentLifeCycle', () => {
}
render() {
if (this.state.isMounted) {
expect(ReactDOM.findDOMNode(this).tagName).toBe('DIV');
expect(findDOMNode(this).tagName).toBe('DIV');
}
return <div />;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOM-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

let React;
let ReactDOM;
let findDOMNode;
let ReactDOMClient;
let ReactDOMServer;

Expand All @@ -21,6 +22,8 @@ describe('ReactDOM', () => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
ReactDOMServer = require('react-dom/server');

Expand Down Expand Up @@ -494,7 +497,7 @@ describe('ReactDOM', () => {
});

const App = () => {
ReactDOM.findDOMNode(instance);
findDOMNode(instance);
return <div />;
};

Expand Down
6 changes: 5 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2121,7 +2121,11 @@ describe('ReactDOMComponent', () => {

componentWillUnmount() {
// Should not throw
expect(ReactDOM.findDOMNode(this).nodeName).toBe('SPAN');
expect(
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
this,
).nodeName,
).toBe('SPAN');
}
}

Expand Down
12 changes: 10 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,18 @@ describe('ReactDOMEventListener', () => {
this.setState({clicked: true});
};
componentDidMount() {
expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild);
expect(
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
this,
),
).toBe(container.firstChild);
}
componentDidUpdate() {
expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild);
expect(
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
this,
),
).toBe(container.firstChild);
}
render() {
if (this.state.clicked) {
Expand Down
30 changes: 24 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMLegacyFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ describe('ReactDOMLegacyFiber', () => {
container,
);

const textNode = ReactDOM.findDOMNode(instance);
const textNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
instance,
);
expect(textNode).toBe(container.firstChild);
expect(textNode.nodeType).toBe(3);
expect(textNode.nodeValue).toBe('foo');
Expand All @@ -130,7 +133,10 @@ describe('ReactDOMLegacyFiber', () => {

expect(container.childNodes.length).toBe(2);

const firstNode = ReactDOM.findDOMNode(instance);
const firstNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
instance,
);
expect(firstNode).toBe(container.firstChild);
expect(firstNode.tagName).toBe('DIV');
});
Expand Down Expand Up @@ -159,7 +165,10 @@ describe('ReactDOMLegacyFiber', () => {

expect(container.childNodes.length).toBe(2);

const firstNode = ReactDOM.findDOMNode(instance);
const firstNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
instance,
);
expect(firstNode).toBe(container.firstChild);
expect(firstNode.tagName).toBe('DIV');
});
Expand All @@ -183,7 +192,10 @@ describe('ReactDOMLegacyFiber', () => {

expect(container.childNodes.length).toBe(2);

const firstNode = ReactDOM.findDOMNode(instance);
const firstNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
instance,
);
expect(firstNode).toBe(container.firstChild);
expect(firstNode.tagName).toBe('DIV');
});
Expand Down Expand Up @@ -878,13 +890,19 @@ describe('ReactDOMLegacyFiber', () => {
}

const myNodeA = ReactDOM.render(<MyNode />, container);
const a = ReactDOM.findDOMNode(myNodeA);
const a =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
myNodeA,
);
expect(a.tagName).toBe('DIV');

const myNodeB = ReactDOM.render(<MyNode flag={true} />, container);
expect(myNodeA === myNodeB).toBe(true);

const b = ReactDOM.findDOMNode(myNodeB);
const b =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode(
myNodeB,
);
expect(b.tagName).toBe('SPAN');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

let React;
let ReactDOM;
let findDOMNode;
let ReactDOMClient;
let Suspense;
let Scheduler;
Expand All @@ -24,6 +25,8 @@ describe('ReactDOMSuspensePlaceholder', () => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
Scheduler = require('scheduler');
act = require('internal-test-utils').act;
Expand Down Expand Up @@ -232,11 +235,11 @@ describe('ReactDOMSuspensePlaceholder', () => {
class Child extends React.Component {
componentDidMount() {
log.push('cDM ' + this.props.id);
ReactDOM.findDOMNode(this);
findDOMNode(this);
}
componentDidUpdate() {
log.push('cDU ' + this.props.id);
ReactDOM.findDOMNode(this);
findDOMNode(this);
}
render() {
return 'child';
Expand Down Expand Up @@ -291,12 +294,12 @@ describe('ReactDOMSuspensePlaceholder', () => {
class Child extends React.Component {
componentDidMount() {
log.push('cDM');
ReactDOM.findDOMNode(this);
findDOMNode(this);
}

componentDidUpdate() {
log.push('cDU');
ReactDOM.findDOMNode(this);
findDOMNode(this);
}

render() {
Expand Down
11 changes: 7 additions & 4 deletions packages/react-dom/src/__tests__/ReactEmptyComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

let React;
let ReactDOM;
let findDOMNode;
let ReactDOMClient;
let TogglingComponent;
let act;
Expand All @@ -25,6 +26,8 @@ describe('ReactEmptyComponent', () => {

React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
Scheduler = require('scheduler');
const InternalTestUtils = require('internal-test-utils');
Expand All @@ -37,12 +40,12 @@ describe('ReactEmptyComponent', () => {
state = {component: this.props.firstComponent};

componentDidMount() {
Scheduler.log('mount ' + ReactDOM.findDOMNode(this)?.nodeName);
Scheduler.log('mount ' + findDOMNode(this)?.nodeName);
this.setState({component: this.props.secondComponent});
}

componentDidUpdate() {
Scheduler.log('update ' + ReactDOM.findDOMNode(this)?.nodeName);
Scheduler.log('update ' + findDOMNode(this)?.nodeName);
}

render() {
Expand Down Expand Up @@ -244,13 +247,13 @@ describe('ReactEmptyComponent', () => {
componentDidMount() {
// Make sure the DOM node resolves properly even if we're replacing a
// `null` component
expect(ReactDOM.findDOMNode(this)).not.toBe(null);
expect(findDOMNode(this)).not.toBe(null);
}

componentWillUnmount() {
// Even though we're getting replaced by `null`, we haven't been
// replaced yet!
expect(ReactDOM.findDOMNode(this)).not.toBe(null);
expect(findDOMNode(this)).not.toBe(null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

let React;
let ReactDOM;
let findDOMNode;
let ReactDOMClient;
let PropTypes;
let act;
Expand All @@ -20,6 +21,8 @@ describe('ReactLegacyCompositeComponent', () => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
PropTypes = require('prop-types');
act = require('internal-test-utils').act;
Expand Down Expand Up @@ -115,7 +118,7 @@ describe('ReactLegacyCompositeComponent', () => {
await act(() => {
root.render(<Parent ref={current => (component = current)} />);
});
expect(ReactDOM.findDOMNode(component).innerHTML).toBe('bar');
expect(findDOMNode(component).innerHTML).toBe('bar');
});

// @gate !disableLegacyContext
Expand Down Expand Up @@ -661,14 +664,14 @@ describe('ReactLegacyCompositeComponent', () => {

const container = document.createElement('div');
const comp = ReactDOM.render(<Component flipped={false} />, container);
expect(ReactDOM.findDOMNode(comp.static0Ref.current).textContent).toBe('A');
expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('B');
expect(findDOMNode(comp.static0Ref.current).textContent).toBe('A');
expect(findDOMNode(comp.static1Ref.current).textContent).toBe('B');

// 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.static0Ref.current).textContent).toBe('B');
expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('A');
expect(findDOMNode(comp.static0Ref.current).textContent).toBe('B');
expect(findDOMNode(comp.static1Ref.current).textContent).toBe('A');
});

// @gate !disableLegacyMode
Expand All @@ -678,12 +681,12 @@ describe('ReactLegacyCompositeComponent', () => {

class Component extends React.Component {
componentDidMount() {
a = ReactDOM.findDOMNode(this);
a = findDOMNode(this);
expect(a).not.toBe(null);
}

componentWillUnmount() {
b = ReactDOM.findDOMNode(this);
b = findDOMNode(this);
expect(b).not.toBe(null);
}

Expand Down
Loading