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

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)
EdisonVan · Apr 15, 2024 · 7e40e38 · 7e40e38
1 parent 25d624d
commit 7e40e38
Show file tree

Hide file tree

Showing 25 changed files with 114 additions and 73 deletions.
diff --git a/packages/react-dom/index.experimental.js b/packages/react-dom/index.experimental.js
@@ -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.

diff --git a/packages/react-dom/index.js b/packages/react-dom/index.js
@@ -14,7 +14,6 @@ export {
   createPortal,
   createRoot,
   hydrateRoot,
-  findDOMNode,
   flushSync,
   render,
   unmountComponentAtNode,

diff --git a/packages/react-dom/index.stable.js b/packages/react-dom/index.stable.js
@@ -12,7 +12,6 @@ export {
   createPortal,
   createRoot,
   hydrateRoot,
-  findDOMNode,
   flushSync,
   render,
   unmountComponentAtNode,

diff --git a/packages/react-dom/src/ReactDOMSharedInternals.js b/packages/react-dom/src/ReactDOMSharedInternals.js
@@ -7,6 +7,7 @@
  * @flow
  */
 
+import type {FindDOMNodeType} from './client/ReactDOMLegacy.js';
 import type {HostDispatcher} from './shared/ReactDOMTypes';
 
 type InternalsType = {
@@ -15,6 +16,7 @@ type InternalsType = {
   ReactDOMCurrentDispatcher: {
     current: HostDispatcher,
   },
+  findDOMNode: null | FindDOMNodeType,
 };
 
 function noop() {}
@@ -35,6 +37,7 @@ const Internals: InternalsType = ({
   ReactDOMCurrentDispatcher: {
     current: DefaultDispatcher,
   },
+  findDOMNode: null,
 }: any);
 
 export default Internals;
diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
@@ -15,6 +15,7 @@ let React;
 let ReactDOM;
 let ReactDOMClient;
 let PropTypes;
+let findDOMNode;
 
 const clone = function (o) {
   return JSON.parse(JSON.stringify(o));
@@ -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');
   });
@@ -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 />;
       }

diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js
@@ -11,6 +11,7 @@
 
 let React;
 let ReactDOM;
+let findDOMNode;
 let ReactDOMClient;
 let ReactDOMServer;
 
@@ -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');
 
@@ -494,7 +497,7 @@ describe('ReactDOM', () => {
     });
 
     const App = () => {
-      ReactDOM.findDOMNode(instance);
+      findDOMNode(instance);
       return <div />;
     };
 

diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -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');
         }
       }
 

diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
@@ -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) {

diff --git a/packages/react-dom/src/__tests__/ReactDOMLegacyFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMLegacyFiber-test.js
@@ -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');
@@ -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');
   });
@@ -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');
   });
@@ -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');
   });
@@ -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');
   });
 

diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js
@@ -11,6 +11,7 @@
 
 let React;
 let ReactDOM;
+let findDOMNode;
 let ReactDOMClient;
 let Suspense;
 let Scheduler;
@@ -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;
@@ -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';
@@ -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() {

diff --git a/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js b/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js
@@ -11,6 +11,7 @@
 
 let React;
 let ReactDOM;
+let findDOMNode;
 let ReactDOMClient;
 let TogglingComponent;
 let act;
@@ -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');
@@ -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() {
@@ -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);
         }
       }
 

diff --git a/packages/react-dom/src/__tests__/ReactLegacyCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactLegacyCompositeComponent-test.js
@@ -11,6 +11,7 @@
 
 let React;
 let ReactDOM;
+let findDOMNode;
 let ReactDOMClient;
 let PropTypes;
 let act;
@@ -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;
@@ -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
@@ -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
@@ -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);
       }