From 3fdfc4a32011cb6dc97143cd6cd3172aaf88c175 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 29 Feb 2024 13:31:49 -0500 Subject: [PATCH] Move string ref coercion to JSX runtime This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless. --- .../src/__tests__/ReactComponent-test.js | 17 +- .../ReactDOMServerIntegrationRefs-test.js | 1 + .../ReactDeprecationWarnings-test.js | 9 +- .../__tests__/ReactFunctionComponent-test.js | 16 +- .../multiple-copies-of-react-test.js | 12 +- packages/react-dom/src/__tests__/refs-test.js | 73 ++++---- .../react-reconciler/src/ReactChildFiber.js | 156 +----------------- .../ReactIncrementalSideEffects-test.js | 1 + .../ReactCoffeeScriptClass-test.coffee | 2 +- .../react/src/__tests__/ReactES6Class-test.js | 2 +- .../src/__tests__/ReactElementClone-test.js | 10 +- .../src/__tests__/ReactStrictMode-test.js | 8 +- .../__tests__/ReactTypeScriptClass-test.ts | 2 +- packages/react/src/jsx/ReactJSXElement.js | 137 ++++++++++++++- 14 files changed, 200 insertions(+), 246 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 1a77cbad680af..f93c52462a960 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -40,17 +40,16 @@ describe('ReactComponent', () => { }); // @gate !disableStringRefs - it('should throw when supplying a string ref outside of render method', async () => { + fit('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(
); }), - ).rejects.toThrow( - 'Element ref was specified as a string (badDiv) but no owner ' + - 'was set', - ); + // 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 () => { @@ -169,18 +168,14 @@ describe('ReactComponent', () => { root.render(); }); }).toErrorDev([ - 'Warning: Component "div" contains the string ref "inner". ' + + 'Warning: 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 **)', - '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://react.dev/link/strict-mode-string-ref\n' + - ' in Component (at **)', ]); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index b7179d55bda21..4278e32984418 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -100,6 +100,7 @@ describe('ReactDOMServerIntegration', () => { '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); diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js index 39a068e1c7ed8..009ae14fccaa5 100644 --- a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js +++ b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js @@ -86,6 +86,7 @@ describe('ReactDeprecationWarnings', () => { 'We recommend using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + 'https://react.dev/link/strict-mode-string-ref' + + '\n in RefComponent (at **)' + '\n in Component (at **)', ); }); @@ -137,10 +138,6 @@ describe('ReactDeprecationWarnings', () => { 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + 'https://react.dev/link/strict-mode-string-ref', - '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://react.dev/link/strict-mode-string-ref', ]); }); @@ -173,10 +170,6 @@ describe('ReactDeprecationWarnings', () => { 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + 'https://react.dev/link/strict-mode-string-ref', - '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://react.dev/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 d353f6f8c6ee6..813cc9fb6e67a 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -185,18 +185,10 @@ describe('ReactFunctionComponent', () => { act(() => { root.render(); }), - ).rejects.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://react.dev/link/refs-must-have-owner for more information.', - ); + ) + // TODO: This throws an AggregateError. Need to update test infra to + // support matching against AggregateError. + .rejects.toThrowError(); }); // @gate !enableRefAsProp || !__DEV__ 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 0d63137b1ab74..2d107cbb19700 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 @@ -30,13 +30,9 @@ describe('when different React version is used with string ref', () => { act(() => { root.render(); }), - ).rejects.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://react.dev/link/refs-must-have-owner for more information.', - ); + ) + // TODO: This throws an AggregateError. Need to update test infra to + // support matching against AggregateError. + .rejects.toThrow(); }); }); diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 4a638ef17c566..a24bdf481e16f 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -129,22 +129,22 @@ describe('reactiverefs', () => { ); }); }).toErrorDev([ - '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://react.dev/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://react.dev/link/strict-mode-string-ref\n' + - ' in span (at **)\n' + - ' in ClickCounter (at **)\n' + + 'Warning: Component "TestRefsComponent" 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://react.dev/link/strict-mode-string-ref\n' + ' in div (at **)\n' + - ' in GeneralContainerComponent (at **)\n' + ' in div (at **)\n' + ' in TestRefsComponent (at **)', + 'Warning: Component "ClickCounter" 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://react.dev/link/strict-mode-string-ref\n' + + ' in div (at **)\n' + + ' in span (at **)\n' + + ' in ClickCounter (at **)', ]); expect(testRefsComponent instanceof TestRefsComponent).toBe(true); @@ -352,12 +352,12 @@ describe('ref swapping', () => { '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 A (at **)', ]); expect(a.refs[1].nodeName).toBe('DIV'); }); - // @gate !disableStringRefs it('provides an error for invalid refs', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -365,16 +365,16 @@ describe('ref swapping', () => { await act(() => { root.render(
); }); - }).rejects.toThrow( - 'Element ref was specified as a string (10) but no owner was set.', - ); + // TODO: This throws an AggregateError. Need to update test infra to + // support matching against AggregateError. + }).rejects.toThrow(); await expect(async () => { await act(() => { root.render(
); }); - }).rejects.toThrow( - 'Element ref was specified as a string (true) but no owner was set.', - ); + // TODO: This throws an AggregateError. Need to update test infra to + // support matching against AggregateError. + }).rejects.toThrow(); await expect(async () => { await act(() => { root.render(
); @@ -520,14 +520,10 @@ describe('creating element with string ref in constructor', () => { await act(() => { root.render(); }); - }).rejects.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://react.dev/link/refs-must-have-owner for more information.', - ); + }) + // TODO: This throws an AggregateError. Need to update test infra to + // support matching against AggregateError. + .rejects.toThrowError(); }); }); @@ -581,10 +577,11 @@ describe('strings refs across renderers', () => { ); }); }).toErrorDev([ - 'Warning: Component "Indirection" contains the string ref "child1". ' + + 'Warning: Component "Parent" 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://react.dev/link/strict-mode-string-ref\n' + + ' in div (at **)\n' + ' in Indirection (at **)\n' + ' in Parent (at **)', ]); @@ -593,20 +590,10 @@ describe('strings refs across renderers', () => { expect(inst.refs.child1.tagName).toBe('DIV'); expect(inst.refs.child1).toBe(div1.firstChild); - await expect(async () => { - // Now both refs should be rendered. - await act(() => { - root.render(); - }); - }).toErrorDev( - [ - '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://react.dev/link/strict-mode-string-ref', - ], - {withoutStack: true}, - ); + // Now both refs should be rendered. + await act(() => { + root.render(); + }); expect(inst.refs.child1.tagName).toBe('DIV'); expect(inst.refs.child1).toBe(div1.firstChild); expect(inst.refs.child2.tagName).toBe('DIV'); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index dde892c5893e9..742206c47fbf5 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -33,17 +33,9 @@ import { REACT_LAZY_TYPE, REACT_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import { - ClassComponent, - HostRoot, - HostText, - HostPortal, - Fragment, -} from './ReactWorkTags'; +import {HostRoot, HostText, HostPortal, Fragment} from './ReactWorkTags'; import isArray from 'shared/isArray'; -import assign from 'shared/assign'; -import {checkPropStringCoercion} from 'shared/CheckStringCoercion'; -import {enableRefAsProp, disableStringRefs} from 'shared/ReactFeatureFlags'; +import {enableRefAsProp} from 'shared/ReactFeatureFlags'; import { createWorkInProgress, @@ -84,7 +76,6 @@ function mergeDebugInfo( let didWarnAboutMaps; let didWarnAboutGenerators; -let didWarnAboutStringRefs; let ownerHasKeyUseWarning; let ownerHasFunctionTypeWarning; let ownerHasSymbolTypeWarning; @@ -93,7 +84,6 @@ let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {}; if (__DEV__) { didWarnAboutMaps = false; didWarnAboutGenerators = false; - didWarnAboutStringRefs = ({}: {[string]: boolean}); /** * Warn if there's no key explicitly set on dynamic arrays of children or @@ -137,10 +127,6 @@ if (__DEV__) { }; } -function isReactClass(type: any) { - return type.prototype && type.prototype.isReactComponent; -} - function unwrapThenable(thenable: Thenable): T { const index = thenableIndexCounter; thenableIndexCounter += 1; @@ -150,157 +136,27 @@ function unwrapThenable(thenable: Thenable): T { return trackUsedThenable(thenableState, thenable, index); } -type CoercedStringRef = ((handle: mixed) => void) & {_stringRef: ?string, ...}; - -function convertStringRefToCallbackRef( - returnFiber: Fiber, - current: Fiber | null, - element: ReactElement, - mixedRef: string | number | boolean, -): CoercedStringRef { - if (__DEV__) { - checkPropStringCoercion(mixedRef, 'ref'); - } - const stringRef = '' + (mixedRef: any); - - const owner: ?Fiber = (element._owner: any); - if (!owner) { - throw new Error( - `Element ref was specified as a string (${stringRef}) 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://react.dev/link/refs-must-have-owner for more information.', - ); - } - if (owner.tag !== ClassComponent) { - throw new Error( - 'Function components cannot have string refs. ' + - 'We recommend using useRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://react.dev/link/strict-mode-string-ref', - ); - } - - if (__DEV__) { - if ( - // Will already warn with "Function components cannot be given refs" - !(typeof element.type === 'function' && !isReactClass(element.type)) - ) { - const componentName = - getComponentNameFromFiber(returnFiber) || 'Component'; - if (!didWarnAboutStringRefs[componentName]) { - 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://react.dev/link/strict-mode-string-ref', - componentName, - stringRef, - ); - didWarnAboutStringRefs[componentName] = true; - } - } - } - - const inst = owner.stateNode; - if (!inst) { - throw new Error( - `Missing owner for string ref ${stringRef}. This error is likely caused by a ` + - 'bug in React. Please file an issue.', - ); - } - - // Check if previous string ref matches new string ref - if ( - current !== null && - current.ref !== null && - typeof current.ref === 'function' && - current.ref._stringRef === stringRef - ) { - // Reuse the existing string ref - const currentRef: CoercedStringRef = ((current.ref: any): CoercedStringRef); - return currentRef; - } - - // Create a new string ref - const ref = function (value: mixed) { - const refs = inst.refs; - if (value === null) { - delete refs[stringRef]; - } else { - refs[stringRef] = value; - } - }; - ref._stringRef = stringRef; - return ref; -} - function coerceRef( returnFiber: Fiber, current: Fiber | null, workInProgress: Fiber, element: ReactElement, ): void { - let mixedRef; + let ref; if (enableRefAsProp) { // TODO: This is a temporary, intermediate step. When enableRefAsProp is on, // we should resolve the `ref` prop during the begin phase of the component // it's attached to (HostComponent, ClassComponent, etc). const refProp = element.props.ref; - mixedRef = refProp !== undefined ? refProp : null; + ref = refProp !== undefined ? refProp : null; } else { // Old behavior. - mixedRef = element.ref; - } - - let coercedRef; - if ( - !disableStringRefs && - (typeof mixedRef === 'string' || - typeof mixedRef === 'number' || - typeof mixedRef === 'boolean') - ) { - coercedRef = convertStringRefToCallbackRef( - returnFiber, - current, - element, - mixedRef, - ); - - if (enableRefAsProp) { - // When enableRefAsProp is on, we should always use the props as the - // source of truth for refs. Not a field on the fiber. - // - // In the case of string refs, this presents a problem, because string - // refs are not passed around internally as strings; they are converted to - // callback refs. The ref used by the reconciler is not the same as the - // one the user provided. - // - // But since this is a deprecated feature anyway, what we can do is clone - // the props object and replace it with the internal callback ref. Then we - // can continue to use the props object as the source of truth. - // - // This means the internal callback ref will leak into userspace. The - // receiving component will receive a callback ref even though the parent - // passed a string. Which is weird, but again, this is a deprecated - // feature, and we're only leaving it around behind a flag so that Meta - // can keep using string refs temporarily while they finish migrating - // their codebase. - const userProvidedProps = workInProgress.pendingProps; - const propsWithInternalCallbackRef = assign({}, userProvidedProps); - propsWithInternalCallbackRef.ref = coercedRef; - workInProgress.pendingProps = propsWithInternalCallbackRef; - } - } else { - coercedRef = mixedRef; + ref = element.ref; } // TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We // should always read the ref from the prop. - workInProgress.ref = coercedRef; + workInProgress.ref = ref; } function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js index 9f892d3f91f33..6226e2bc228f2 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js @@ -1374,6 +1374,7 @@ describe('ReactIncrementalSideEffects', () => { '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 Bar (at **)\n' + ' in Foo (at **)', ]); expect(fooInstance.refs.bar.test).toEqual('test'); diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index f29f38a128b34..033156d25f0fb 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -558,7 +558,7 @@ describe 'ReactCoffeeScriptClass', -> '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 Foo (at **)' + ' in _Class (at **)' ]); expect(ref.current.refs.inner.getName()).toBe 'foo' diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index de9eab6399f9c..e7226dc0bee4e 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -602,7 +602,7 @@ describe('ReactES6Class', () => { '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 Foo (at **)', + ' in Inner (at **)', ]); expect(ref.current.refs.inner.getName()).toBe('foo'); }); diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js index 8dd9fceebc9c5..11bb847518e41 100644 --- a/packages/react/src/__tests__/ReactElementClone-test.js +++ b/packages/react/src/__tests__/ReactElementClone-test.js @@ -359,16 +359,20 @@ describe('ReactElementClone', () => { const clone = React.cloneElement(element, props); expect(clone.type).toBe(ComponentClass); expect(clone.key).toBe('12'); - if (gate(flags => flags.enableRefAsProp)) { + if (gate(flags => flags.enableRefAsProp && flags.disableStringRefs)) { expect(clone.props.ref).toBe('34'); expect(() => expect(clone.ref).toBe('34')).toErrorDev( 'Accessing element.ref was removed in React 19', {withoutStack: true}, ); expect(clone.props).toEqual({foo: 'ef', ref: '34'}); - } else { - expect(clone.ref).toBe('34'); + } else if ( + gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs) + ) { + expect(clone.ref).toBe(element.ref); expect(clone.props).toEqual({foo: 'ef'}); + } else { + // Not going to bother testing every possible combination. } if (__DEV__) { expect(Object.isFrozen(element)).toBe(true); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index 588b65f139ebe..eabf2a7bd59bb 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -997,11 +997,11 @@ describe('string refs', () => { root.render(); }); }).toErrorDev( - 'Warning: Component "StrictMode" contains the string ref "somestring". ' + + 'Warning: Component "OuterComponent" 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://react.dev/link/strict-mode-string-ref\n' + - ' in OuterComponent (at **)', + ' in InnerComponent (at **)', ); await act(() => { @@ -1036,11 +1036,11 @@ describe('string refs', () => { root.render(); }); }).toErrorDev( - 'Warning: Component "StrictMode" contains the string ref "somestring". ' + + 'Warning: Component "OuterComponent" 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://react.dev/link/strict-mode-string-ref\n' + - ' in OuterComponent (at **)', + ' in InnerComponent (at **)', ); await act(() => { diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index fa59b6ce644e5..5a6e81bccf329 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -704,7 +704,7 @@ describe('ReactTypeScriptClass', function() { '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 ClassicRefs (at **)', + ' in Inner (at **)', ]); expect(ref.current.refs.inner.getName()).toBe('foo'); }); diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index d20c1d4a25b56..6727626b4e5d4 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -23,6 +23,9 @@ import { disableStringRefs, disableDefaultPropsExceptForClasses, } from 'shared/ReactFeatureFlags'; +import {checkPropStringCoercion} from 'shared/CheckStringCoercion'; +import {ClassComponent} from 'react-reconciler/src/ReactWorkTags'; +import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame; @@ -342,6 +345,9 @@ export function jsxProd(type, config, maybeKey) { if (hasValidRef(config)) { if (!enableRefAsProp) { ref = config.ref; + if (!disableStringRefs) { + ref = coerceStringRef(ref, ReactCurrentOwner.current, type); + } } } @@ -353,7 +359,15 @@ export function jsxProd(type, config, maybeKey) { propName !== 'key' && (enableRefAsProp || propName !== 'ref') ) { - props[propName] = config[propName]; + if (enableRefAsProp && !disableStringRefs && propName === 'ref') { + props.ref = coerceStringRef( + config[propName], + ReactCurrentOwner.current, + type, + ); + } else { + props[propName] = config[propName]; + } } } @@ -555,6 +569,9 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { if (hasValidRef(config)) { if (!enableRefAsProp) { ref = config.ref; + if (!disableStringRefs) { + ref = coerceStringRef(ref, ReactCurrentOwner.current, type); + } } if (!disableStringRefs) { warnIfStringRefCannotBeAutoConverted(config, self); @@ -569,7 +586,15 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { propName !== 'key' && (enableRefAsProp || propName !== 'ref') ) { - props[propName] = config[propName]; + if (enableRefAsProp && !disableStringRefs && propName === 'ref') { + props.ref = coerceStringRef( + config[propName], + ReactCurrentOwner.current, + type, + ); + } else { + props[propName] = config[propName]; + } } } @@ -687,6 +712,9 @@ export function createElement(type, config, children) { if (hasValidRef(config)) { if (!enableRefAsProp) { ref = config.ref; + if (!disableStringRefs) { + ref = coerceStringRef(ref, ReactCurrentOwner.current, type); + } } if (__DEV__ && !disableStringRefs) { @@ -714,7 +742,15 @@ export function createElement(type, config, children) { propName !== '__self' && propName !== '__source' ) { - props[propName] = config[propName]; + if (enableRefAsProp && !disableStringRefs && propName === 'ref') { + props.ref = coerceStringRef( + config[propName], + ReactCurrentOwner.current, + type, + ); + } else { + props[propName] = config[propName]; + } } } } @@ -820,6 +856,9 @@ export function cloneElement(element, config, children) { if (!enableRefAsProp) { // Silently steal the ref from the parent. ref = config.ref; + if (!disableStringRefs) { + ref = coerceStringRef(ref, owner, element.type); + } } owner = ReactCurrentOwner.current; } @@ -866,7 +905,11 @@ export function cloneElement(element, config, children) { // Resolve default props props[propName] = defaultProps[propName]; } else { - props[propName] = config[propName]; + if (enableRefAsProp && !disableStringRefs && propName === 'ref') { + props.ref = coerceStringRef(config[propName], owner, element.type); + } else { + props[propName] = config[propName]; + } } } } @@ -1086,3 +1129,89 @@ function validateFragmentProps(fragment) { } } } + +function coerceStringRef(mixedRef, owner, type) { + if (disableStringRefs) { + return mixedRef; + } + + let stringRef; + if (typeof mixedRef === 'string') { + stringRef = mixedRef; + } else { + if (typeof mixedRef === 'number' || typeof mixedRef === 'boolean') { + if (__DEV__) { + checkPropStringCoercion(mixedRef, 'ref'); + } + stringRef = '' + mixedRef; + } else { + return mixedRef; + } + } + + return stringRefAsCallbackRef.bind(null, stringRef, type, owner); +} + +function stringRefAsCallbackRef(stringRef, type, owner, value) { + if (disableStringRefs) { + return; + } + if (!owner) { + throw new Error( + `Element ref was specified as a string (${stringRef}) 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://react.dev/link/refs-must-have-owner for more information.', + ); + } + if (owner.tag !== ClassComponent) { + throw new Error( + 'Function components cannot have string refs. ' + + 'We recommend using useRef() instead. ' + + 'Learn more about using refs safely here: ' + + 'https://react.dev/link/strict-mode-string-ref', + ); + } + + if (__DEV__) { + if ( + // Will already warn with "Function components cannot be given refs" + !(typeof type === 'function' && !isReactClass(type)) + ) { + const componentName = getComponentNameFromFiber(owner) || 'Component'; + if (!didWarnAboutStringRefs[componentName]) { + 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://react.dev/link/strict-mode-string-ref', + componentName, + stringRef, + ); + didWarnAboutStringRefs[componentName] = true; + } + } + } + + const inst = owner.stateNode; + if (!inst) { + throw new Error( + `Missing owner for string ref ${stringRef}. This error is likely caused by a ` + + 'bug in React. Please file an issue.', + ); + } + + const refs = inst.refs; + if (value === null) { + delete refs[stringRef]; + } else { + refs[stringRef] = value; + } +} + +function isReactClass(type) { + return type.prototype && type.prototype.isReactComponent; +}