Skip to content

Commit

Permalink
[react-dom] Remove findDOMNode from OSS builds (#28267)
Browse files Browse the repository at this point in the history
In the next major `findDOMNode` is being removed. This PR removes the
API from the react-dom entrypoints for OSS builds and re-exposes the
implementation as part of internals.

`findDOMNode` is being retained for Meta builds and so all tests that
currently use it will continue to do so by accessing it from internals.
Once the replacement API ships in an upcoming minor any tests that were
using this API incidentally can be updated to use the new API and any
tests asserting `findDOMNode`'s behavior directly can stick around until
we remove it entirely (once Meta has moved away from it)
  • Loading branch information
gnoff authored Mar 27, 2024
1 parent 8436bcc commit 9ad40b1
Show file tree
Hide file tree
Showing 25 changed files with 114 additions and 73 deletions.
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,
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

0 comments on commit 9ad40b1

Please sign in to comment.