diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index b1886ad0a6933..5af06211a7524 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -12,6 +12,7 @@ import type {ReactPortal} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {ExpirationTime} from 'react-reconciler/src/ReactFiberExpirationTime'; +import getComponentName from 'shared/getComponentName'; import {Placement, Deletion} from 'shared/ReactTypeOfSideEffect'; import { getIteratorFn, @@ -26,6 +27,7 @@ import { HostPortal, Fragment, } from 'shared/ReactTypeOfWork'; +import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; import emptyObject from 'fbjs/lib/emptyObject'; import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; @@ -38,16 +40,20 @@ import { createFiberFromPortal, } from './ReactFiber'; import ReactDebugCurrentFiber from './ReactDebugCurrentFiber'; +import {StrictMode} from './ReactTypeOfMode'; const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber; let didWarnAboutMaps; +let didWarnAboutStringRefInStrictMode; let ownerHasKeyUseWarning; let ownerHasFunctionTypeWarning; let warnForMissingKey = (child: mixed) => {}; if (__DEV__) { didWarnAboutMaps = false; + didWarnAboutStringRefInStrictMode = {}; + /** * Warn if there's no key explicitly set on dynamic arrays of children or * object keys are not valid. This allows us to keep track of children between @@ -92,9 +98,32 @@ if (__DEV__) { const isArray = Array.isArray; -function coerceRef(current: Fiber | null, element: ReactElement) { +function coerceRef( + returnFiber: Fiber, + current: Fiber | null, + element: ReactElement, +) { let mixedRef = element.ref; if (mixedRef !== null && typeof mixedRef !== 'function') { + if (__DEV__) { + if (returnFiber.mode & StrictMode) { + const componentName = getComponentName(returnFiber) || 'Component'; + if (!didWarnAboutStringRefInStrictMode[componentName]) { + warning( + false, + 'A string ref has been found within a strict mode tree. ' + + 'String refs are a source of potential bugs and should be avoided. ' + + 'We recommend using a ref callback instead.' + + '\n%s' + + '\n\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-string-ref', + getStackAddendumByWorkInProgressFiber(returnFiber), + ); + didWarnAboutStringRefInStrictMode[componentName] = true; + } + } + } + if (element._owner) { const owner: ?Fiber = (element._owner: any); let inst; @@ -340,7 +369,7 @@ function ChildReconciler(shouldTrackSideEffects) { if (current !== null && current.type === element.type) { // Move based on index const existing = useFiber(current, element.props, expirationTime); - existing.ref = coerceRef(current, element); + existing.ref = coerceRef(returnFiber, current, element); existing.return = returnFiber; if (__DEV__) { existing._debugSource = element._source; @@ -354,7 +383,7 @@ function ChildReconciler(shouldTrackSideEffects) { returnFiber.mode, expirationTime, ); - created.ref = coerceRef(current, element); + created.ref = coerceRef(returnFiber, current, element); created.return = returnFiber; return created; } @@ -439,7 +468,7 @@ function ChildReconciler(shouldTrackSideEffects) { returnFiber.mode, expirationTime, ); - created.ref = coerceRef(null, newChild); + created.ref = coerceRef(returnFiber, null, newChild); created.return = returnFiber; return created; } @@ -1077,7 +1106,7 @@ function ChildReconciler(shouldTrackSideEffects) { : element.props, expirationTime, ); - existing.ref = coerceRef(child, element); + existing.ref = coerceRef(returnFiber, child, element); existing.return = returnFiber; if (__DEV__) { existing._debugSource = element._source; @@ -1109,7 +1138,7 @@ function ChildReconciler(shouldTrackSideEffects) { returnFiber.mode, expirationTime, ); - created.ref = coerceRef(currentFirstChild, element); + created.ref = coerceRef(returnFiber, currentFirstChild, element); created.return = returnFiber; return created; } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js index d093ebbb16d17..e10e2bb4c57f3 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js @@ -1074,7 +1074,9 @@ describe('ReactIncrementalSideEffects', () => { } ReactNoop.render(); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'Warning: A string ref has been found within a strict mode tree.', + ); expect(fooInstance.refs.bar.test).toEqual('test'); }); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.internal.js b/packages/react/src/__tests__/ReactStrictMode-test.internal.js index e4c1fa94d142f..9276dc18bde03 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.internal.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.internal.js @@ -745,4 +745,89 @@ describe('ReactStrictMode', () => { expect(rendered.toJSON()).toBe('count:2'); }); }); + + describe('string refs', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactTestRenderer = require('react-test-renderer'); + }); + + 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; + } + } + + let renderer; + expect(() => { + renderer = ReactTestRenderer.create(); + }).toWarnDev( + 'Warning: A string ref has been found within a strict mode tree. ' + + 'String refs are a source of potential bugs and should be avoided. ' + + 'We recommend using a ref callback instead.\n\n' + + ' in OuterComponent (at **)\n\n' + + 'Learn more about using refs safely here:\n' + + 'https://fb.me/react-strict-mode-string-ref', + ); + + // Dedup + renderer.update(); + }); + + 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; + } + } + + let renderer; + expect(() => { + renderer = ReactTestRenderer.create(); + }).toWarnDev( + 'Warning: A string ref has been found within a strict mode tree. ' + + 'String refs are a source of potential bugs and should be avoided. ' + + 'We recommend using a ref callback instead.\n\n' + + ' in InnerComponent (at **)\n' + + ' in OuterComponent (at **)\n\n' + + 'Learn more about using refs safely here:\n' + + 'https://fb.me/react-strict-mode-string-ref', + ); + + // Dedup + renderer.update(); + }); + }); });