From e0e03eb5079ef16917ac92384c88ac6affc740cf Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 2 Oct 2022 18:16:02 +0200 Subject: [PATCH] Drop support for string refs --- packages/jest-react/src/JestReact.js | 20 +- .../react-client/src/ReactFlightClient.js | 12 +- .../src/__tests__/ReactComponent-test.js | 79 ----- .../ReactDOMServerIntegrationRefs-test.js | 35 --- .../ReactDeprecationWarnings-test.internal.js | 101 ------- .../__tests__/ReactFunctionComponent-test.js | 30 +- .../multiple-copies-of-react-test.js | 7 +- packages/react-dom/src/__tests__/refs-test.js | 283 +----------------- .../src/createReactNoop.js | 41 ++- .../src/ReactChildFiber.new.js | 133 +------- .../src/ReactChildFiber.old.js | 133 +------- .../src/ReactInternalTypes.js | 5 +- .../ReactIncrementalSideEffects-test.js | 38 --- packages/react/src/ReactElement.js | 63 +--- packages/react/src/ReactElementValidator.js | 1 + .../ReactCoffeeScriptClass-test.coffee | 24 +- .../react/src/__tests__/ReactES6Class-test.js | 24 +- .../react/src/__tests__/ReactElement-test.js | 1 + .../ReactElementValidator-test.internal.js | 11 +- .../src/__tests__/ReactStrictMode-test.js | 99 ------ .../__tests__/ReactTypeScriptClass-test.ts | 25 +- packages/react/src/jsx/ReactJSXElement.js | 11 +- packages/shared/ReactElementType.js | 3 +- packages/shared/ReactFeatureFlags.js | 2 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.testing.js | 1 - .../forks/ReactFeatureFlags.testing.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - scripts/error-codes/codes.json | 3 +- 33 files changed, 145 insertions(+), 1047 deletions(-) diff --git a/packages/jest-react/src/JestReact.js b/packages/jest-react/src/JestReact.js index f67ddd8636f1a..cf1dff753b4e0 100644 --- a/packages/jest-react/src/JestReact.js +++ b/packages/jest-react/src/JestReact.js @@ -63,9 +63,15 @@ export function unstable_toMatchRenderedOutput(root, expectedJSX) { props: { children: actualJSXChildren, }, - _owner: null, _store: __DEV__ ? {} : undefined, }; + + Object.defineProperty(actualJSX, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); } } } else { @@ -82,7 +88,7 @@ function jsonChildToJSXChild(jsonChild) { return jsonChild; } else { const jsxChildren = jsonChildrenToJSXChildren(jsonChild.children); - return { + const element = { $$typeof: REACT_ELEMENT_TYPE, type: jsonChild.type, key: null, @@ -91,9 +97,17 @@ function jsonChildToJSXChild(jsonChild) { jsxChildren === null ? jsonChild.props : {...jsonChild.props, children: jsxChildren}, - _owner: null, _store: __DEV__ ? {} : undefined, }; + + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + + return element; } } diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index c023a1feca632..852685a8732ab 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -379,15 +379,21 @@ function createElement(type, key, props): React$Element { key: key, ref: null, props: props, - - // Record the component responsible for creating this element. - _owner: null, }; if (__DEV__) { // We don't really need to add any of these but keeping them for good measure. // Unfortunately, _store is enumerable in jest matchers so for equality to // work, I need to keep it or make _store non-enumerable in the other file. element._store = {}; + + // Record the component responsible for creating this element. + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + Object.defineProperty(element._store, 'validated', { configurable: false, enumerable: false, diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 4df1b8dfc1e17..e489a0ae673d0 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -12,7 +12,6 @@ let React; let ReactDOM; let ReactDOMServer; -let ReactFeatureFlags; let ReactTestUtils; describe('ReactComponent', () => { @@ -22,7 +21,6 @@ describe('ReactComponent', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); }); @@ -104,83 +102,6 @@ describe('ReactComponent', () => { } }); - it('should support string refs on owned components', () => { - const innerObj = {}; - const outerObj = {}; - - class Wrapper extends React.Component { - getObject = () => { - return this.props.object; - }; - - render() { - return
{this.props.children}
; - } - } - - class Component extends React.Component { - render() { - const inner = ; - const outer = ( - - {inner} - - ); - return outer; - } - - componentDidMount() { - expect(this.refs.inner.getObject()).toEqual(innerObj); - expect(this.refs.outer.getObject()).toEqual(outerObj); - } - } - - expect(() => { - ReactTestUtils.renderIntoDocument(); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "div" 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://reactjs.org/link/strict-mode-string-ref\n' + - ' in div (at **)\n' + - ' in Wrapper (at **)\n' + - ' in Component (at **)', - 'Warning: Component "Component" contains the string ref "outer". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in Component (at **)', - ] - : [], - ); - }); - - it('should not have string refs on unmounted components', () => { - class Parent extends React.Component { - render() { - return ( - -
- - ); - } - - componentDidMount() { - expect(this.refs && this.refs.test).toEqual(undefined); - } - } - - class Child extends React.Component { - render() { - return
; - } - } - - ReactTestUtils.renderIntoDocument(} />); - }); - it('should support callback-style refs', () => { const innerObj = {}; const outerObj = {}; diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index 041dc9ebe7683..e296f37af96e5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -14,7 +14,6 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; -let ReactFeatureFlags; let ReactTestUtils; function initModules() { @@ -23,7 +22,6 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. @@ -36,7 +34,6 @@ function initModules() { const { resetModules, - asyncReactDOMRender, clientRenderOnServerString, expectMarkupMatch, } = ReactDOMServerIntegrationUtils(initModules); @@ -80,38 +77,6 @@ describe('ReactDOMServerIntegration', () => { expect(refElement).not.toBe(null); expect(refElement).toBe(e); }); - - it('should have string refs on client when rendered over server markup', async () => { - class RefsComponent extends React.Component { - render() { - return
; - } - } - - const markup = ReactDOMServer.renderToString(); - const root = document.createElement('div'); - root.innerHTML = markup; - let component = null; - resetModules(); - await expect(async () => { - await asyncReactDOMRender( - (component = e)} />, - root, - true, - ); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: 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://reactjs.org/link/strict-mode-string-ref\n' + - ' in RefsComponent (at **)', - ] - : [], - ); - expect(component.refs.myDiv).toBe(root.firstChild); - }); }); it('should forward refs', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js index a72c62cf7f442..5d70db4b4b2e1 100644 --- a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js @@ -13,7 +13,6 @@ let React; let ReactFeatureFlags; let ReactNoop; let Scheduler; -let JSXDEVRuntime; describe('ReactDeprecationWarnings', () => { beforeEach(() => { @@ -22,16 +21,11 @@ describe('ReactDeprecationWarnings', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); - if (__DEV__) { - JSXDEVRuntime = require('react/jsx-dev-runtime'); - } ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true; - ReactFeatureFlags.warnAboutStringRefs = true; }); afterEach(() => { ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false; - ReactFeatureFlags.warnAboutStringRefs = false; }); it('should warn when given defaultProps', () => { @@ -50,99 +44,4 @@ describe('ReactDeprecationWarnings', () => { 'release. Use JavaScript default parameters instead.', ); }); - - it('should warn when given string refs', () => { - class RefComponent extends React.Component { - render() { - return null; - } - } - class Component extends React.Component { - render() { - return ; - } - } - - ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - 'Warning: Component "Component" contains the string ref "refComponent". ' + - '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://reactjs.org/link/strict-mode-string-ref' + - '\n in Component (at **)', - ); - }); - - it('should not warn when owner and self are the same for string refs', () => { - ReactFeatureFlags.warnAboutStringRefs = false; - - class RefComponent extends React.Component { - render() { - return null; - } - } - class Component extends React.Component { - render() { - return ; - } - } - ReactNoop.renderLegacySyncRoot(); - expect(Scheduler).toFlushWithoutYielding(); - }); - - it('should warn when owner and self are different for string refs', () => { - class RefComponent extends React.Component { - render() { - return null; - } - } - class Component extends React.Component { - render() { - return ; - } - } - - ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([ - 'Warning: Component "Component" contains the string ref "refComponent". ' + - 'Support for string refs will be removed in a future major release. ' + - 'This case cannot be automatically converted to an arrow function. ' + - 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - ]); - }); - - if (__DEV__) { - it('should warn when owner and self are different for string refs', () => { - class RefComponent extends React.Component { - render() { - return null; - } - } - class Component extends React.Component { - render() { - return JSXDEVRuntime.jsxDEV( - RefComponent, - {ref: 'refComponent'}, - null, - false, - {}, - {}, - ); - } - } - - ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - 'Warning: Component "Component" contains the string ref "refComponent". ' + - 'Support for string refs will be removed in a future major release. ' + - 'This case cannot be automatically converted to an arrow function. ' + - 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - ); - }); - } }); diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js index 9f51cb5fba4f1..7b23f8e0dfb87 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -154,21 +154,10 @@ describe('ReactFunctionComponent', () => { expect(function() { ReactTestUtils.renderIntoDocument(); - }).toThrowError( - __DEV__ - ? 'Function components cannot have string refs. We recommend using useRef() instead.' - : // It happens because we don't save _owner in production for - // function components. - 'Element ref was specified as a string (me) but no owner was set. This could happen for one of' + - ' the following reasons:\n' + - '1. You may be adding a ref to a function component\n' + - "2. You may be adding a ref to a component that was not created inside a component's render method\n" + - '3. You have multiple copies of React loaded\n' + - 'See https://reactjs.org/link/refs-must-have-owner for more information.', - ); + }).toThrowError(); }); - it('should warn when given a string ref', () => { + it('should throw when given a string ref', () => { function Indirection(props) { return
{props.children}
; } @@ -185,20 +174,9 @@ describe('ReactFunctionComponent', () => { expect(() => ReactTestUtils.renderIntoDocument(), - ).toErrorDev( - 'Warning: Function components cannot be given refs. ' + - 'Attempts to access this ref will fail. ' + - 'Did you mean to use React.forwardRef()?\n\n' + - 'Check the render method ' + - 'of `ParentUsingStringRef`.\n' + - ' in FunctionComponent (at **)\n' + - ' in div (at **)\n' + - ' in Indirection (at **)\n' + - ' in ParentUsingStringRef (at **)', + ).toThrow( + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); - - // No additional warnings should be logged - ReactTestUtils.renderIntoDocument(); }); it('should warn when given a function ref', () => { diff --git a/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js b/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js index 1de130b2fed2d..295d1105d4974 100644 --- a/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js +++ b/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js @@ -25,12 +25,7 @@ describe('when different React version is used with string ref', () => { expect(() => { ReactTestUtils.renderIntoDocument(); }).toThrow( - 'Element ref was specified as a string (foo) but no owner was set. This could happen for one of' + - ' the following reasons:\n' + - '1. You may be adding a ref to a function component\n' + - "2. You may be adding a ref to a component that was not created inside a component's render method\n" + - '3. You have multiple copies of React loaded\n' + - 'See https://reactjs.org/link/refs-must-have-owner for more information.', + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); }); }); diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 9d1965c1a465a..e3d1ce5f9387b 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -14,179 +14,6 @@ let ReactDOM = require('react-dom'); let ReactFeatureFlags = require('shared/ReactFeatureFlags'); let ReactTestUtils = require('react-dom/test-utils'); -// This is testing if string refs are deleted from `instance.refs` -// Once support for string refs is removed, this test can be removed. -// Detaching is already tested in refs-detruction-test.js -describe('reactiverefs', () => { - let container; - - beforeEach(() => { - jest.resetModules(); - React = require('react'); - ReactDOM = require('react-dom'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactTestUtils = require('react-dom/test-utils'); - }); - - afterEach(() => { - if (container) { - document.body.removeChild(container); - container = null; - } - }); - - /** - * Counts clicks and has a renders an item for each click. Each item rendered - * has a ref of the form "clickLogN". - */ - class ClickCounter extends React.Component { - state = {count: this.props.initialCount}; - - triggerReset = () => { - this.setState({count: this.props.initialCount}); - }; - - handleClick = () => { - this.setState({count: this.state.count + 1}); - }; - - render() { - const children = []; - let i; - for (i = 0; i < this.state.count; i++) { - children.push( -
, - ); - } - return ( - - {children} - - ); - } - } - - const expectClickLogsLengthToBe = function(instance, length) { - const clickLogs = ReactTestUtils.scryRenderedDOMComponentsWithClass( - instance, - 'clickLogDiv', - ); - expect(clickLogs.length).toBe(length); - expect(Object.keys(instance.refs.myCounter.refs).length).toBe(length); - }; - - /** - * Render a TestRefsComponent and ensure that the main refs are wired up. - */ - const renderTestRefsComponent = function() { - /** - * Only purpose is to test that refs are tracked even when applied to a - * component that is injected down several layers. Ref systems are difficult to - * build in such a way that ownership is maintained in an airtight manner. - */ - class GeneralContainerComponent extends React.Component { - render() { - return
{this.props.children}
; - } - } - - /** - * Notice how refs ownership is maintained even when injecting a component - * into a different parent. - */ - class TestRefsComponent extends React.Component { - doReset = () => { - this.refs.myCounter.triggerReset(); - }; - - render() { - return ( -
-
- Reset Me By Clicking This. -
- - - -
- ); - } - } - - container = document.createElement('div'); - document.body.appendChild(container); - - let testRefsComponent; - expect(() => { - testRefsComponent = ReactDOM.render(, container); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "div" contains the string ref "resetDiv". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in div (at **)\n' + - ' in TestRefsComponent (at **)', - 'Warning: Component "span" contains the string ref "clickLog0". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in span (at **)\n' + - ' in ClickCounter (at **)\n' + - ' in div (at **)\n' + - ' in GeneralContainerComponent (at **)\n' + - ' in div (at **)\n' + - ' in TestRefsComponent (at **)', - ] - : [], - ); - - expect(testRefsComponent instanceof TestRefsComponent).toBe(true); - - const generalContainer = testRefsComponent.refs.myContainer; - expect(generalContainer instanceof GeneralContainerComponent).toBe(true); - - const counter = testRefsComponent.refs.myCounter; - expect(counter instanceof ClickCounter).toBe(true); - - return testRefsComponent; - }; - - /** - * Ensure that for every click log there is a corresponding ref (from the - * perspective of the injected ClickCounter component. - */ - it('Should increase refs with an increase in divs', () => { - const testRefsComponent = renderTestRefsComponent(); - const clickIncrementer = ReactTestUtils.findRenderedDOMComponentWithClass( - testRefsComponent, - 'clickIncrementer', - ); - - expectClickLogsLengthToBe(testRefsComponent, 1); - - // After clicking the reset, there should still only be one click log ref. - testRefsComponent.refs.resetDiv.click(); - expectClickLogsLengthToBe(testRefsComponent, 1); - - // Begin incrementing clicks (and therefore refs). - clickIncrementer.click(); - expectClickLogsLengthToBe(testRefsComponent, 2); - - clickIncrementer.click(); - expectClickLogsLengthToBe(testRefsComponent, 3); - - // Now reset again - testRefsComponent.refs.resetDiv.click(); - expectClickLogsLengthToBe(testRefsComponent, 1); - }); -}); - if (!ReactFeatureFlags.disableModulePatternComponents) { describe('factory components', () => { it('Should correctly get the ref', () => { @@ -340,44 +167,34 @@ describe('ref swapping', () => { expect(refCalled).toBe(1); }); - it('coerces numbers to strings', () => { + it('throws on numbers', () => { class A extends React.Component { render() { return
; } } - let a; expect(() => { - a = ReactTestUtils.renderIntoDocument(); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "A" contains the string ref "1". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in A (at **)', - ] - : [], + ReactTestUtils.renderIntoDocument(); + }).toThrow( + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); - expect(a.refs[1].nodeName).toBe('DIV'); }); it('provides an error for invalid refs', () => { expect(() => { ReactTestUtils.renderIntoDocument(
); }).toThrow( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); expect(() => { ReactTestUtils.renderIntoDocument(
); }).toThrow( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); expect(() => { ReactTestUtils.renderIntoDocument(
); }).toThrow( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); // This works ReactTestUtils.renderIntoDocument(
); @@ -398,7 +215,7 @@ describe('ref swapping', () => { ref: undefined, }); }).toThrow( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', + 'Expected ref to be a function, an object returned by React.createRef(), or null.', ); }); @@ -530,85 +347,9 @@ describe('creating element with string ref in constructor', () => { ReactTestUtils = require('react-dom/test-utils'); expect(function() { - ReactTestUtils.renderIntoDocument(); - }).toThrowError( - 'Element ref was specified as a string (p) but no owner was set. This could happen for one of' + - ' the following reasons:\n' + - '1. You may be adding a ref to a function component\n' + - "2. You may be adding a ref to a component that was not created inside a component's render method\n" + - '3. You have multiple copies of React loaded\n' + - 'See https://reactjs.org/link/refs-must-have-owner for more information.', - ); - }); -}); - -describe('strings refs across renderers', () => { - it('does not break', () => { - class Parent extends React.Component { - render() { - // This component owns both refs. - return ( - } - child2={
} - /> - ); - } - } - - class Indirection extends React.Component { - componentDidUpdate() { - // One ref is being rendered later using another renderer copy. - jest.resetModules(); - const AnotherCopyOfReactDOM = require('react-dom'); - AnotherCopyOfReactDOM.render(this.props.child2, div2); - } - render() { - // The other one is being rendered directly. - return this.props.child1; - } - } - - const div1 = document.createElement('div'); - const div2 = document.createElement('div'); - - let inst; - expect(() => { - inst = ReactDOM.render(, div1); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "Indirection" contains the string ref "child1". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in Indirection (at **)\n' + - ' in Parent (at **)', - ] - : [], - ); - - // Only the first ref has rendered yet. - expect(inst.refs.child1.tagName).toBe('DIV'); - expect(inst.refs.child1).toBe(div1.firstChild); - - expect(() => { - // Now both refs should be rendered. - ReactDOM.render(, div1); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "Root" contains the string ref "child2". ' + - '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://reactjs.org/link/strict-mode-string-ref', - ] - : [], - {withoutStack: true}, - ); - expect(inst.refs.child1.tagName).toBe('DIV'); - expect(inst.refs.child1).toBe(div1.firstChild); - expect(inst.refs.child2.tagName).toBe('DIV'); - expect(inst.refs.child2).toBe(div2.firstChild); + expect(() => { + ReactTestUtils.renderIntoDocument(); + }).toErrorDev(['']); + }).toThrowError(); }); }); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 5555657e3589c..7b0880a1007bb 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -692,15 +692,25 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { if (children !== null) { props.children = children; } - return { + const element = { $$typeof: REACT_ELEMENT_TYPE, type: instance.type, key: null, ref: null, props: props, - _owner: null, _store: __DEV__ ? {} : undefined, }; + + if (__DEV__) { + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + } + + return element; } // This is a text instance const textInstance: TextInstance = (child: any); @@ -732,15 +742,25 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return null; } if (isArray(children)) { - return { + const element = { $$typeof: REACT_ELEMENT_TYPE, type: REACT_FRAGMENT_TYPE, key: null, ref: null, props: {children}, - _owner: null, _store: __DEV__ ? {} : undefined, }; + + if (__DEV__) { + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + } + + return element; } return children; } @@ -751,15 +771,24 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return null; } if (isArray(children)) { - return { + const element = { $$typeof: REACT_ELEMENT_TYPE, type: REACT_FRAGMENT_TYPE, key: null, ref: null, props: {children}, - _owner: null, _store: __DEV__ ? {} : undefined, }; + if (__DEV__) { + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + } + + return element; } return children; } diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index ea72a3de92047..09342db54d2c0 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -26,10 +26,8 @@ import { REACT_PORTAL_TYPE, REACT_LAZY_TYPE, } from 'shared/ReactSymbols'; -import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags'; +import {HostText, HostPortal, Fragment} from './ReactWorkTags'; import isArray from 'shared/isArray'; -import {warnAboutStringRefs} from 'shared/ReactFeatureFlags'; -import {checkPropStringCoercion} from 'shared/CheckStringCoercion'; import { createWorkInProgress, @@ -40,13 +38,11 @@ import { createFiberFromPortal, } from './ReactFiber.new'; import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading.new'; -import {StrictLegacyMode} from './ReactTypeOfMode'; import {getIsHydrating} from './ReactFiberHydrationContext.new'; import {pushTreeFork} from './ReactFiberTreeContext.new'; let didWarnAboutMaps; let didWarnAboutGenerators; -let didWarnAboutStringRefs; let ownerHasKeyUseWarning; let ownerHasFunctionTypeWarning; let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {}; @@ -54,7 +50,6 @@ let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {}; if (__DEV__) { didWarnAboutMaps = false; didWarnAboutGenerators = false; - didWarnAboutStringRefs = {}; /** * Warn if there's no key explicitly set on dynamic arrays of children or @@ -97,10 +92,6 @@ if (__DEV__) { }; } -function isReactClass(type) { - return type.prototype && type.prototype.isReactComponent; -} - function coerceRef( returnFiber: Fiber, current: Fiber | null, @@ -112,125 +103,9 @@ function coerceRef( typeof mixedRef !== 'function' && typeof mixedRef !== 'object' ) { - if (__DEV__) { - // TODO: Clean this up once we turn on the string ref warning for - // everyone, because the strict mode case will no longer be relevant - if ( - (returnFiber.mode & StrictLegacyMode || warnAboutStringRefs) && - // We warn in ReactElement.js if owner and self are equal for string refs - // because these cannot be automatically converted to an arrow function - // using a codemod. Therefore, we don't have to warn about string refs again. - !( - element._owner && - element._self && - element._owner.stateNode !== element._self - ) && - // Will already throw with "Function components cannot have string refs" - !( - element._owner && - ((element._owner: any): Fiber).tag !== ClassComponent - ) && - // Will already warn with "Function components cannot be given refs" - !(typeof element.type === 'function' && !isReactClass(element.type)) && - // Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set" - element._owner - ) { - const componentName = - getComponentNameFromFiber(returnFiber) || 'Component'; - if (!didWarnAboutStringRefs[componentName]) { - if (warnAboutStringRefs) { - console.error( - 'Component "%s" contains the string ref "%s". 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://reactjs.org/link/strict-mode-string-ref', - componentName, - mixedRef, - ); - } else { - console.error( - 'A string ref, "%s", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - mixedRef, - ); - } - didWarnAboutStringRefs[componentName] = true; - } - } - } - - if (element._owner) { - const owner: ?Fiber = (element._owner: any); - let inst; - if (owner) { - const ownerFiber = ((owner: any): Fiber); - - if (ownerFiber.tag !== ClassComponent) { - throw new Error( - 'Function components cannot have string refs. ' + - 'We recommend using useRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - ); - } - - inst = ownerFiber.stateNode; - } - - if (!inst) { - throw new Error( - `Missing owner for string ref ${mixedRef}. This error is likely caused by a ` + - 'bug in React. Please file an issue.', - ); - } - // Assigning this to a const so Flow knows it won't change in the closure - const resolvedInst = inst; - - if (__DEV__) { - checkPropStringCoercion(mixedRef, 'ref'); - } - const stringRef = '' + mixedRef; - // Check if previous string ref matches new string ref - if ( - current !== null && - current.ref !== null && - typeof current.ref === 'function' && - current.ref._stringRef === stringRef - ) { - return current.ref; - } - const ref = function(value) { - const refs = resolvedInst.refs; - if (value === null) { - delete refs[stringRef]; - } else { - refs[stringRef] = value; - } - }; - ref._stringRef = stringRef; - return ref; - } else { - if (typeof mixedRef !== 'string') { - throw new Error( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', - ); - } - - if (!element._owner) { - throw new Error( - `Element ref was specified as a string (${mixedRef}) but no owner was set. This could happen for one of` + - ' the following reasons:\n' + - '1. You may be adding a ref to a function component\n' + - "2. You may be adding a ref to a component that was not created inside a component's render method\n" + - '3. You have multiple copies of React loaded\n' + - 'See https://reactjs.org/link/refs-must-have-owner for more information.', - ); - } - } + throw new Error( + 'Expected ref to be a function, an object returned by React.createRef(), or null.', + ); } return mixedRef; } diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index 9ced326c55960..494cf0e17b6bb 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -26,10 +26,8 @@ import { REACT_PORTAL_TYPE, REACT_LAZY_TYPE, } from 'shared/ReactSymbols'; -import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags'; +import {HostText, HostPortal, Fragment} from './ReactWorkTags'; import isArray from 'shared/isArray'; -import {warnAboutStringRefs} from 'shared/ReactFeatureFlags'; -import {checkPropStringCoercion} from 'shared/CheckStringCoercion'; import { createWorkInProgress, @@ -40,13 +38,11 @@ import { createFiberFromPortal, } from './ReactFiber.old'; import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading.old'; -import {StrictLegacyMode} from './ReactTypeOfMode'; import {getIsHydrating} from './ReactFiberHydrationContext.old'; import {pushTreeFork} from './ReactFiberTreeContext.old'; let didWarnAboutMaps; let didWarnAboutGenerators; -let didWarnAboutStringRefs; let ownerHasKeyUseWarning; let ownerHasFunctionTypeWarning; let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {}; @@ -54,7 +50,6 @@ let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {}; if (__DEV__) { didWarnAboutMaps = false; didWarnAboutGenerators = false; - didWarnAboutStringRefs = {}; /** * Warn if there's no key explicitly set on dynamic arrays of children or @@ -97,10 +92,6 @@ if (__DEV__) { }; } -function isReactClass(type) { - return type.prototype && type.prototype.isReactComponent; -} - function coerceRef( returnFiber: Fiber, current: Fiber | null, @@ -112,125 +103,9 @@ function coerceRef( typeof mixedRef !== 'function' && typeof mixedRef !== 'object' ) { - if (__DEV__) { - // TODO: Clean this up once we turn on the string ref warning for - // everyone, because the strict mode case will no longer be relevant - if ( - (returnFiber.mode & StrictLegacyMode || warnAboutStringRefs) && - // We warn in ReactElement.js if owner and self are equal for string refs - // because these cannot be automatically converted to an arrow function - // using a codemod. Therefore, we don't have to warn about string refs again. - !( - element._owner && - element._self && - element._owner.stateNode !== element._self - ) && - // Will already throw with "Function components cannot have string refs" - !( - element._owner && - ((element._owner: any): Fiber).tag !== ClassComponent - ) && - // Will already warn with "Function components cannot be given refs" - !(typeof element.type === 'function' && !isReactClass(element.type)) && - // Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set" - element._owner - ) { - const componentName = - getComponentNameFromFiber(returnFiber) || 'Component'; - if (!didWarnAboutStringRefs[componentName]) { - if (warnAboutStringRefs) { - console.error( - 'Component "%s" contains the string ref "%s". 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://reactjs.org/link/strict-mode-string-ref', - componentName, - mixedRef, - ); - } else { - console.error( - 'A string ref, "%s", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - mixedRef, - ); - } - didWarnAboutStringRefs[componentName] = true; - } - } - } - - if (element._owner) { - const owner: ?Fiber = (element._owner: any); - let inst; - if (owner) { - const ownerFiber = ((owner: any): Fiber); - - if (ownerFiber.tag !== ClassComponent) { - throw new Error( - 'Function components cannot have string refs. ' + - 'We recommend using useRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - ); - } - - inst = ownerFiber.stateNode; - } - - if (!inst) { - throw new Error( - `Missing owner for string ref ${mixedRef}. This error is likely caused by a ` + - 'bug in React. Please file an issue.', - ); - } - // Assigning this to a const so Flow knows it won't change in the closure - const resolvedInst = inst; - - if (__DEV__) { - checkPropStringCoercion(mixedRef, 'ref'); - } - const stringRef = '' + mixedRef; - // Check if previous string ref matches new string ref - if ( - current !== null && - current.ref !== null && - typeof current.ref === 'function' && - current.ref._stringRef === stringRef - ) { - return current.ref; - } - const ref = function(value) { - const refs = resolvedInst.refs; - if (value === null) { - delete refs[stringRef]; - } else { - refs[stringRef] = value; - } - }; - ref._stringRef = stringRef; - return ref; - } else { - if (typeof mixedRef !== 'string') { - throw new Error( - 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', - ); - } - - if (!element._owner) { - throw new Error( - `Element ref was specified as a string (${mixedRef}) but no owner was set. This could happen for one of` + - ' the following reasons:\n' + - '1. You may be adding a ref to a function component\n' + - "2. You may be adding a ref to a component that was not created inside a component's render method\n" + - '3. You have multiple copies of React loaded\n' + - 'See https://reactjs.org/link/refs-must-have-owner for more information.', - ); - } - } + throw new Error( + 'Expected ref to be a function, an object returned by React.createRef(), or null.', + ); } return mixedRef; } diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index a5f28fd5d2450..d13c7c549038e 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -126,10 +126,7 @@ export type Fiber = { // The ref last used to attach this node. // I'll avoid adding an owner field for prod and model that as functions. - ref: - | null - | (((handle: mixed) => void) & {_stringRef: ?string, ...}) - | RefObject, + ref: null | ((handle: mixed) => void) | RefObject, // Input is the data coming into process this fiber. Arguments. Props. pendingProps: any, // This type will be more specific once we overload the tag. diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js index 6bac75ab30f48..5a19f4bac4fb2 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js @@ -11,7 +11,6 @@ 'use strict'; let React; -let ReactFeatureFlags; let ReactNoop; let Scheduler; @@ -20,7 +19,6 @@ describe('ReactIncrementalSideEffects', () => { jest.resetModules(); React = require('react'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); }); @@ -1287,40 +1285,4 @@ describe('ReactIncrementalSideEffects', () => { // TODO: Test that mounts, updates, refs, unmounts and deletions happen in the // expected way for aborted and resumed render life-cycles. - - it('supports string refs', () => { - let fooInstance = null; - - class Bar extends React.Component { - componentDidMount() { - this.test = 'test'; - } - render() { - return
; - } - } - - class Foo extends React.Component { - render() { - fooInstance = this; - return ; - } - } - - ReactNoop.render(); - expect(() => { - expect(Scheduler).toFlushWithoutYielding(); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "Foo" contains the string ref "bar". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in Foo (at **)', - ] - : [], - ); - expect(fooInstance.refs.bar.test).toEqual('test'); - }); }); diff --git a/packages/react/src/ReactElement.js b/packages/react/src/ReactElement.js index bd15c85fdfd48..c344adb536a19 100644 --- a/packages/react/src/ReactElement.js +++ b/packages/react/src/ReactElement.js @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import getComponentNameFromType from 'shared/getComponentNameFromType'; import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols'; import assign from 'shared/assign'; import hasOwnProperty from 'shared/hasOwnProperty'; @@ -20,13 +19,7 @@ const RESERVED_PROPS = { __source: true, }; -let specialPropKeyWarningShown, - specialPropRefWarningShown, - didWarnAboutStringRefs; - -if (__DEV__) { - didWarnAboutStringRefs = {}; -} +let specialPropKeyWarningShown, specialPropRefWarningShown; function hasValidRef(config) { if (__DEV__) { @@ -96,35 +89,6 @@ function defineRefPropWarningGetter(props, displayName) { }); } -function warnIfStringRefCannotBeAutoConverted(config) { - if (__DEV__) { - if ( - typeof config.ref === 'string' && - ReactCurrentOwner.current && - config.__self && - ReactCurrentOwner.current.stateNode !== config.__self - ) { - const componentName = getComponentNameFromType( - ReactCurrentOwner.current.type, - ); - - if (!didWarnAboutStringRefs[componentName]) { - console.error( - 'Component "%s" contains the string ref "%s". ' + - 'Support for string refs will be removed in a future major release. ' + - 'This case cannot be automatically converted to an arrow function. ' + - 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref', - componentName, - config.ref, - ); - didWarnAboutStringRefs[componentName] = true; - } - } - } -} - /** * Factory method to create a new React element. This no longer adheres to * the class pattern, so do not use new to call it. Also, instanceof check @@ -155,9 +119,6 @@ const ReactElement = function(type, key, ref, self, source, owner, props) { key: key, ref: ref, props: props, - - // Record the component responsible for creating this element. - _owner: owner, }; if (__DEV__) { @@ -167,6 +128,14 @@ const ReactElement = function(type, key, ref, self, source, owner, props) { // commonly used development environments. element._store = {}; + // Record the component responsible for creating this element. + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: owner, + }); + // To make comparing ReactElements easier for testing purposes, we make // the validation flag non-enumerable (where possible, which should // include every environment we run tests in), so the test framework @@ -308,7 +277,6 @@ export function jsxDEV(type, config, maybeKey, source, self) { if (hasValidRef(config)) { ref = config.ref; - warnIfStringRefCannotBeAutoConverted(config); } // Remaining properties are added to a new props object @@ -373,10 +341,6 @@ export function createElement(type, config, children) { if (config != null) { if (hasValidRef(config)) { ref = config.ref; - - if (__DEV__) { - warnIfStringRefCannotBeAutoConverted(config); - } } if (hasValidKey(config)) { if (__DEV__) { @@ -506,13 +470,18 @@ export function cloneElement(element, config, children) { const source = element._source; // Owner will be preserved, unless ref is overridden - let owner = element._owner; + let owner; + if (__DEV__) { + owner = element._owner; + } if (config != null) { if (hasValidRef(config)) { // Silently steal the ref from the parent. ref = config.ref; - owner = ReactCurrentOwner.current; + if (__DEV__) { + owner = ReactCurrentOwner.current; + } } if (hasValidKey(config)) { if (__DEV__) { diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index b67f52fe9f785..4c4dd5efcda08 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -134,6 +134,7 @@ function validateExplicitKey(element, parentType) { // assigning it a key. let childOwner = ''; if ( + __DEV__ && element && element._owner && element._owner !== ReactCurrentOwner.current diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index c9dee4ac6c7fa..721f734c4ac0d 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -9,7 +9,6 @@ PropTypes = null React = null ReactDOM = null ReactDOMClient = null -ReactFeatureFlags = null act = null describe 'ReactCoffeeScriptClass', -> @@ -23,7 +22,6 @@ describe 'ReactCoffeeScriptClass', -> React = require 'react' ReactDOM = require 'react-dom' ReactDOMClient = require 'react-dom/client' - ReactFeatureFlags = require 'shared/ReactFeatureFlags' act = require('jest-react').act PropTypes = require 'prop-types' container = document.createElement 'div' @@ -530,29 +528,19 @@ describe 'ReactCoffeeScriptClass', -> test React.createElement(Foo), 'DIV', 'bar-through-context' - it 'supports string refs', -> + it 'supports refs', -> class Foo extends React.Component + innerRef: React.createRef() render: -> React.createElement(InnerComponent, name: 'foo' - ref: 'inner' + ref: this.innerRef ) ref = React.createRef() - expect(-> - test(React.createElement(Foo, ref: ref), 'DIV', 'foo') - ).toErrorDev( - if ReactFeatureFlags.warnAboutStringRefs - then [ - 'Warning: Component "Foo" 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://reactjs.org/link/strict-mode-string-ref\n' + - ' in Foo (at **)' - ] - else [] - ); - expect(ref.current.refs.inner.getName()).toBe 'foo' + test(React.createElement(Foo, ref: ref), 'DIV', 'foo') + + expect(ref.current.innerRef.current.getName()).toBe 'foo' it 'supports drilling through to the DOM using findDOMNode', -> ref = React.createRef() diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index 53a44f9fb17df..cc888995aadac 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -13,7 +13,6 @@ let PropTypes; let React; let ReactDOM; let ReactDOMClient; -let ReactFeatureFlags; let act; describe('ReactES6Class', () => { @@ -32,7 +31,6 @@ describe('ReactES6Class', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); act = require('jest-react').act; container = document.createElement('div'); root = ReactDOMClient.createRoot(container); @@ -570,27 +568,17 @@ describe('ReactES6Class', () => { test(, 'DIV', 'bar-through-context'); }); - it('supports string refs', () => { + it('supports refs', () => { class Foo extends React.Component { + innerRef = React.createRef(); render() { - return ; + return ; } } const ref = React.createRef(); - expect(() => { - test(, 'DIV', 'foo'); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "Foo" 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://reactjs.org/link/strict-mode-string-ref\n' + - ' in Foo (at **)', - ] - : [], - ); - expect(ref.current.refs.inner.getName()).toBe('foo'); + test(, 'DIV', 'foo'); + + expect(ref.current.innerRef.current.getName()).toBe('foo'); }); it('supports drilling through to the DOM using findDOMNode', () => { diff --git a/packages/react/src/__tests__/ReactElement-test.js b/packages/react/src/__tests__/ReactElement-test.js index f1e9cffb8424d..bd280747839d0 100644 --- a/packages/react/src/__tests__/ReactElement-test.js +++ b/packages/react/src/__tests__/ReactElement-test.js @@ -212,6 +212,7 @@ describe('ReactElement', () => { expect(element.props).toEqual({foo: '56'}); }); + // @gate __DEV__ it('preserves the owner on the element', () => { let element; diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index e0150cc4aa0dc..1de10a2a9f496 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -513,9 +513,18 @@ describe('ReactElementValidator', () => { key: null, ref: null, props: {}, - _owner: null, }; + if (__DEV__) { + // Record the component responsible for creating this element. + Object.defineProperty(child, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: null, + }); + } + void (
{[child]}
); }); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index b41497ae3ab72..7a40b150fecd8 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -895,105 +895,6 @@ describe('symbol checks', () => { }); }); -describe('string refs', () => { - beforeEach(() => { - jest.resetModules(); - React = require('react'); - ReactDOM = require('react-dom'); - ReactDOMClient = require('react-dom/client'); - }); - - it('should warn within a strict tree', () => { - const {StrictMode} = React; - - class OuterComponent extends React.Component { - render() { - return ( - - - - ); - } - } - - class InnerComponent extends React.Component { - render() { - return null; - } - } - - const container = document.createElement('div'); - expect(() => { - ReactDOM.render(, container); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? 'Warning: Component "StrictMode" contains the string ref "somestring". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in OuterComponent (at **)' - : 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref\n' + - ' in OuterComponent (at **)', - ); - - // Dedup - ReactDOM.render(, container); - }); - - it('should warn within a strict tree', () => { - const {StrictMode} = React; - - class OuterComponent extends React.Component { - render() { - return ( - - - - ); - } - } - - class InnerComponent extends React.Component { - render() { - return ; - } - } - - class MiddleComponent extends React.Component { - render() { - return null; - } - } - - const container = document.createElement('div'); - expect(() => { - ReactDOM.render(, container); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? 'Warning: Component "InnerComponent" contains the string ref "somestring". ' + - '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://reactjs.org/link/strict-mode-string-ref\n' + - ' in InnerComponent (at **)\n' + - ' in OuterComponent (at **)' - : 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref\n' + - ' in InnerComponent (at **)\n' + - ' in OuterComponent (at **)', - ); - - // Dedup - ReactDOM.render(, container); - }); -}); - describe('context legacy', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index 39304e9d6f2af..59045542827c2 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -308,10 +308,11 @@ class ProvideContext extends React.Component { } } -// it supports classic refs -class ClassicRefs extends React.Component { +// it supports refs +class Refs extends React.Component { + innerRef = React.createRef() render() { - return React.createElement(Inner, {name: 'foo', ref: 'inner'}); + return React.createElement(Inner, {name: 'foo', ref: this.innerRef}); } } @@ -687,22 +688,10 @@ describe('ReactTypeScriptClass', function() { test(React.createElement(ProvideContext), 'DIV', 'bar-through-context'); }); - it('supports string refs', function() { + it('supports refs', function() { const ref = React.createRef(); - expect(() => { - test(React.createElement(ClassicRefs, {ref: ref}), 'DIV', 'foo'); - }).toErrorDev( - ReactFeatureFlags.warnAboutStringRefs - ? [ - 'Warning: Component "ClassicRefs" 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://reactjs.org/link/strict-mode-string-ref\n' + - ' in ClassicRefs (at **)', - ] - : [], - ); - expect(ref.current.refs.inner.getName()).toBe('foo'); + test(React.createElement(Refs, {ref: ref}), 'DIV', 'foo'); + expect(ref.current.innerRef.current.getName()).toBe('foo'); }); it('supports drilling through to the DOM using findDOMNode', function() { diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 47d9cec8f3e42..70f19e31c02f6 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -155,9 +155,6 @@ const ReactElement = function(type, key, ref, self, source, owner, props) { key, ref, props, - - // Record the component responsible for creating this element. - _owner: owner, }; if (__DEV__) { @@ -167,6 +164,14 @@ const ReactElement = function(type, key, ref, self, source, owner, props) { // commonly used development environments. element._store = {}; + // Record the component responsible for creating this element. + Object.defineProperty(element, '_owner', { + configurable: false, + enumerable: false, + writable: false, + value: owner, + }); + // To make comparing ReactElements easier for testing purposes, we make // the validation flag non-enumerable (where possible, which should // include every environment we run tests in), so the test framework diff --git a/packages/shared/ReactElementType.js b/packages/shared/ReactElementType.js index 7507eb9e1c6e5..1d33330918db9 100644 --- a/packages/shared/ReactElementType.js +++ b/packages/shared/ReactElementType.js @@ -18,11 +18,10 @@ export type ReactElement = { key: any, ref: any, props: any, - // ReactFiber - _owner: any, // __DEV__ _store: {validated: boolean, ...}, + _owner: any, _self: React$Element, _shadowChildren: any, _source: Source, diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3a52dfd100a5c..d11f6279597ac 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -219,8 +219,6 @@ export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate lat // a deprecated pattern we want to get rid of in the future export const warnAboutSpreadingKeyToJSX = true; -export const warnAboutStringRefs = true; - // ----------------------------------------------------------------------------- // Debugging and DevTools // ----------------------------------------------------------------------------- diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f14d33de631d5..6967b9141119b 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -42,7 +42,6 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 296d4012ab2f9..8b972d5cc657c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -32,7 +32,6 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 86f0d54eea169..2f14e55bc8ff4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -32,7 +32,6 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index f734afa27227d..588602dfdd810 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -32,7 +32,6 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 7ec00cbb2e9a9..6f9a2c8f7c636 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -32,7 +32,6 @@ export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index efe5ef51e26ab..4b96fb2087bc4 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -32,7 +32,6 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index f9e6b26567c31..fe4ed60c646d0 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -32,7 +32,6 @@ export const enableScopeAPI = true; export const enableCreateEventHandleAPI = true; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = true; export const disableLegacyContext = __EXPERIMENTAL__; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index b0516f7f498b7..ac10ba62c883a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -66,7 +66,6 @@ export const enableSchedulingProfiler: boolean = export const enableSchedulerDebugging = true; export const warnAboutDeprecatedLifecycles = true; export const disableLegacyContext = __EXPERIMENTAL__; -export const warnAboutStringRefs = true; export const warnAboutDefaultPropsOnFunctionComponents = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 4a3a856d15eaa..3c6dd5d429853 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -445,5 +445,6 @@ "457": "acquireHeadResource encountered a resource type it did not expect: \"%s\". This is a bug in React.", "458": "Currently React only supports one RSC renderer at a time.", "459": "Expected a suspended thenable. This is a bug in React. Please file an issue.", - "460": "Suspense Exception: This is not a real error! It's an implementation detail of `use` to interrupt the current render. You must either rethrow it immediately, or move the `use` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary, or call the promise's `.catch` method and pass the result to `use`" + "460": "Suspense Exception: This is not a real error! It's an implementation detail of `use` to interrupt the current render. You must either rethrow it immediately, or move the `use` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary, or call the promise's `.catch` method and pass the result to `use`", + "461": "Expected ref to be a function, an object returned by React.createRef(), or null." }