[string-refs] cleanup string ref code (#31443)

facebook · Nov 6, 2024 · e137890 · e137890
1 parent a88b9e5
commit e137890
Show file tree

Hide file tree

Showing 36 changed files with 35 additions and 1,160 deletions.
diff --git a/packages/jest-react/src/JestReact.js b/packages/jest-react/src/JestReact.js
@@ -6,7 +6,6 @@
  */
 
 import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
-import {disableStringRefs} from 'shared/ReactFeatureFlags';
 const {assertConsoleLogsCleared} = require('internal-test-utils/consoleMock');
 
 import isArray from 'shared/isArray';
@@ -56,23 +55,13 @@ function createJSXElementForTestComparison(type, props) {
       value: null,
     });
     return element;
-  } else if (!__DEV__ && disableStringRefs) {
-    return {
-      $$typeof: REACT_ELEMENT_TYPE,
-      type: type,
-      key: null,
-      ref: null,
-      props: props,
-    };
   } else {
     return {
       $$typeof: REACT_ELEMENT_TYPE,
       type: type,
       key: null,
       ref: null,
       props: props,
-      _owner: null,
-      _store: __DEV__ ? {} : undefined,
     };
   }
 }

diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js
@@ -41,7 +41,6 @@ import type {Postpone} from 'react/src/ReactPostpone';
 import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences';
 
 import {
-  disableStringRefs,
   enableBinaryFlight,
   enablePostpone,
   enableFlightReadableStream,
@@ -688,16 +687,6 @@ function createElement(
       enumerable: false,
       get: nullRefGetter,
     });
-  } else if (!__DEV__ && disableStringRefs) {
-    element = ({
-      // This tag allows us to uniquely identify this as a React Element
-      $$typeof: REACT_ELEMENT_TYPE,
-
-      type,
-      key,
-      ref: null,
-      props,
-    }: any);
   } else {
     element = ({
       // This tag allows us to uniquely identify this as a React Element
@@ -707,9 +696,6 @@ function createElement(
       key,
       ref: null,
       props,
-
-      // Record the component responsible for creating this element.
-      _owner: __DEV__ && owner === null ? response._debugRootOwner : owner,
     }: any);
   }
 

diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -3268,9 +3268,7 @@ describe('ReactFlight', () => {
         expect(greeting._owner).toBe(greeting._debugInfo[0]);
       } else {
         expect(greeting._debugInfo).toBe(undefined);
-        expect(greeting._owner).toBe(
-          gate(flags => flags.disableStringRefs) ? undefined : null,
-        );
+        expect(greeting._owner).toBe(undefined);
       }
       ReactNoop.render(greeting);
     });

diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js
@@ -42,19 +42,6 @@ describe('ReactComponent', () => {
     }).toThrowError(/Target container is not a DOM element./);
   });
 
-  // @gate !disableStringRefs
-  it('should throw when supplying a string ref outside of render method', async () => {
-    const container = document.createElement('div');
-    const root = ReactDOMClient.createRoot(container);
-    await expect(
-      act(() => {
-        root.render(<div ref="badDiv" />);
-      }),
-      // TODO: This throws an AggregateError. Need to update test infra to
-      // support matching against AggregateError.
-    ).rejects.toThrow();
-  });
-
   it('should throw (in dev) when children are mutated during render', async () => {
     function Wrapper(props) {
       props.children[1] = <p key={1} />; // Mutation is illegal
@@ -132,105 +119,6 @@ describe('ReactComponent', () => {
     }
   });
 
-  // @gate !disableStringRefs
-  it('string refs do not detach and reattach on every render', async () => {
-    let refVal;
-    class Child extends React.Component {
-      componentDidUpdate() {
-        // The parent ref should still be attached because it hasn't changed
-        // since the last render. If the ref had changed, then this would be
-        // undefined because refs are attached during the same phase (layout)
-        // as componentDidUpdate, in child -> parent order. So the new parent
-        // ref wouldn't have attached yet.
-        refVal = this.props.contextRef();
-      }
-
-      render() {
-        if (this.props.show) {
-          return <div>child</div>;
-        }
-      }
-    }
-
-    class Parent extends React.Component {
-      render() {
-        return (
-          <div id="test-root" ref="root">
-            <Child
-              contextRef={() => this.refs.root}
-              show={this.props.showChild}
-            />
-          </div>
-        );
-      }
-    }
-
-    const container = document.createElement('div');
-    const root = ReactDOMClient.createRoot(container);
-
-    await act(() => {
-      root.render(<Parent />);
-    });
-
-    assertConsoleErrorDev(['contains the string ref']);
-
-    expect(refVal).toBe(undefined);
-    await act(() => {
-      root.render(<Parent showChild={true} />);
-    });
-    expect(refVal).toBe(container.querySelector('#test-root'));
-  });
-
-  // @gate !disableStringRefs
-  it('should support string refs on owned components', async () => {
-    const innerObj = {};
-    const outerObj = {};
-
-    class Wrapper extends React.Component {
-      getObject = () => {
-        return this.props.object;
-      };
-
-      render() {
-        return <div>{this.props.children}</div>;
-      }
-    }
-
-    class Component extends React.Component {
-      render() {
-        const inner = <Wrapper object={innerObj} ref="inner" />;
-        const outer = (
-          <Wrapper object={outerObj} ref="outer">
-            {inner}
-          </Wrapper>
-        );
-        return outer;
-      }
-
-      componentDidMount() {
-        expect(this.refs.inner.getObject()).toEqual(innerObj);
-        expect(this.refs.outer.getObject()).toEqual(outerObj);
-      }
-    }
-
-    const container = document.createElement('div');
-    const root = ReactDOMClient.createRoot(container);
-    await expect(async () => {
-      await act(() => {
-        root.render(<Component />);
-      });
-    }).toErrorDev([
-      'Component "Component" contains the string ref "inner". ' +
-        'Support for string refs will be removed in a future major release. ' +
-        'We recommend using useRef() or createRef() instead. ' +
-        'Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref\n' +
-        '    in Wrapper (at **)\n' +
-        '    in div (at **)\n' +
-        '    in Wrapper (at **)\n' +
-        '    in Component (at **)',
-    ]);
-  });
-
   it('should not have string refs on unmounted components', async () => {
     class Parent extends React.Component {
       render() {

diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
@@ -537,11 +537,8 @@ describe('ReactCompositeComponent', () => {
   });
 
   it('should cleanup even if render() fatals', async () => {
-    const dispatcherEnabled =
-      __DEV__ ||
-      !gate(flags => flags.disableStringRefs) ||
-      gate(flags => flags.enableCache);
-    const ownerEnabled = __DEV__ || !gate(flags => flags.disableStringRefs);
+    const dispatcherEnabled = __DEV__ || gate(flags => flags.enableCache);
+    const ownerEnabled = __DEV__;
 
     let stashedDispatcher;
     class BadComponent extends React.Component {

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js
@@ -29,12 +29,8 @@ function initModules() {
   };
 }
 
-const {
-  resetModules,
-  asyncReactDOMRender,
-  clientRenderOnServerString,
-  expectMarkupMatch,
-} = ReactDOMServerIntegrationUtils(initModules);
+const {resetModules, clientRenderOnServerString, expectMarkupMatch} =
+  ReactDOMServerIntegrationUtils(initModules);
 
 describe('ReactDOMServerIntegration', () => {
   beforeEach(() => {
@@ -75,36 +71,6 @@ describe('ReactDOMServerIntegration', () => {
       expect(refElement).not.toBe(null);
       expect(refElement).toBe(e);
     });
-
-    // @gate !disableStringRefs
-    it('should have string refs on client when rendered over server markup', async () => {
-      class RefsComponent extends React.Component {
-        render() {
-          return <div ref="myDiv" />;
-        }
-      }
-
-      const markup = ReactDOMServer.renderToString(<RefsComponent />);
-      const root = document.createElement('div');
-      root.innerHTML = markup;
-      let component = null;
-      resetModules();
-      await expect(async () => {
-        await asyncReactDOMRender(
-          <RefsComponent ref={e => (component = e)} />,
-          root,
-          true,
-        );
-      }).toErrorDev([
-        'Component "RefsComponent" contains the string ref "myDiv". ' +
-          'Support for string refs will be removed in a future major release. ' +
-          'We recommend using useRef() or createRef() instead. ' +
-          'Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref\n' +
-          '    in div (at **)\n' +
-          '    in RefsComponent (at **)',
-      ]);
-      expect(component.refs.myDiv).toBe(root.firstChild);
-    });
   });
 
   it('should forward refs', async () => {