Deprecate findDOMNode in StrictMode (#13841)

* Deprecate findDOMNode in StrictMode

There are two scenarios. One is that we pass a component instance that is
already in strict mode or the node that we find is in strict mode if
an outer component renders into strict mode.

I use a separate method findHostInstanceWithWarning for this so that
a) I can pass the method name (findDOMNode/findNodeHandle).
b) Can ignore this warning in React Native mixins/NativeComponent that use this helper.

I don't want to expose the fiber to the renderers themselves.
This commit is contained in:
Sebastian Markbåge
2018-10-12 15:42:00 -07:00
committed by GitHub
parent c9be16f5b6
commit 4773fdf7cd
9 changed files with 344 additions and 11 deletions
+67 -1
View File
@@ -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 <div />;
}
}
expect(() => ReactTestUtils.renderIntoDocument(<Bar />)).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 (
<StrictMode>
<div ref={n => (child = n)} />
</StrictMode>
);
}
}
ReactTestUtils.renderIntoDocument(
<ContainsStrictModeChild ref={n => (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 <div ref={n => (child = n)} />;
}
}
ReactTestUtils.renderIntoDocument(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
);
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);
});
});
+6 -1
View File
@@ -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);
},
+12 -1
View File
@@ -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;
}
+12 -1
View File
@@ -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;
}
@@ -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 (
<StrictMode>
<View ref={n => (child = n)} />
</StrictMode>
);
}
}
ReactFabric.render(<ContainsStrictModeChild ref={n => (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 <View ref={n => (child = n)} />;
}
}
ReactFabric.render(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
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);
});
});
@@ -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 (
<StrictMode>
<View ref={n => (child = n)} />
</StrictMode>
);
}
}
ReactNative.render(<ContainsStrictModeChild ref={n => (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 <View ref={n => (child = n)} />;
}
}
ReactNative.render(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
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);
});
});
+6
View File
@@ -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);
},
+67
View File
@@ -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 {
@@ -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]);