diff --git a/packages/react-dom/src/__tests__/findDOMNode-test.js b/packages/react-dom/src/__tests__/findDOMNode-test.js index 2e7dab041f148..2be58e0c67115 100644 --- a/packages/react-dom/src/__tests__/findDOMNode-test.js +++ b/packages/react-dom/src/__tests__/findDOMNode-test.js @@ -12,6 +12,7 @@ const React = require('react'); const ReactDOM = require('react-dom'); const ReactTestUtils = require('react-dom/test-utils'); +const StrictMode = React.StrictMode; describe('findDOMNode', () => { it('findDOMNode should return null if passed null', () => { @@ -94,7 +95,72 @@ describe('findDOMNode', () => { return
; } } - expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); }); + + it('findDOMNode should warn if used to find a host component inside StrictMode', () => { + let parent = undefined; + let child = undefined; + + class ContainsStrictModeChild extends React.Component { + render() { + return ( + +
(child = n)} /> + + ); + } + } + + ReactTestUtils.renderIntoDocument( + (parent = n)} />, + ); + + let match; + expect(() => (match = ReactDOM.findDOMNode(parent))).toWarnDev([ + 'Warning: findDOMNode is deprecated in StrictMode. ' + + 'findDOMNode was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in div (at **)' + + '\n in StrictMode (at **)' + + '\n in ContainsStrictModeChild (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child); + }); + + it('findDOMNode should warn if passed a component that is inside StrictMode', () => { + let parent = undefined; + let child = undefined; + + class IsInStrictMode extends React.Component { + render() { + return
(child = n)} />; + } + } + + ReactTestUtils.renderIntoDocument( + + (parent = n)} /> + , + ); + + let match; + expect(() => (match = ReactDOM.findDOMNode(parent))).toWarnDev([ + 'Warning: findDOMNode is deprecated in StrictMode. ' + + 'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in div (at **)' + + '\n in IsInStrictMode (at **)' + + '\n in StrictMode (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child); + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 83563ee14f9d5..9803142d41fb3 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -614,7 +614,12 @@ const ReactDOM: Object = { if ((componentOrElement: any).nodeType === ELEMENT_NODE) { return (componentOrElement: any); } - + if (__DEV__) { + return DOMRenderer.findHostInstanceWithWarning( + componentOrElement, + 'findDOMNode', + ); + } return DOMRenderer.findHostInstance(componentOrElement); }, diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 806bfb0080e4e..77914d1c3889e 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -29,6 +29,8 @@ import warningWithoutStack from 'shared/warningWithoutStack'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; const findHostInstance = ReactFabricRenderer.findHostInstance; +const findHostInstanceWithWarning = + ReactFabricRenderer.findHostInstanceWithWarning; function findNodeHandle(componentOrHandle: any): ?number { if (__DEV__) { @@ -60,7 +62,16 @@ function findNodeHandle(componentOrHandle: any): ?number { if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { return componentOrHandle.canonical._nativeTag; } - const hostInstance = findHostInstance(componentOrHandle); + let hostInstance; + if (__DEV__) { + hostInstance = findHostInstanceWithWarning( + componentOrHandle, + 'findNodeHandle', + ); + } else { + hostInstance = findHostInstance(componentOrHandle); + } + if (hostInstance == null) { return hostInstance; } diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 203c3db213e7b..2414c540f8645 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -32,6 +32,8 @@ import warningWithoutStack from 'shared/warningWithoutStack'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; const findHostInstance = ReactNativeFiberRenderer.findHostInstance; +const findHostInstanceWithWarning = + ReactNativeFiberRenderer.findHostInstanceWithWarning; function findNodeHandle(componentOrHandle: any): ?number { if (__DEV__) { @@ -63,7 +65,16 @@ function findNodeHandle(componentOrHandle: any): ?number { if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { return componentOrHandle.canonical._nativeTag; } - const hostInstance = findHostInstance(componentOrHandle); + let hostInstance; + if (__DEV__) { + hostInstance = findHostInstanceWithWarning( + componentOrHandle, + 'findNodeHandle', + ); + } else { + hostInstance = findHostInstance(componentOrHandle); + } + if (hostInstance == null) { return hostInstance; } diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index d81c084af3eb3..8281020d4884a 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -15,6 +15,7 @@ let ReactFabric; let createReactNativeComponentClass; let UIManager; let FabricUIManager; +let StrictMode; jest.mock('shared/ReactFeatureFlags', () => require('shared/forks/ReactFeatureFlags.native-fabric-oss'), @@ -25,6 +26,7 @@ describe('ReactFabric', () => { jest.resetModules(); React = require('react'); + StrictMode = React.StrictMode; ReactFabric = require('react-native-renderer/fabric'); FabricUIManager = require('FabricUIManager'); UIManager = require('UIManager'); @@ -436,4 +438,79 @@ describe('ReactFabric', () => { // This could change in the future. expect(touchStart2).toBeCalled(); }); + + it('findNodeHandle should warn if used to find a host component inside StrictMode', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + let parent = undefined; + let child = undefined; + + class ContainsStrictModeChild extends React.Component { + render() { + return ( + + (child = n)} /> + + ); + } + } + + ReactFabric.render( (parent = n)} />, 11); + + let match; + expect(() => (match = ReactFabric.findNodeHandle(parent))).toWarnDev([ + 'Warning: findNodeHandle is deprecated in StrictMode. ' + + 'findNodeHandle was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in RCTView (at **)' + + '\n in StrictMode (at **)' + + '\n in ContainsStrictModeChild (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child._nativeTag); + }); + + it('findNodeHandle should warn if passed a component that is inside StrictMode', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + let parent = undefined; + let child = undefined; + + class IsInStrictMode extends React.Component { + render() { + return (child = n)} />; + } + } + + ReactFabric.render( + + (parent = n)} /> + , + 11, + ); + + let match; + expect(() => (match = ReactFabric.findNodeHandle(parent))).toWarnDev([ + 'Warning: findNodeHandle is deprecated in StrictMode. ' + + 'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in RCTView (at **)' + + '\n in IsInStrictMode (at **)' + + '\n in StrictMode (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child._nativeTag); + }); }); diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js index 79381c3db959c..3921aa1eb6759 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js @@ -11,6 +11,7 @@ 'use strict'; let React; +let StrictMode; let ReactNative; let createReactNativeComponentClass; let UIManager; @@ -20,6 +21,7 @@ describe('ReactNative', () => { jest.resetModules(); React = require('react'); + StrictMode = React.StrictMode; ReactNative = require('react-native-renderer'); UIManager = require('UIManager'); createReactNativeComponentClass = require('ReactNativeViewConfigRegistry') @@ -258,4 +260,79 @@ describe('ReactNative', () => { 11, ); }); + + it('findNodeHandle should warn if used to find a host component inside StrictMode', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + let parent = undefined; + let child = undefined; + + class ContainsStrictModeChild extends React.Component { + render() { + return ( + + (child = n)} /> + + ); + } + } + + ReactNative.render( (parent = n)} />, 11); + + let match; + expect(() => (match = ReactNative.findNodeHandle(parent))).toWarnDev([ + 'Warning: findNodeHandle is deprecated in StrictMode. ' + + 'findNodeHandle was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in RCTView (at **)' + + '\n in StrictMode (at **)' + + '\n in ContainsStrictModeChild (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child._nativeTag); + }); + + it('findNodeHandle should warn if passed a component that is inside StrictMode', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + let parent = undefined; + let child = undefined; + + class IsInStrictMode extends React.Component { + render() { + return (child = n)} />; + } + } + + ReactNative.render( + + (parent = n)} /> + , + 11, + ); + + let match; + expect(() => (match = ReactNative.findNodeHandle(parent))).toWarnDev([ + 'Warning: findNodeHandle is deprecated in StrictMode. ' + + 'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n' + + '\n in RCTView (at **)' + + '\n in IsInStrictMode (at **)' + + '\n in StrictMode (at **)' + + '\n' + + '\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + ]); + expect(match).toBe(child._nativeTag); + }); }); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 5cb913b5d1a1f..00fab26d8e2b7 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -522,6 +522,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { if (typeof component.id === 'number') { return component; } + if (__DEV__) { + return NoopRenderer.findHostInstanceWithWarning( + component, + 'findInstance', + ); + } return NoopRenderer.findHostInstance(component); }, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index ca072d0c938f6..4d383f3354983 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -61,6 +61,8 @@ import { import {createUpdate, enqueueUpdate} from './ReactUpdateQueue'; import ReactFiberInstrumentation from './ReactFiberInstrumentation'; import * as ReactCurrentFiber from './ReactCurrentFiber'; +import {getStackByFiberInDevAndProd} from './ReactCurrentFiber'; +import {StrictMode} from './ReactTypeOfMode'; type OpaqueRoot = FiberRoot; @@ -82,9 +84,11 @@ type DevToolsConfig = {| |}; let didWarnAboutNestedUpdates; +let didWarnAboutFindNodeInStrictMode; if (__DEV__) { didWarnAboutNestedUpdates = false; + didWarnAboutFindNodeInStrictMode = {}; } function getContextForSubtree( @@ -209,6 +213,67 @@ function findHostInstance(component: Object): PublicInstance | null { return hostFiber.stateNode; } +function findHostInstanceWithWarning( + component: Object, + methodName: string, +): PublicInstance | null { + if (__DEV__) { + const fiber = ReactInstanceMap.get(component); + if (fiber === undefined) { + if (typeof component.render === 'function') { + invariant(false, 'Unable to find node on an unmounted component.'); + } else { + invariant( + false, + 'Argument appears to not be a ReactComponent. Keys: %s', + Object.keys(component), + ); + } + } + const hostFiber = findCurrentHostFiber(fiber); + if (hostFiber === null) { + return null; + } + if (hostFiber.mode & StrictMode) { + const componentName = getComponentName(fiber.type) || 'Component'; + if (!didWarnAboutFindNodeInStrictMode[componentName]) { + didWarnAboutFindNodeInStrictMode[componentName] = true; + if (fiber.mode & StrictMode) { + warningWithoutStack( + false, + '%s is deprecated in StrictMode. ' + + '%s was passed an instance of %s which is inside StrictMode. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n%s' + + '\n\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + methodName, + methodName, + componentName, + getStackByFiberInDevAndProd(hostFiber), + ); + } else { + warningWithoutStack( + false, + '%s is deprecated in StrictMode. ' + + '%s was passed an instance of %s which renders StrictMode children. ' + + 'Instead, add a ref directly to the element you want to reference.' + + '\n%s' + + '\n\nLearn more about using refs safely here:' + + '\nhttps://fb.me/react-strict-mode-find-node', + methodName, + methodName, + componentName, + getStackByFiberInDevAndProd(hostFiber), + ); + } + } + } + return hostFiber.stateNode; + } + return findHostInstance(component); +} + export function createContainer( containerInfo: Container, isConcurrent: boolean, @@ -266,6 +331,8 @@ export function getPublicRootInstance( export {findHostInstance}; +export {findHostInstanceWithWarning}; + export function findHostInstanceWithNoPortals( fiber: Fiber, ): PublicInstance | null { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js index b19d7de430bc9..fa3dc6e7e0139 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js @@ -138,22 +138,35 @@ describe('ReactIncrementalReflection', () => { let classInstance = null; + function findInstance(inst) { + // We ignore warnings fired by findInstance because we are testing + // that the actual behavior still works as expected even though it + // is deprecated. + let oldConsoleError = console.error; + console.error = jest.fn(); + try { + return ReactNoop.findInstance(inst); + } finally { + console.error = oldConsoleError; + } + } + class Component extends React.Component { UNSAFE_componentWillMount() { classInstance = this; - ops.push('componentWillMount', ReactNoop.findInstance(this)); + ops.push('componentWillMount', findInstance(this)); } componentDidMount() { - ops.push('componentDidMount', ReactNoop.findInstance(this)); + ops.push('componentDidMount', findInstance(this)); } UNSAFE_componentWillUpdate() { - ops.push('componentWillUpdate', ReactNoop.findInstance(this)); + ops.push('componentWillUpdate', findInstance(this)); } componentDidUpdate() { - ops.push('componentDidUpdate', ReactNoop.findInstance(this)); + ops.push('componentDidUpdate', findInstance(this)); } componentWillUnmount() { - ops.push('componentWillUnmount', ReactNoop.findInstance(this)); + ops.push('componentWillUnmount', findInstance(this)); } render() { ops.push('render'); @@ -193,7 +206,7 @@ describe('ReactIncrementalReflection', () => { expect(classInstance).toBeDefined(); // The instance has been complete but is still not committed so it should // not find any host nodes in it. - expect(ReactNoop.findInstance(classInstance)).toBe(null); + expect(findInstance(classInstance)).toBe(null); expect(ReactNoop.flush).toWarnDev( 'componentWillMount: Please update the following components ' + @@ -206,7 +219,7 @@ describe('ReactIncrementalReflection', () => { const hostSpan = classInstance.span; expect(hostSpan).toBeDefined(); - expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); + expect(findInstance(classInstance)).toBe(hostSpan); expect(ops).toEqual(['componentDidMount', hostSpan]);